Uses the u32 hole left in ApplicableDeclarationBlock.
Details
Details
- Reviewers
emilio - Group Reviewers
Restricted Project - Commits
- rMOZILLACENTRAL5d0e8b9abbf4: Bug 1886441: Part 1 - Introduce scope proximity in cascade ordering. r=firefox…
- Bugzilla Bug ID
- 1886441
Diff Detail
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Just a couple minor questions, but looks good. I have some perf concerns that would be good to address, since in the past keeping ApplicableDeclarationBlock small was key for performance.
It feels like this could use the 8 bits remaining of source_order unless I'm missing something. How big do we expect the scope proximity to get realistically? In any case I'd rather start limited and expand it than having to limit it in the future.
| servo/components/style/applicable_declarations.rs | ||
|---|---|---|
| 151 | Or just `other.0.cmp(&self.0)? | |
| 228 | Hmm, we've gone quite out of our way to avoid growing this, and this was important for performance. Have you run this through stylebench? Also, do we need the whole 32 bits in any case? Can we stash it in the remaining 8 bits of source_order? At the very least we should probably add a limit on it, we don't support DOM trees that deep). Maybe 8 or 16 bits is enough. | |
| servo/components/style/stylist.rs | ||
| 2834 | I'm confused, precomputed pseudo-elements are UA only. we can just assert that we're not on a scope rule of any sort, right? Just like we assert the layer id and origin. | |
Comment Actions
Oh, though you're right right now this doesn't grow ApplicableDeclarationBlock in 64-bit builds...
Can we:
- Add an assert of the size of ApplicableDeclarationBlock so we notice if we grow it.
- Consider limiting it regardless, to keep some bits for the next fancy CSS feature?