[issue30014] Speedup DefaultSelectors.modify() by 2x

Andrew Svetlov added the comment: PR was merged 6 months ago, closing the issue. -- nosy: +asvetlov resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue30014] Speedup DefaultSelectors.modify() by 2x

STINNER Victor added the comment: New changeset fbfaa6fd57f8dc8a3da808acb8422370fad2f27b by Victor Stinner (Giampaolo Rodola) in branch 'master': bpo-30014: make poll-like selector's modify() method faster (#1030)

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: OK, https://github.com/python/cpython/pull/1030 should be good to go. -- ___ Python tracker ___

[issue30014] Speedup DefaultSelectors.modify() by 2x

STINNER Victor added the comment: I also like the plan starting with refactoring ;-) -- ___ Python tracker ___

[issue30014] Speedup DefaultSelectors.modify() by 2x

Charles-François Natali added the comment: The rationale for rejecting wouldn't be "DRY does not apply in this case", it would be that this makes the code more complicated, and that a negligible speedup would not be worth it. Now, thanks to your benchmark, a 10% speedup is not negligible, so

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: For the sake of experiment I'm attaching a toy echo server which uses modify() to switch between EVENT_READ and EVENT_WRITE. Without patch I get 35000 req/sec, with patch around 39000 req/sec (11.4% faster). To be entirely honest a smarter echo server would

[issue30014] Speedup DefaultSelectors.modify() by 2x

Changes by Giampaolo Rodola' : Added file: http://bugs.python.org/file46793/echo_client.py ___ Python tracker ___

[issue30014] Speedup DefaultSelectors.modify() by 2x

Charles-François Natali added the comment: This refactoring was already suggested a long time ago, and at the time both Guido and I didn't like it because it makes the code actually more complicated: DRY in this case doesn't apply IMO. Also, this whole thread is a repeat of:

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: @neologix: here's a PR which refactors the poll-related classes: https://github.com/python/cpython/pull/1035/files >From there, we'll be able to avoid modify() code duplication. -- ___ Python tracker

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: In certain protocols modify() is supposed to be used on every interaction between client and server. E.g. an FTP server does this: - register(fd, EVENT_READ); recv() # wait command from client - modify(fd, EVENT_WRITE); send(response) # send

[issue30014] Speedup DefaultSelectors.modify() by 2x

Charles-François Natali added the comment: > I don't think that selector.modify() can be a bottleneck, but IMHO the change > is simple and safe enough to be worth it. In a network server with 10k > client, an optimization making .modify() 1.52x faster is welcomed. IMHO it complicates the code

[issue30014] Speedup DefaultSelectors.modify() by 2x

STINNER Victor added the comment: Giampaolo Rodola': "Your old patch uses unregister() and register() so I'm not sure what speedup that'll take as the whole point is to avoid doing that." My patch calls generic unregister() and register() of the base classes, but it only uses one syscall per

[issue30014] Speedup DefaultSelectors.modify() by 2x

STINNER Victor added the comment: @neologix: Do you have a GitHub account? It's hard to me to see if https://github.com/neologix is you or not, and your GitHub username is not filled in the Bug Tracker database. -- ___ Python tracker

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: PR: https://github.com/python/cpython/pull/1030 -- pull_requests: +1187 ___ Python tracker ___

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: Hey Stinner, thanks for chiming in! Your old patch uses unregister() and register() so I'm not sure what speedup that'll take as the whole point is to avoid doing that. You may want to rewrite it and benchmark it but it looks like it'll be slower.

[issue30014] Speedup DefaultSelectors.modify() by 2x

STINNER Victor added the comment: @Giampaolo Rodola: CPython development moved to GitHub, can you please create a pull request instead of a patch? Thank you. https://docs.python.org/devguide/pullrequest.html Hum, I see that my old patch of issue #18932 (selectors_optimize_modify-2.patch) is

[issue30014] Speedup DefaultSelectors.modify() by 2x

STINNER Victor added the comment: Hi Giampaolo Rodola'! It seems like you proposed the same idea 4 years ago and I wrote a similar patch: issue #18932 :-) I suggest you to use my perf module to produce more reliable benchmarks. Here is my results on my computer smithers tuned for benchmarks:

[issue30014] Speedup DefaultSelectors.modify() by 2x

Giampaolo Rodola' added the comment: modify() can be used often in verbose protocols such as FTP and SMTP where a lot of requests and responses are continuously exchanged between client and server, so you need to switch from EVENT_READ to EVENT_WRITE all the time. I did a similar change some

[issue30014] Speedup DefaultSelectors.modify() by 2x

Charles-François Natali added the comment: Hm, do you have a realistic benchmark which would show the benefit? Because this is really a micro-benchmark, and I'm not convinced that Selector.modify() is a significant bottleneck in a real-world application. --

[issue30014] Speedup DefaultSelectors.modify() by 2x

Changes by Giampaolo Rodola' : -- components: +Library (Lib), asyncio stage: -> patch review type: -> performance ___ Python tracker

[issue30014] Speedup DefaultSelectors.modify() by 2x

Changes by Giampaolo Rodola' : -- nosy: +gvanrossum, haypo, neologix, yselivanov ___ Python tracker ___

[issue30014] Speedup DefaultSelectors.modify() by 2x

Changes by Giampaolo Rodola' : -- versions: +Python 3.7 Added file: http://bugs.python.org/file46788/bench.py ___ Python tracker ___

[issue30014] Speedup DefaultSelectors.modify() by 2x

New submission from Giampaolo Rodola': Patch in attachment modifies DefaultSelector.modify() so that it uses the underlying selector's modify() method instead of unregister() and register() resulting in a 2x speedup. Without patch: ~/svn/cpython {master}$ ./python bench.py