Cache event handlers - #868
Conversation
5fd45d4 to
fec4cf1
Compare
|
PipelineRetryFailed |
028eda0 to
e81fa41
Compare
Clear separation of concerns, a function that collects events can be timed separate from the execution of synchronous handlers.
12d694d to
eccd2dc
Compare
The type has to be changed for tuple for it to be hashable.
eccd2dc to
f865e54
Compare
"itertools.chain" is memory efficient but doesn't allow caching.
f865e54 to
c62604a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #868 +/- ##
=======================================
Coverage 70.51% 70.51%
=======================================
Files 61 61
Lines 14371 14394 +23
=======================================
+ Hits 10133 10150 +17
- Misses 4238 4244 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This looks okay on the first sight, but I'd like to run it though openqa, just in case. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026082719-devel&flavor=pull-requests Test run included the following:
Upload failures
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026050504-devel&flavor=update
Failed tests10 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/176874#dependencies 37 fixed
Unstable testsDetails
Performance TestsPerformance degradation:20 performance degradations
Remaining performance tests:91 tests
|
|
Not sure to which PR this is related: https://openqa.qubes-os.org/tests/192802/logfile?filename=serial_terminal.txt |
This one is an outdated test, it's called I guess an issue with Fedora update... |
| @staticmethod | ||
| @functools.cache | ||
| def _sort_handlers(handlers): | ||
| handlers = tuple( | ||
| sorted( | ||
| handlers, | ||
| key=(lambda handler: hasattr(handler, "ha_bound")), | ||
| reverse=True, | ||
| ) | ||
| ) | ||
| return handlers | ||
|
|
||
| @staticmethod | ||
| @functools.cache | ||
| def _get_handler_funcs(sorted_handlers) -> tuple[list, list]: | ||
| sync_funcs = [] | ||
| async_funcs = [] | ||
| for func in sorted_handlers: | ||
| if asyncio.iscoroutinefunction(func): | ||
| async_funcs.append(func) | ||
| else: | ||
| sync_funcs.append(func) | ||
| return sync_funcs, async_funcs |
There was a problem hiding this comment.
Those two are always used together, do they really need to be separate functions? Merging them could avoid creating yet another intermediate tuple.
| @functools.cache | ||
| def _get_ordered_mro(mro, pre_event: bool) -> tuple: | ||
| order = tuple( | ||
| method for method in mro if hasattr(method, "__handlers__") |
There was a problem hiding this comment.
Those aren't really methods, but classes.
3 participants
Speed up collecting event handlers.
Without the changes:
% ~/stats.py -r 30 -b QubesCollectOldHandlers
['0.000558', '0.000465', '0.000459', '0.000391', '0.000374', '0.000432', '0.000420', '0.000435', '0.000382', '0.000373', '0.000426', '0.000411', '0.000448', '0.000414', '0.000375', '0.000432', '0.000407', '0.000420', '0.000426', '0.000399', '0.000460', '0.000424', '0.000416', '0.000379', '0.000393', '0.000387', '0.000434', '0.000428', '0.000375', '0.000384']
With the changes:
% PYTHONPATH=. ~/stats.py -r 30 -b QubesCollectNewHandlers
['0.000255', '0.000075', '0.000045', '0.000042', '0.000041', '0.000040', '0.000040', '0.000040', '0.000040', '0.000385', '0.000043', '0.000041', '0.000041', '0.000040', '0.000040', '0.000049', '0.000042', '0.000046', '0.000041', '0.000041', '0.000040', '0.000040', '0.000040', '0.000040', '0.000049', '0.000040', '0.000040', '0.000040', '0.000040', '0.000045']
Didn't run tests, so it is draft.
To benchmark, I used: https://github.com/ben-grande/bench, with something similar to:
Note that the old code does not have
_get_event_funcs, because it runs the synchornous funcs in_fire_event, I had to copy from one function to the other, and remove the event handling from_get_event_funcs.