From 8c60a6716d7ff3bb4a9e13a9b89ee5622e4fde7f Mon Sep 17 00:00:00 2001 From: Chris Dickens Date: Thu, 5 Jun 2014 02:04:11 -0700 Subject: core: Reuse poll fds across calls to handle_events() Prior to this patch, the array of poll fds given to poll() was allocated and freed every time handle_events() was called. This is unnecessary if the list of poll fds has not changed since the last call to handle_events(). With this patch, the array and count of poll fds is stored in the context and only reallocated when the list of poll fds changes, saving any unnecessary overhead. Signed-off-by: Chris Dickens --- libusb/io.c | 77 ++++++++++++++++++++++++++++++--------------------- libusb/libusbi.h | 25 ++++++++++------- libusb/version_nano.h | 2 +- 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/libusb/io.c b/libusb/io.c index 430f67e..9f7fc51 100644 --- a/libusb/io.c +++ b/libusb/io.c @@ -20,6 +20,7 @@ */ #include "config.h" +#include #include #include #include @@ -1115,7 +1116,7 @@ int usbi_io_init(struct libusb_context *ctx) usbi_mutex_init(&ctx->event_waiters_lock, NULL); usbi_cond_init(&ctx->event_waiters_cond, NULL); list_init(&ctx->flying_transfers); - list_init(&ctx->pollfds); + list_init(&ctx->ipollfds); /* FIXME should use an eventfd on kernels that support it */ r = usbi_pipe(ctx->ctrl_pipe); @@ -1194,6 +1195,8 @@ void usbi_io_exit(struct libusb_context *ctx) usbi_mutex_destroy(&ctx->events_lock); usbi_mutex_destroy(&ctx->event_waiters_lock); usbi_cond_destroy(&ctx->event_waiters_cond); + if (ctx->pollfds) + free(ctx->pollfds); } static int calculate_timeout(struct usbi_transfer *transfer) @@ -1982,26 +1985,39 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv) else internal_nfds = 2; + /* only reallocate the poll fds when the list of poll fds has been modified + * since the last poll, otherwise reuse them to save the additional overhead */ usbi_mutex_lock(&ctx->pollfds_lock); - list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd) - nfds++; + if (ctx->pollfds_modified) { + usbi_dbg("poll fds modified, reallocating"); - /* TODO: malloc when number of fd's changes, not on every poll */ - if (nfds != 0) - fds = malloc(sizeof(*fds) * nfds); - if (!fds) { - usbi_mutex_unlock(&ctx->pollfds_lock); - return LIBUSB_ERROR_NO_MEM; - } + if (ctx->pollfds) { + free(ctx->pollfds); + ctx->pollfds = NULL; + } + + /* sanity check - it is invalid for a context to have fewer than the + * required internal fds (memory corruption?) */ + assert(ctx->num_pollfds >= internal_nfds); + + ctx->pollfds = calloc(ctx->num_pollfds, sizeof(*ctx->pollfds)); + if (!ctx->pollfds) { + usbi_mutex_unlock(&ctx->pollfds_lock); + return LIBUSB_ERROR_NO_MEM; + } + + list_for_each_entry(ipollfd, &ctx->ipollfds, list, struct usbi_pollfd) { + struct libusb_pollfd *pollfd = &ipollfd->pollfd; + i++; + ctx->pollfds[i].fd = pollfd->fd; + ctx->pollfds[i].events = pollfd->events; + } - list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd) { - struct libusb_pollfd *pollfd = &ipollfd->pollfd; - int fd = pollfd->fd; - i++; - fds[i].fd = fd; - fds[i].events = pollfd->events; - fds[i].revents = 0; + /* reset the flag now that we have the updated list */ + ctx->pollfds_modified = 0; } + fds = ctx->pollfds; + nfds = ctx->num_pollfds; usbi_mutex_unlock(&ctx->pollfds_lock); timeout_ms = (int)(tv->tv_sec * 1000) + (tv->tv_usec / 1000); @@ -2014,14 +2030,11 @@ redo_poll: usbi_dbg("poll() %d fds with timeout in %dms", nfds, timeout_ms); r = usbi_poll(fds, nfds, timeout_ms); usbi_dbg("poll() returned %d", r); - if (r == 0) { - free(fds); + if (r == 0) return handle_timeouts(ctx); - } else if (r == -1 && errno == EINTR) { - free(fds); + else if (r == -1 && errno == EINTR) return LIBUSB_ERROR_INTERRUPTED; - } else if (r < 0) { - free(fds); + else if (r < 0) { usbi_err(ctx, "poll failed %d err=%d\n", r, errno); return LIBUSB_ERROR_IO; } @@ -2096,7 +2109,6 @@ handled: goto redo_poll; } - free(fds); return r; } @@ -2477,7 +2489,9 @@ int usbi_add_pollfd(struct libusb_context *ctx, int fd, short events) ipollfd->pollfd.fd = fd; ipollfd->pollfd.events = events; usbi_mutex_lock(&ctx->pollfds_lock); - list_add_tail(&ipollfd->list, &ctx->pollfds); + list_add_tail(&ipollfd->list, &ctx->ipollfds); + ctx->num_pollfds++; + ctx->pollfds_modified = 1; usbi_mutex_unlock(&ctx->pollfds_lock); if (ctx->fd_added_cb) @@ -2493,7 +2507,7 @@ void usbi_remove_pollfd(struct libusb_context *ctx, int fd) usbi_dbg("remove fd %d", fd); usbi_mutex_lock(&ctx->pollfds_lock); - list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd) + list_for_each_entry(ipollfd, &ctx->ipollfds, list, struct usbi_pollfd) if (ipollfd->pollfd.fd == fd) { found = 1; break; @@ -2506,6 +2520,8 @@ void usbi_remove_pollfd(struct libusb_context *ctx, int fd) } list_del(&ipollfd->list); + ctx->num_pollfds--; + ctx->pollfds_modified = 1; usbi_mutex_unlock(&ctx->pollfds_lock); free(ipollfd); if (ctx->fd_removed_cb) @@ -2535,20 +2551,17 @@ const struct libusb_pollfd ** LIBUSB_CALL libusb_get_pollfds( struct libusb_pollfd **ret = NULL; struct usbi_pollfd *ipollfd; size_t i = 0; - size_t cnt = 0; USBI_GET_CONTEXT(ctx); usbi_mutex_lock(&ctx->pollfds_lock); - list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd) - cnt++; - ret = calloc(cnt + 1, sizeof(struct libusb_pollfd *)); + ret = calloc(ctx->num_pollfds + 1, sizeof(struct libusb_pollfd *)); if (!ret) goto out; - list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd) + list_for_each_entry(ipollfd, &ctx->ipollfds, list, struct usbi_pollfd) ret[i++] = (struct libusb_pollfd *) ipollfd; - ret[cnt] = NULL; + ret[ctx->num_pollfds] = NULL; out: usbi_mutex_unlock(&ctx->pollfds_lock); diff --git a/libusb/libusbi.h b/libusb/libusbi.h index f4e9973..6e7c5ff 100644 --- a/libusb/libusbi.h +++ b/libusb/libusbi.h @@ -220,6 +220,14 @@ static inline void usbi_dbg(const char *format, ...) #define IS_XFERIN(xfer) (0 != ((xfer)->endpoint & LIBUSB_ENDPOINT_IN)) #define IS_XFEROUT(xfer) (!IS_XFERIN(xfer)) +/* Internal abstraction for poll (needs struct usbi_transfer on Windows) */ +#if defined(OS_LINUX) || defined(OS_DARWIN) || defined(OS_OPENBSD) || defined(OS_NETBSD) +#include +#include "os/poll_posix.h" +#elif defined(OS_WINDOWS) || defined(OS_WINCE) +#include "os/poll_windows.h" +#endif + /* Internal abstraction for thread synchronization */ #if defined(THREADS_POSIX) #include "os/threads_posix.h" @@ -257,8 +265,13 @@ struct libusb_context { struct list_head flying_transfers; usbi_mutex_t flying_transfers_lock; - /* list of poll fds */ - struct list_head pollfds; + /* list and count of poll fds and an array of poll fd structures that is + * (re)allocated as necessary prior to polling, and a flag to indicate + * when the list of poll fds has changed since the last poll. */ + struct list_head ipollfds; + POLL_NFDS_TYPE num_pollfds; + struct pollfd *pollfds; + unsigned int pollfds_modified; usbi_mutex_t pollfds_lock; /* a counter that is set when we want to interrupt event handling, in order @@ -445,14 +458,6 @@ int usbi_get_config_index_by_value(struct libusb_device *dev, void usbi_connect_device (struct libusb_device *dev); void usbi_disconnect_device (struct libusb_device *dev); -/* Internal abstraction for poll (needs struct usbi_transfer on Windows) */ -#if defined(OS_LINUX) || defined(OS_DARWIN) || defined(OS_OPENBSD) || defined(OS_NETBSD) -#include -#include "os/poll_posix.h" -#elif defined(OS_WINDOWS) || defined(OS_WINCE) -#include "os/poll_windows.h" -#endif - #if (defined(OS_WINDOWS) || defined(OS_WINCE)) && !defined(__GNUC__) #define snprintf _snprintf #define vsnprintf _vsnprintf diff --git a/libusb/version_nano.h b/libusb/version_nano.h index 42fac54..ada73a8 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 10905 +#define LIBUSB_NANO 10911 -- cgit v1.2.3