gh-libusb/hidapi
Watch
1
0
Fork
You've already forked hidapi
 0
Code Issues 58 Pull requests 18 Releases 13 Wiki Activity

hidapi/libusb: maintain in-memory cache of vendor/product strings #571

Open
slouken wants to merge 2 commits from slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch into master
pull from: slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch
merge into: gh-libusb:master
Conversation 13 Commits 2 Files changed 1 +122 -4
slouken commented 2023-05-26 11:02:32 -05:00 (Migrated from github.com)

The get_usb_string call is rather expensive on some USB devices, so we cache the vendor/product strings for future lookups (e.g. when hid_enumerate is invoked again later).

This way, we only need to ask libusb for strings for devices we haven't seen since before we started.

This is an important performance improvement when frequently iterating devices using libusb.

The get_usb_string call is rather expensive on some USB devices, so we cache the vendor/product strings for future lookups (e.g. when hid_enumerate is invoked again later). This way, we only need to ask libusb for strings for devices we haven't seen since before we started. This is an important performance improvement when frequently iterating devices using libusb.
Youw commented 2023-06-01 10:03:04 -05:00 (Migrated from github.com)

What about serial_number? Shouldn't it be cached as well?

What about serial_number? Shouldn't it be cached as well?
Youw commented 2023-06-01 10:05:56 -05:00 (Migrated from github.com)

And the caching probably needs to be device-specific, not vid/pid-specific. I have two USB/HID devices on my table right now: both have the same vid/pid but different Manufacturer and Product Name (FW is localized for different countries).

And the caching probably needs to be device-specific, not vid/pid-specific. I have two USB/HID devices on my table right now: both have the same vid/pid but different Manufacturer and Product Name (FW is localized for different countries).
👍 1
Youw commented 2023-06-01 10:09:04 -05:00 (Migrated from github.com)

I also think none of it would be required when https://github.com/libusb/libusb/pull/1258 is completed.

I also think none of it would be required when https://github.com/libusb/libusb/pull/1258 is completed.
👍 1
slouken commented 2023-06-01 10:35:28 -05:00 (Migrated from github.com)

And the caching probably needs to be device-specific, not vid/pid-specific. I have two USB/HID devices on my table right now: both have the same vid/pid but different Manufacturer and Product Name (FW is localized for different countries).

Good point.

> And the caching probably needs to be device-specific, not vid/pid-specific. I have two USB/HID devices on my table right now: both have the same vid/pid but different Manufacturer and Product Name (FW is localized for different countries). Good point.
mcuee commented 2024-03-05 23:10:50 -06:00 (Migrated from github.com)

Unfortunately https://github.com/libusb/libusb/pull/1258 is closed without being merged, due to some API discussions.

Unfortunately https://github.com/libusb/libusb/pull/1258 is closed without being merged, due to some API discussions. * https://github.com/libusb/libusb/pull/1258
Youw commented 2024-03-06 05:48:21 -06:00 (Migrated from github.com)

But the idea to get is done is still alive.
It is just a matter of getting it done.
I'd rather spend time making that than improving the change in this PR.

But the idea to get is done is still alive. It is just a matter of getting it done. I'd rather spend time making that than improving the change in this PR.
👍 1
mcuee commented 2024-03-06 06:03:58 -06:00 (Migrated from github.com)

But the idea to get is done is still alive. It is just a matter of getting it done. I'd rather spend time making that than improving the change in this PR.

Good point. I agree with you here.

In that case, do you want to close this PR as not planned?

> But the idea to get is done is still alive. It is just a matter of getting it done. I'd rather spend time making that than improving the change in this PR. Good point. I agree with you here. In that case, do you want to close this PR as not planned?
Youw commented 2024-03-06 07:02:49 -06:00 (Migrated from github.com)

I don't have a strong oppinion.
It doesn't bother me having it open.

I don't have a strong oppinion. It doesn't bother me having it open.
👍 1
mcuee commented 2026-05-20 22:34:18 -05:00 (Migrated from github.com)
* https://github.com/libusb/libusb/pull/1532 has been merged in libusb.
slouken commented 2026-05-21 09:08:35 -05:00 (Migrated from github.com)

I was looking at this patch again, and you're right, it should be device specific. We can add the path to the hash key, but that isn't 100% guaranteed to be unique if you unplug and plug a different device with the same vid/pid to the same port. Is there another way to guarantee that it's device specific?

I was looking at this patch again, and you're right, it should be device specific. We can add the path to the hash key, but that isn't 100% guaranteed to be unique if you unplug and plug a different device with the same vid/pid to the same port. Is there another way to guarantee that it's device specific?
Youw commented 2026-05-22 09:12:31 -05:00 (Migrated from github.com)

I believe it could be at most platform-specific guarantee. On Windows devices with different serial numbers would have different paths even when connected to the same port, but on linux/libusb I believe it will be the same (by default) unless we add explicit additiona logic to distinguish devices (e.g. by serial number).

But I don't believe any of that is needed. I suggest abandining this PR/approach, and instead of caching anything on HIDAPI side - instead rely on OS caching, as implemented by libusb: libusb_get_device_string.

That implementation should probably be optional (in case HIDAPI is attempted to compile/link/run against older version of libusb, where this is not available), but totally covers what this PR suggests (at least logically).

I believe it could be at most platform-specific guarantee. On Windows devices with different serial numbers would have different paths even when connected to the same port, but on linux/libusb I believe it will be the same (by default) unless we add explicit additiona logic to distinguish devices (e.g. by serial number). But I don't believe any of that is needed. I suggest abandining this PR/approach, and instead of caching anything on HIDAPI side - instead rely on OS caching, as implemented by libusb: [`libusb_get_device_string`](https://github.com/libusb/libusb/pull/1532). That implementation should probably be optional (in case HIDAPI is attempted to compile/link/run against older version of libusb, where this is not available), but totally covers what this PR suggests (at least logically).
slouken commented 2026-05-22 09:24:15 -05:00 (Migrated from github.com)

I believe it could be at most platform-specific guarantee. On Windows devices with different serial numbers would have different paths even when connected to the same port, but on linux/libusb I believe it will be the same (by default) unless we add explicit additiona logic to distinguish devices (e.g. by serial number).

But I don't believe any of that is needed. I suggest abandining this PR/approach, and instead of caching anything on HIDAPI side - instead rely on OS caching, as implemented by libusb: libusb_get_device_string.

That implementation should probably be optional (in case HIDAPI is attempted to compile/link/run against older version of libusb, where this is not available), but totally covers what this PR suggests (at least logically).

I don't believe the OS implements caching, which is the problem. Also note this comment on the PR: "While the read can be fast, it can also take seconds."

> I believe it could be at most platform-specific guarantee. On Windows devices with different serial numbers would have different paths even when connected to the same port, but on linux/libusb I believe it will be the same (by default) unless we add explicit additiona logic to distinguish devices (e.g. by serial number). > > But I don't believe any of that is needed. I suggest abandining this PR/approach, and instead of caching anything on HIDAPI side - instead rely on OS caching, as implemented by libusb: [`libusb_get_device_string`](https://github.com/libusb/libusb/pull/1532). > > That implementation should probably be optional (in case HIDAPI is attempted to compile/link/run against older version of libusb, where this is not available), but totally covers what this PR suggests (at least logically). I don't believe the OS implements caching, which is the problem. Also note this comment on the PR: "While the read can be fast, it can also take seconds."
Youw commented 2026-05-22 09:55:19 -05:00 (Migrated from github.com)

Also note this comment on the PR: "While the read can be fast, it can also take seconds."

On one Ubuntu Linux machine, reading the /sys/bus/usb/ files to get the strings takes a variable amount of time **on one device, an xHCI root hub**. While the read can be fast, it can also take seconds.

HIDAPI doesn't try to open/read HUBs, that is not an issue.

I don't believe the OS implements caching

That's the thing - it does. That's one of the reasons why this API is allowed and works w/o actually opening the device (which requires elevated permissions sometime) - it only reads the information available from USB descriptor which automatically opened and parsed by the OS once, when the device just been attached (and even before it appeared as an available USB device thru OS API).

> Also note this comment on the PR: "While the read can be fast, it can also take seconds." `On one Ubuntu Linux machine, reading the /sys/bus/usb/ files to get the strings takes a variable amount of time **on one device, an xHCI root hub**. While the read can be fast, it can also take seconds.` HIDAPI doesn't try to open/read HUBs, that is not an issue. > I don't believe the OS implements caching That's the thing - it does. That's one of the reasons why this API is allowed and works w/o actually opening the device (which requires elevated permissions sometime) - it only _reads_ the information available from USB descriptor which automatically opened and parsed by the OS once, when the device just been attached (and even before it appeared as an available USB device thru OS API).
This pull request has changes conflicting with the target branch.
  • libusb/hid.c
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch:slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git switch 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'master'
git merge --no-ff 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git switch 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git rebase 'master'
git switch 'master'
git merge --ff-only 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git switch 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git rebase 'master'
git switch 'master'
git merge --no-ff 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git switch 'master'
git merge --squash 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git switch 'master'
git merge --ff-only 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git switch 'master'
git merge 'slouken/0001-hidapi-libusb-maintain-in-memory-cache-of-vendor-pro.patch'
git push origin 'master'
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
3rdparty
3rd-party related issue/question

  
Android
Android OS related

  
API
API change, Version 1 stuff

  
Author does not respond

   
bsd
FreeBSD, NetBSD, OpenBSD, etc

  
bug
Something isn't working

  
build system/CI
Anything related to building the project or running on CI

  
ci-virtual-device
Run the virtual HID device CI jobs (Windows driver + libusb raw-gadget)

  
CMake
Related to CMake build system

  
Contributions Welcome
Any external help is welcome

  
Core
Related to common codes like hidapi.h

  
dependencies
Pull requests that update a dependency file

  
device-specific
Issues/PRs related to a specific device

  
documentation
Improvements or additions to documentation

  
don't_merge
Don't merge this PR as is

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
Example
Related to hidtest or new examples

  
Haiku
Related to Haiku OS (using libusb backend)

  
hidraw
Related to Linux/hidraw backend

  
hotplug
Related to hotplug

  
info required
Need more input from PR/issue author

  
invalid
Not a HIDAPI issue

  
libusb
Related to libusb backend

  
macOS
Related to macOS backend

  
Misc
Typo, whitespace, etc.

  
Not an issue

   
question
Information is requested

  
Release
Related to new release

  
Test
Related to built-in test (not hidtest)

  
Test GUI
Related to a Test GUI application

  
Windows
Related to Windows backend

  
wontfix
This will not be worked on

No labels
3rdparty
Android
API
Author does not respond
bsd
bug
build system/CI
ci-virtual-device
CMake
Contributions Welcome
Core
dependencies
device-specific
documentation
don't_merge
duplicate
enhancement
Example
Haiku
hidraw
hotplug
info required
invalid
libusb
macOS
Misc
Not an issue
question
Release
Test
Test GUI
Windows
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libusb/hidapi!571
No description provided.