search

libimobiledevice / libimobiledevice-glue

A library with common code used by libraries and tools around the libimobiledevice project
GNU Lesser General Public License v2.1
91 stars 70 forks source link

Fix select failing when the process has many file descriptors by using poll instead #26

Closed Syrco closed 9 months ago

Syrco commented 1 year ago

This happens, for example, when a process links with libusbmuxd, and the process is file descriptor heavy. Note: 'poll' means the syscall poll, not a polling approach. This patch does not add a busy wait and is not a performance regression.

nikias commented 1 year ago

Generally I like this approach. I need to make sure it works as expected, and also cross platform. I any case you will need to change the indentation to use tabs instead of spaces.

Syrco commented 1 year ago

I've fixed the indentation

Syrco commented 1 year ago

@nikias Is there anything else I can do to help? Should I squash the commits?

nikias commented 1 year ago

@Syrco no, but I noticed a small performance drop with this solution and I am trying to figure out if I can improve it.

amazingmo commented 9 months ago

Hello, It has been almost a year since this ticket was last commented on. Are there any updates? It would be good to know if the small performance issue got worked out, or even, what the magnitude of the drop was.

nikias commented 9 months ago

Couldn't get around making it faster, but it was noticeable performance drop. Need to investigate again.

nikias commented 9 months ago

The other issue is that Windows (via Winsock API) has select() but not poll(), so we need to make it use poll only on platforms that support it.

amazingmo commented 9 months ago

@Syrco are you still tracking this? Wondering if you would be prepared to make a change so that poll() over select() is a runtime selectable strategy? From what nikias said it sounds like most users would prefer it to stay the same, except for when you absolutely need more than 1024 file descriptors.

nikias commented 9 months ago

I am fine with poll() if I don't have any performance drop. Need to verify again. Other than that we can ifdef it for windows and keep using select.

nikias commented 9 months ago

Found the bottleneck which is literally just having the compiler inline the additional function, then it should be good. Working out the select/poll conditional compilation now.

nikias commented 9 months ago

Merged with bbc3c4e58f7991495e885b38a03fd20abecd4583 and 2d517ebcebe326e79186e41ee7bbd1cf5ed1f2b9.

amazingmo commented 9 months ago

Thank you.