Message ID | 1397821874-13546-1-git-send-email-s.vorobiov@samsung.com |
---|---|
State | New |
Headers | show |
Please see my remarks below. Am 18.04.2014 13:51, schrieb Stanislav Vorobiov: > From: Sangho Park <sangho1206.park@samsung.com> > > g_poll has a problem on windows when using > timeouts < 10ms, in glib/gpoll.c: > > /* If not, and we have a significant timeout, poll again with > * timeout then. Note that this will return indication for only > * one event, or only for messages. We ignore timeouts less than > * ten milliseconds as they are mostly pointless on Windows, the > * MsgWaitForMultipleObjectsEx() call will timeout right away > * anyway. > */ > if (retval == 0 && (timeout == INFINITE || timeout >= 10)) > retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); > > so whenever g_poll is called with timeout < 10ms it does > a quick poll instead of wait, this causes significant performance > degradation of qemu, thus we should use WaitForMultipleObjectsEx > directly > > Signed-off-by: Stanislav Vorobiov <s.vorobiov@samsung.com> > --- > include/glib-compat.h | 19 +++++++++ > include/qemu-common.h | 12 ------ > util/oslib-win32.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 131 insertions(+), 12 deletions(-) > > diff --git a/include/glib-compat.h b/include/glib-compat.h > index 8aa77af..2d4258e 100644 > --- a/include/glib-compat.h > +++ b/include/glib-compat.h > @@ -24,4 +24,23 @@ static inline guint g_timeout_add_seconds(guint interval, GSourceFunc function, > } > #endif > > +#ifdef _WIN32 > +/* > + * g_poll has a problem on windows when using > + * timeouts < 10ms, so use wrapper. > + */ > +#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, qemu_timeout_ns_to_ms(timeout)) qemu_timeout_ns_to_ms is not needed here (g_poll is called with timeout given in ms, and so is g_poll_fixed). > +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout_ms); > +#elif !GLIB_CHECK_VERSION(2, 20, 0) > +/* > + * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly > + * on older systems. > + */ > +static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) > +{ > + GMainContext *ctx = g_main_context_default(); > + return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); > +} > +#endif > + > #endif > diff --git a/include/qemu-common.h b/include/qemu-common.h > index a998e8d..3f3fd60 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -124,18 +124,6 @@ int qemu_main(int argc, char **argv, char **envp); > void qemu_get_timedate(struct tm *tm, int offset); > int qemu_timedate_diff(struct tm *tm); > > -#if !GLIB_CHECK_VERSION(2, 20, 0) > -/* > - * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly > - * on older systems. > - */ > -static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) > -{ > - GMainContext *ctx = g_main_context_default(); > - return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); > -} > -#endif > - > /** > * is_help_option: > * @s: string to test > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 93f7d35..006259e 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -238,3 +238,115 @@ char *qemu_get_exec_dir(void) > { > return g_strdup(exec_dir); > } > + > +/* > + * g_poll has a problem on windows when using Windows > + * timeouts < 10ms, in glib/gpoll.c: > + * > + * // If not, and we have a significant timeout, poll again with > + * // timeout then. Note that this will return indication for only > + * // one event, or only for messages. We ignore timeouts less than > + * // ten milliseconds as they are mostly pointless on Windows, the > + * // MsgWaitForMultipleObjectsEx() call will timeout right away > + * // anyway. > + * > + * if (retval == 0 && (timeout == INFINITE || timeout >= 10)) > + * retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); > + * > + * So whenever g_poll is called with timeout < 10ms it does > + * a quick poll instead of wait, this causes significant performance > + * degradation of qemu, thus we should use WaitForMultipleObjectsEx QEMU > + * directly > + */ > +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout_ms) > +{ > + guint i; > + HANDLE handles[MAXIMUM_WAIT_OBJECTS]; > + gint nhandles = 0; > + int num_completed = 0; > + > + for (i = 0; i < nfds; i++) { > + gint j; > + > + if (fds[i].fd <= 0) { > + continue; > + } > + > + /* don't add same handle several times > + */ > + for (j = 0; j < nhandles; j++) { > + if (handles[j] == (HANDLE)fds[i].fd) { > + break; > + } > + } > + > + if (j == nhandles) { > + if (nhandles == MAXIMUM_WAIT_OBJECTS) { > + fprintf(stderr, "Too many handles to wait for!\n"); > + break; > + } else { > + handles[nhandles++] = (HANDLE)fds[i].fd; > + } > + } > + } > + > + for (i = 0; i < nfds; ++i) { > + fds[i].revents = 0; > + } > + > + if (timeout_ms == -1) { > + timeout_ms = INFINITE; > + } > + > + if (nhandles == 0) { > + if (timeout_ms == INFINITE) { > + return -1; > + } else { > + SleepEx(timeout_ms, TRUE); > + return 0; > + } > + } > + > + while (1) { > + DWORD res; > + gint j; > + > + res = WaitForMultipleObjectsEx(nhandles, handles, FALSE, > + timeout_ms, TRUE); > + > + if (res == WAIT_FAILED) { > + for (i = 0; i < nfds; ++i) { > + fds[i].revents = 0; > + } > + > + return -1; > + } else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || > + ((int)res < WAIT_OBJECT_0) || > + (res >= (WAIT_OBJECT_0 + nhandles))) { > + break; > + } > + > + for (i = 0; i < nfds; ++i) { > + if (handles[res - WAIT_OBJECT_0] == (HANDLE)fds[i].fd) { > + fds[i].revents = fds[i].events; > + } > + } > + > + ++num_completed; > + > + if (nhandles <= 1) { > + break; > + } > + > + /* poll the rest of the handles > + */ > + for (j = res - WAIT_OBJECT_0 + 1; j < nhandles; j++) { > + handles[j - 1] = handles[j]; > + } > + --nhandles; > + > + timeout_ms = 0; > + } > + > + return num_completed; > +} > Please send patches as top level e-mails (no In-Reply-To) and cc at least one maintainer (otherwise you risk that your patch is not noticed). Thanks, Stefan
diff --git a/include/glib-compat.h b/include/glib-compat.h index 8aa77af..2d4258e 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -24,4 +24,23 @@ static inline guint g_timeout_add_seconds(guint interval, GSourceFunc function, } #endif +#ifdef _WIN32 +/* + * g_poll has a problem on windows when using + * timeouts < 10ms, so use wrapper. + */ +#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, qemu_timeout_ns_to_ms(timeout)) +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout_ms); +#elif !GLIB_CHECK_VERSION(2, 20, 0) +/* + * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly + * on older systems. + */ +static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) +{ + GMainContext *ctx = g_main_context_default(); + return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); +} +#endif + #endif diff --git a/include/qemu-common.h b/include/qemu-common.h index a998e8d..3f3fd60 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -124,18 +124,6 @@ int qemu_main(int argc, char **argv, char **envp); void qemu_get_timedate(struct tm *tm, int offset); int qemu_timedate_diff(struct tm *tm); -#if !GLIB_CHECK_VERSION(2, 20, 0) -/* - * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly - * on older systems. - */ -static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) -{ - GMainContext *ctx = g_main_context_default(); - return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); -} -#endif - /** * is_help_option: * @s: string to test diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 93f7d35..006259e 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -238,3 +238,115 @@ char *qemu_get_exec_dir(void) { return g_strdup(exec_dir); } + +/* + * g_poll has a problem on windows when using + * timeouts < 10ms, in glib/gpoll.c: + * + * // If not, and we have a significant timeout, poll again with + * // timeout then. Note that this will return indication for only + * // one event, or only for messages. We ignore timeouts less than + * // ten milliseconds as they are mostly pointless on Windows, the + * // MsgWaitForMultipleObjectsEx() call will timeout right away + * // anyway. + * + * if (retval == 0 && (timeout == INFINITE || timeout >= 10)) + * retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); + * + * So whenever g_poll is called with timeout < 10ms it does + * a quick poll instead of wait, this causes significant performance + * degradation of qemu, thus we should use WaitForMultipleObjectsEx + * directly + */ +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout_ms) +{ + guint i; + HANDLE handles[MAXIMUM_WAIT_OBJECTS]; + gint nhandles = 0; + int num_completed = 0; + + for (i = 0; i < nfds; i++) { + gint j; + + if (fds[i].fd <= 0) { + continue; + } + + /* don't add same handle several times + */ + for (j = 0; j < nhandles; j++) { + if (handles[j] == (HANDLE)fds[i].fd) { + break; + } + } + + if (j == nhandles) { + if (nhandles == MAXIMUM_WAIT_OBJECTS) { + fprintf(stderr, "Too many handles to wait for!\n"); + break; + } else { + handles[nhandles++] = (HANDLE)fds[i].fd; + } + } + } + + for (i = 0; i < nfds; ++i) { + fds[i].revents = 0; + } + + if (timeout_ms == -1) { + timeout_ms = INFINITE; + } + + if (nhandles == 0) { + if (timeout_ms == INFINITE) { + return -1; + } else { + SleepEx(timeout_ms, TRUE); + return 0; + } + } + + while (1) { + DWORD res; + gint j; + + res = WaitForMultipleObjectsEx(nhandles, handles, FALSE, + timeout_ms, TRUE); + + if (res == WAIT_FAILED) { + for (i = 0; i < nfds; ++i) { + fds[i].revents = 0; + } + + return -1; + } else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || + ((int)res < WAIT_OBJECT_0) || + (res >= (WAIT_OBJECT_0 + nhandles))) { + break; + } + + for (i = 0; i < nfds; ++i) { + if (handles[res - WAIT_OBJECT_0] == (HANDLE)fds[i].fd) { + fds[i].revents = fds[i].events; + } + } + + ++num_completed; + + if (nhandles <= 1) { + break; + } + + /* poll the rest of the handles + */ + for (j = res - WAIT_OBJECT_0 + 1; j < nhandles; j++) { + handles[j - 1] = handles[j]; + } + --nhandles; + + timeout_ms = 0; + } + + return num_completed; +}