Repository navigation
Reduce overhead to add/remove asyncio readers and writers #106527
Description
Activity
Interesting. Have you measured your alternative? Is it faster? I'm a little surprised that calling
.get_map().get(key)is slower than calling.get_key(key)plus raising and catching an exception -- that would suggest a function call is faster than raising an catching an exception.Maybe we would do better trying to make exception handling faster? (I guess this code is unusual because in the common case the exception gets raised, so our "zero-overhead exceptions" don't help, since it's only zero overhead when it doesn't get raised.)
@kumaraditya303 Thoughts? The PR seems well thought out.
Have you measured your alternative? Is it faster?
The flames disappear from the
py-spyafter the change. I also did a cProfile and its faster. This isn't a great comparison though as its doesn't quantify the actual performance improvement and only shows more of the real world impact. I'll see if I can put together something that benchmarks it
original: 1.048221042030491 new: 0.626125167007558I think this is close enough to benchmark the change:
import asyncio import timeit loop = asyncio.get_event_loop() original_code = """ try: key = selector.get_key(1) except KeyError: pass """ new_code = """ key = selector.get_map().get(1) if key is None: pass """ original_time = timeit.timeit( original_code, number=1000000, globals={"selector": loop._selector}, ) new_time = timeit.timeit( new_code, number=1000000, globals={"selector": loop._selector}, ) print("original: %s" % original_time) print("new: %s" % new_time)
Yeah, that’s acceptable.
Reacted by J. Nick Koston_SelectorMapping.__getitem__has the same pattern
probably something like this would offer the same speed up
Line 73 in da98ed0
def __getitem__(self, fileobj): fd = self._selector._fileobj_lookup(fileobj) key = self._selector._fd_to_key.get(fd) if key is None: raise KeyError("{!r} is not registered".format(fileobj)) return key
I'll wait for the review of the linked PR and pending the outcome there will open another PR for this. In the mean time I'll push the above code to production and run it for a bit.
That helps a bit as it avoids one except catch but on the add_reader case the last KeyError has to raise
It also looks like the selectors could remove
_key_from_fdand replace it with a simpleself._fd_to_key.get()unless_key_from_fdis a defined API promise. That's roughly a 10% reduction of run time of the code inselectors.pywhen running an asyncio event loop with ~10000 calls toselectper minute since it doesn't have the extra function depth overhead.That should also likely be another PR. Probably worth doing since its called in the
selectloop and this was on a mostly idle system.@@ -332,7 +332,7 @@ def select(self, timeout=None): if fd in w: events |= EVENT_WRITE - key = self._key_from_fd(fd) + key = self._fd_to_key.get(fd) if key: ready.append((key, events & key.events)) return ready @@ -422,7 +422,7 @@ def select(self, timeout=None): if event & ~self._EVENT_WRITE: events |= EVENT_READ - key = self._key_from_fd(fd) + key = self._fd_to_key.get(fd) if key: ready.append((key, events & key.events)) return ready @@ -475,7 +475,7 @@ def select(self, timeout=None): if event & ~select.EPOLLOUT: events |= EVENT_READ - key = self._key_from_fd(fd) + key = self._fd_to_key.get(fd) if key: ready.append((key, events & key.events)) return ready @@ -570,7 +570,7 @@ def select(self, timeout=None): if flag == select.KQ_FILTER_WRITE: events |= EVENT_WRITE - key = self._key_from_fd(fd) + key = self._fd_to_key.get(fd) if key: ready.append((key, events & key.events)) return ready
rough profile (different points in time with similar workloads)
before:
after
Maybe we would do better trying to make exception handling faster? (I guess this code is unusual because in the common case the exception gets raised, so our "zero-overhead exceptions" don't help, since it's only zero overhead when it doesn't get raised.)
I think that would also be worthwhile, FWIW. I was quite surprised how much switching to LBYL idioms sped things up in #103318, largely because of how slow exception handling is.
(N.B. I don't think longterm goals like that should impact whether this proposed change is accepted or not.)
FWIW I think it is much better to optimize selectors module rather than
asyncioin this case.FWIW I think it is much better to optimize selectors module rather than
asyncioin this case.I agree that selectors are the primary source of the bottleneck I'm seeing. (nothing else is called 100000x per minute on busy systems besides
_run_once)More discussion on that in #106555
Thanks for working on this.
Reacted by J. Nick Koston
Pitch
The current code path for adding and removing asyncio readers and writers always has to do a
tryexcept KeyErrorto add a new reader/writercpython/Lib/asyncio/selector_events.py
Line 316 in b3648f0
For use cases where readers are added and removed frequently (hard to change the design without breaking changes) this adds up quickly.
Linked PRs