Cache event handlers - #868
Merged
Merged
Conversation
ben-grande
force-pushed
the
cache-event-collection
branch
from
August 11, 2026 11:53
5fd45d4 to
fec4cf1
Compare
Contributor
Author
|
PipelineRetryFailed |
ben-grande
force-pushed
the
cache-event-collection
branch
2 times, most recently
from
August 12, 2026 07:48
028eda0 to
e81fa41
Compare
Clear separation of concerns, a function that collects events can be timed separate from the execution of synchronous handlers.
ben-grande
force-pushed
the
cache-event-collection
branch
3 times, most recently
from
August 12, 2026 13:08
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.75% +0.24%
==========================================
Files 61 61
Lines 14371 14358 -13
==========================================
+ Hits 10133 10159 +26
+ Misses 4238 4199 -39
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:
|
ben-grande
marked this pull request as ready for review
August 12, 2026 13:29
Member
|
This looks okay on the first sight, but I'd like to run it though openqa, just in case. |
This was referenced Aug 19, 2026
Merged
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
|
This was referenced Aug 19, 2026
Contributor
Author
|
Not sure to which PR this is related: https://openqa.qubes-os.org/tests/192802/logfile?filename=serial_terminal.txt |
Member
This one is an outdated test, it's called I guess an issue with Fedora update... |
This was referenced Aug 21, 2026
This was referenced Aug 26, 2026
marmarek
requested changes
Aug 27, 2026
Comment on lines
+191
to
+213
| @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 |
Member
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.
Contributor
Author
There was a problem hiding this comment.
Separate caches
benchmark
class QubesCollectNewHandlers(QubesPerf):
"""
Benchmark time to collect event handlers
"""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.qube = self.app.domains["default-dvm"]
self.funcs = self.qube._get_h(
event="domain-feature-set:internal", pre_event=False
)
def test(self, previous_value=None) -> tuple[list, list]:
# pylint: disable=unused-argument
#sync_funcs, async_funcs = self.qube._get_event_funcs(
# event="domain-feature-set:internal", pre_event=False
#)
#return sync_funcs, async_funcs
self.qube._sort_handlers.cache_clear()
self.qube._get_handler_funcs.cache_clear()
for handlers in self.funcs:
sorted_handlers = self.qube._sort_handlers(handlers)
curr_sync_func, curr_async_funcs = self.qube._get_handler_funcs(
sorted_handlers
)% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000381
- Max: 0.0000381
- Min: 0.0000082
- Avg: 0.0000096
- Med: 0.0000089
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000325
- Max: 0.0000325
- Min: 0.0000073
- Avg: 0.0000092
- Med: 0.0000091
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000244
- Max: 0.0000244
- Min: 0.0000078
- Avg: 0.0000102
- Med: 0.0000101
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000586
- Max: 0.0000586
- Min: 0.0000148
- Avg: 0.0000156
- Med: 0.0000152
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000372
- Max: 0.0000398
- Min: 0.0000075
- Avg: 0.0000096
- Med: 0.0000084
Same function/cache
benchmark
class QubesCollectNewHandlers(QubesPerf):
"""
Benchmark time to collect event handlers
"""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.qube = self.app.domains["default-dvm"]
self.funcs = self.qube._get_h(
event="domain-feature-set:internal", pre_event=False
)
def test(self, previous_value=None) -> tuple[list, list]:
# pylint: disable=unused-argument
#sync_funcs, async_funcs = self.qube._get_event_funcs(
# event="domain-feature-set:internal", pre_event=False
#)
#return sync_funcs, async_funcs
#self.qube._sort_handlers.cache_clear()
#self.qube._get_handler_funcs.cache_clear()
self.qube._get_handler_funcs_and_sort.cache_clear()
for handlers in self.funcs:
#sorted_handlers = self.qube._sort_handlers(handlers)
#curr_sync_func, curr_async_funcs = self.qube._get_handler_funcs(
# sorted_handlers
#)
#sorted_handlers = self.qube._sort_handlers(handlers)
curr_sync_func, curr_async_funcs = self.qube._get_handler_funcs_and_sort(
handlers
)% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000363
- Max: 0.0000363
- Min: 0.0000082
- Avg: 0.0000103
- Med: 0.0000100
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000233
- Max: 0.0000233
- Min: 0.0000070
- Avg: 0.0000077
- Med: 0.0000075
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000505
- Max: 0.0000505
- Min: 0.0000123
- Avg: 0.0000131
- Med: 0.0000127
[user@dom0 ~]
% PYTHONPATH=qubes-core-admin ~/stats.py -r 300 -b QubesCollectNewHandlers -c 7
- 1st: 0.0000283
- Max: 0.0000283
- Min: 0.0000068
- Avg: 0.0000082
- Med: 0.0000081
Note, I reran the benchmark multiple times, as the number is too small, I noticed several times that noise affected a lot.
It might slightly help as it can be cached, but from this measurement, there is no noticeable difference, so I will merge the functions.
| @functools.cache | ||
| def _get_ordered_mro(mro, pre_event: bool) -> tuple: | ||
| order = tuple( | ||
| method for method in mro if hasattr(method, "__handlers__") |
Member
There was a problem hiding this comment.
Those aren't really methods, but classes.
This was referenced Aug 27, 2026
Merged
The type has to be changed for tuple for it to be hashable.
ben-grande
force-pushed
the
cache-event-collection
branch
from
August 31, 2026 17:03
c62604a to
db9dcac
Compare
"itertools.chain" is memory efficient but doesn't allow caching.
ben-grande
force-pushed
the
cache-event-collection
branch
from
August 31, 2026 17:12
db9dcac to
6b44b2d
Compare
Member
|
PipelineRetry |
marmarek
approved these changes
Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.