Allow some AttrValue variants to be serialized lazily#365
Merged
Conversation
Collaborator
|
What's the rationale for not also implementing this for |
Member
Author
Two reasons:
|
mrobinson
force-pushed
the
enable-lazy-serialization
branch
from
a1679eb to
616a2ed
Compare
SimonSapin
reviewed
May 18, 2026
|
|
||
| impl<T: MallocSizeOf> MallocSizeOf for std::sync::OnceLock<T> { | ||
| fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { | ||
| self.get().size_of(ops) |
Member
There was a problem hiding this comment.
I think this might call <Option<&T>>::size_of which is always zero because of the &, but it should optionally call T::size_of which may not be zero
Member
Author
There was a problem hiding this comment.
I've addressed this in the latest push.
mrobinson
force-pushed
the
enable-lazy-serialization
branch
from
616a2ed to
0e116a4
Compare
SimonSapin
approved these changes
May 18, 2026
This allows some `AttrValue` variants, the ones that are easily serializable and are sometimes set without existing serializations, to be lazily serialized. The goal here is to allow storing these values, but only serializing when necessary. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson
force-pushed
the
enable-lazy-serialization
branch
from
0e116a4 to
acf239d
Compare
pull Bot
pushed a commit
to A-Archives-and-Forks/servo
that referenced
this pull request
May 19, 2026
…ervo#44931) This change allows creating an `AttrValue` without a serialization. This is *particularly* important when setting the `style` attribute on elements, as many pages set the `style` attribute but never read it. Eagerly serializing the attribute in these cases is pure overhead. This kind of thing shows up prominently in profiles. This change starts adapting Servo to Stylo's support for lazy serialization of some attributes. In addition, some eager serialization during attribute mutations is avoided as well. Followup changes will: - Make it more obvious when a potentially expensive serialization of an attribute is performed. - Make it clear when an attribute is constructed with a pre-existing serialization. Tthese changes lead to a 25% speedup when running `tests/blink_perf_tests/perf_tests/layout/flexbox-deeply-nested-column-flow.html` on my system. Stylo PR: servo/stylo#365 Testing: This should not change observable behavior and unfortunately we do not have automated performance tests. Fixes: servo#17399. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
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.
This allows some
AttrValuevariants, the ones that are easilyserializable and are sometimes set without existing serializations, to
be lazily serialized. The goal here is to allow storing these values,
but only serializing when necessary.
Servo PR: servo/servo#44931