perf(schema): compute optionally_keyed_by fields once per Schema class - #1049
Merged
Merged
Conversation
Schema.__post_init__ walked every annotation of the class and called get_origin() and get_args() on each of them to find the fields using optionally_keyed_by, every time an instance was created. The answer only depends on the class, so it is now computed once per class and cached. Creating an instance of a 10-field schema with two keyed-by fields takes 4.9us instead of 8.3us. In Firefox's task graph generation (38,223 tasks, `./mach taskgraph full -j 8`), the full task set is generated ~1-2s sooner (roughly 8%).
jcristau
reviewed
Sep 23, 2026
jcristau
approved these changes
Sep 24, 2026
Comment on lines
+373
to
+374
| # Find the fields that use `optionally_keyed_by` once per class, rather | ||
| # than every time an instance is validated. |
Contributor
There was a problem hiding this comment.
This in the commit message already I don't think it is needed as a comment.
| TestSchema.validate({"field": {"by-foo": {"a": "b"}}}) | ||
|
|
||
|
|
||
| def test_optionally_keyed_by_per_class(): |
Contributor
There was a problem hiding this comment.
Might be useful to have a test with a subclass, to ensure keyed-by is found for fields defined in both?
Contributor
Author
There was a problem hiding this comment.
There was actually a bug here, so added test and fixed
marco-c
force-pushed
the
schema-cache-keyed-by-fields
branch
from
September 24, 2026 08:59
8e32f34 to
f11aab5
Compare
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
2 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.
Schema.post_init walked every annotation of the class and called get_origin() and get_args() on each of them to find the fields using optionally_keyed_by, every time an instance was created. The answer only depends on the class, so it is now computed once per class and cached.
Creating an instance of a 10-field schema with two keyed-by fields takes 4.9us instead of 8.3us. In Firefox's task graph generation (38,223 tasks,
./mach taskgraph full -j 8), the full task set is generated ~1-2s sooner (roughly 8%).