[prev in list] [next in list] [prev in thread] [next in thread]
List: libusb-devel
Subject: Re: [Libusb-devel] OSX performance bug (patches for consideration)
From: Vitali Lovich <vlovich () gmail ! com>
Date: 2011-12-17 8:32:45
Message-ID: CAF8PYMhbSN4ZXYNo4Aks9ruV43ooK29qq2o-EbgmowpGNuwJHg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Take 2 with the patches. 0001 merges in the 0003 bug fixes from above.
0002 is unchanged.
Cheers,
Vitali
On Sat, Dec 17, 2011 at 3:29 AM, Vitali Lovich <vlovich@gmail.com> wrote:
> Here's the patch that fixes the I/O issue I found.
>
>
> On Fri, Dec 16, 2011 at 4:39 PM, Vitali Lovich <vlovich@gmail.com> wrote:
>
>> The first patch caches the device handle when the libusb_device structure
>> is created. The second patch removes darwin_get_device all-together &
>> cleans up some other code paths.
>>
>> I tested the performance impact & it definitely fixed my problem. Only
>> cursory smoke test performed (i.e. my app still works) - don't think these
>> patches introduce any memory leaks or instability.
>>
>
>
[Attachment #5 (text/html)]
Take 2 with the patches. 0001 merges in the 0003 bug fixes from above. 0002 is \
unchanged.<br><br>Cheers,<br>Vitali<br><br><div class="gmail_quote">On Sat, Dec 17, \
2011 at 3:29 AM, Vitali Lovich <span dir="ltr"><<a \
href="mailto:vlovich@gmail.com">vlovich@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Here's the patch that fixes the I/O issue I found.<div \
class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">
On Fri, Dec 16, 2011 at 4:39 PM, Vitali Lovich <span dir="ltr"><<a \
href="mailto:vlovich@gmail.com" target="_blank">vlovich@gmail.com</a>></span> \
wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">The first patch caches the device handle when the \
libusb_device structure is created. The second patch removes darwin_get_device \
all-together & cleans up some other code paths.<div>
<br></div><div>I tested the performance impact & it definitely fixed my problem. \
Only cursory smoke test performed (i.e. my app still works) - don't think these \
patches introduce any memory leaks or instability.</div>
</blockquote></div><br>
</div></div></blockquote></div><br>
--20cf30050ed20e556804b445919c--
["0001-Cache-device-location-when-device-struct-is-created.patch" (application/octet-stream)]
From 238d7948fcccef77ab3097a42617f8364449bb03 Mon Sep 17 00:00:00 2001
From: Vitali Lovich <vlovich@vlovich.com>
Date: Fri, 16 Dec 2011 16:22:03 -0500
Subject: [PATCH 1/2] Cache device location when device struct is created
Instead of searching for it unnecessarily on every open, cache the
result & free it when the device struct is destroyed.
---
libusb/os/darwin_usb.c | 59 +++++++++++++++++++++++++++--------------------
1 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c
index 0fb7252..511f2a9 100644
--- a/libusb/os/darwin_usb.c
+++ b/libusb/os/darwin_usb.c
@@ -554,9 +554,6 @@ static int darwin_get_config_descriptor(struct libusb_device *dev, uint8_t confi
*host_endian = 0;
}
- if (!priv->device)
- (*device)->Release (device);
-
return darwin_to_libusb (kresult);
}
@@ -724,6 +721,10 @@ static int process_new_device (struct libusb_context *ctx, usb_device_t **device
/* save our location, we'll need this later */
priv->location = locationID;
+ if (!priv->device) {
+ priv->device = device;
+ device = NULL;
+ }
snprintf(priv->sys_path, 28, "%08x-%03i", locationID, address);
ret = usbi_sanitize_device (dev);
@@ -742,6 +743,16 @@ static int process_new_device (struct libusb_context *ctx, usb_device_t **device
usbi_info (ctx, "found device with address %d at %s", dev->device_address, priv->sys_path);
} while (0);
+ if (device) {
+ kern_return_t kresult;
+ kresult = (*(device))->Release(device);
+ if (kresult) {
+ /* Log the fact that we had a problem closing the device */
+ usbi_err (DEVICE_CTX (dev), "Release: %s", darwin_error_str(kresult));
+ }
+ device = NULL;
+ }
+
if (need_unref)
libusb_unref_device(dev);
@@ -760,8 +771,6 @@ static int darwin_get_device_list(struct libusb_context *ctx, struct discovered_
while ((device = usb_get_next_device (deviceIterator, &location)) != NULL) {
(void) process_new_device (ctx, device, location, _discdevs);
-
- (*(device))->Release(device);
}
IOObjectRelease(deviceIterator);
@@ -776,13 +785,15 @@ static int darwin_open (struct libusb_device_handle *dev_handle) {
IOReturn kresult;
if (0 == dpriv->open_count) {
- kresult = darwin_get_device (dpriv->location, &darwin_device);
- if (kresult) {
- usbi_err (HANDLE_CTX (dev_handle), "could not find device: %s", darwin_error_str (kresult));
- return darwin_to_libusb (kresult);
- }
+ if (!dpriv->device) {
+ kresult = darwin_get_device (dpriv->location, &darwin_device);
+ if (kresult) {
+ usbi_err (HANDLE_CTX (dev_handle), "could not find device: %s", darwin_error_str (kresult));
+ return darwin_to_libusb (kresult);
+ }
- dpriv->device = darwin_device;
+ dpriv->device = darwin_device;
+ }
/* try to open the device */
kresult = (*(dpriv->device))->USBDeviceOpenSeize (dpriv->device);
@@ -797,8 +808,6 @@ static int darwin_open (struct libusb_device_handle *dev_handle) {
break;
default:
- (*(dpriv->device))->Release (dpriv->device);
- dpriv->device = NULL;
return darwin_to_libusb (kresult);
}
} else {
@@ -808,9 +817,6 @@ static int darwin_open (struct libusb_device_handle *dev_handle) {
usbi_err (HANDLE_CTX (dev_handle), "CreateDeviceAsyncEventSource: %s", darwin_error_str(kresult));
(*(dpriv->device))->USBDeviceClose (dpriv->device);
- (*(dpriv->device))->Release (dpriv->device);
-
- dpriv->device = NULL;
return darwin_to_libusb (kresult);
}
@@ -874,15 +880,6 @@ static void darwin_close (struct libusb_device_handle *dev_handle) {
usbi_err (HANDLE_CTX (dev_handle), "USBDeviceClose: %s", darwin_error_str(kresult));
}
}
-
- kresult = (*(dpriv->device))->Release(dpriv->device);
- if (kresult) {
- /* Log the fact that we had a problem closing the file, however failing a
- * close isn't really an error, so return success anyway */
- usbi_err (HANDLE_CTX (dev_handle), "Release: %s", darwin_error_str(kresult));
- }
-
- dpriv->device = NULL;
}
/* file descriptors are maintained per-instance */
@@ -1239,6 +1236,18 @@ static int darwin_detach_kernel_driver (struct libusb_device_handle *dev_handle,
}
static void darwin_destroy_device(struct libusb_device *dev) {
+ struct darwin_device_priv *dpriv = (struct darwin_device_priv *)dev->os_priv;
+ IOReturn kresult;
+
+ if (dpriv->device) {
+ kresult = (*(dpriv->device))->Release(dpriv->device);
+ if (kresult) {
+ /* Log the fact that we had a problem closing the file, however failing a
+ * close isn't really an error, so return success anyway */
+ usbi_err (DEVICE_CTX (dev), "Release: %s", darwin_error_str(kresult));
+ }
+ dpriv->device = NULL;
+ }
}
static int submit_bulk_transfer(struct usbi_transfer *itransfer) {
--
1.7.6
["0002-Remove-darwin_get_device.patch" (application/octet-stream)]
From b55f039d24ff1f557b2c601f0958e535f1782743 Mon Sep 17 00:00:00 2001
From: Vitali Lovich <vlovich@gmail.com>
Date: Fri, 16 Dec 2011 16:30:07 -0500
Subject: [PATCH 2/2] Remove darwin_get_device
It is no longer necessary
---
libusb/os/darwin_usb.c | 49 +-----------------------------------------------
1 files changed, 1 insertions(+), 48 deletions(-)
diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c
index 511f2a9..80e3853 100644
--- a/libusb/os/darwin_usb.c
+++ b/libusb/os/darwin_usb.c
@@ -204,33 +204,6 @@ static usb_device_t **usb_get_next_device (io_iterator_t deviceIterator, UInt32
return device;
}
-static kern_return_t darwin_get_device (uint32_t dev_location, usb_device_t ***darwin_device) {
- kern_return_t kresult;
- UInt32 location;
- io_iterator_t deviceIterator;
-
- kresult = usb_setup_device_iterator (&deviceIterator);
- if (kresult)
- return kresult;
-
- /* This port of libusb uses locations to keep track of devices. */
- while ((*darwin_device = usb_get_next_device (deviceIterator, &location)) != NULL) {
- if (location == dev_location)
- break;
-
- (**darwin_device)->Release(*darwin_device);
- }
-
- IOObjectRelease (deviceIterator);
-
- if (!(*darwin_device))
- return kIOReturnNoDevice;
-
- return kIOReturnSuccess;
-}
-
-
-
static void darwin_devices_detached (void *ptr, io_iterator_t rem_devices) {
struct libusb_context *ctx = (struct libusb_context *)ptr;
struct libusb_device_handle *handle;
@@ -530,17 +503,7 @@ static int darwin_get_config_descriptor(struct libusb_device *dev, uint8_t confi
if (!priv)
return LIBUSB_ERROR_OTHER;
- if (!priv->device) {
- kresult = darwin_get_device (priv->location, &device);
- if (kresult || !device) {
- usbi_err (DEVICE_CTX (dev), "could not find device: %s", darwin_error_str (kresult));
-
- return darwin_to_libusb (kresult);
- }
-
- /* don't have to open the device to get a config descriptor */
- } else
- device = priv->device;
+ device = priv->device;
kresult = (*device)->GetConfigurationDescriptorPtr (device, config_index, &desc);
if (kresult == kIOReturnSuccess) {
@@ -785,16 +748,6 @@ static int darwin_open (struct libusb_device_handle *dev_handle) {
IOReturn kresult;
if (0 == dpriv->open_count) {
- if (!dpriv->device) {
- kresult = darwin_get_device (dpriv->location, &darwin_device);
- if (kresult) {
- usbi_err (HANDLE_CTX (dev_handle), "could not find device: %s", darwin_error_str (kresult));
- return darwin_to_libusb (kresult);
- }
-
- dpriv->device = darwin_device;
- }
-
/* try to open the device */
kresult = (*(dpriv->device))->USBDeviceOpenSeize (dpriv->device);
--
1.7.6
------------------------------------------------------------------------------
Learn Windows Azure Live! Tuesday, Dec 13, 2011
Microsoft is holding a special Learn Windows Azure training event for
developers. It will provide a great way to learn Windows Azure and what it
provides. You can attend the event by watching it streamed LIVE online.
Learn more at http://p.sf.net/sfu/ms-windowsazure
_______________________________________________
Libusb-devel mailing list
Libusb-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusb-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic