Closed Bug 1371297 Opened 6 years ago Closed 6 years ago

stylo: We could shrink ApplicableDeclarationBlock by a word on 64-bit

Categories

(Core :: CSS Parsing and Computation, enhancement)

53 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug)

Details

ApplicableDeclarationBlock has 2 words of StyleSource, a u8 for CascadeLevel, a word for usize, and a u32 for specificity.  Apparently rust doesn't reorder struct members yet, so we end up with 5 words on both 32-bit and 64-bit.  If I move the CascadeLevel to the end, we get 32 bytes on 64-bit instead of 40.

Not sure whether it's worth doing this or just waiting for rust to start reordering things.
(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #0)
> ApplicableDeclarationBlock has 2 words of StyleSource, a u8 for
> CascadeLevel, a word for usize, and a u32 for specificity.  Apparently rust
> doesn't reorder struct members yet, so we end up with 5 words on both 32-bit
> and 64-bit.  If I move the CascadeLevel to the end, we get 32 bytes on
> 64-bit instead of 40.
> 
> Not sure whether it's worth doing this or just waiting for rust to start
> reordering things.

It is worth doing, IMO. Smaller data structures means more stuff fitting in cache, less memmov, better jemalloc performance, the list goes on.
Mind posting your patch?
Flags: needinfo?(bzbarsky)
https://github.com/servo/servo/pull/17242
Flags: needinfo?(bzbarsky)
https://hg.mozilla.org/integration/autoland/rev/16a2f50d486db9d24262b5888a4269ae81d56381
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.