Open Bug 1502049 (small-dl) Opened Last year Updated 7 months ago

[meta] Optimize display item memory alignment and padding

Categories

(Core :: Web Painting, enhancement, P2)

enhancement

Tracking

()

People

(Reporter: miko, Unassigned)

References

(Depends on 4 open bugs, Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(2 files)

Since memory for display items is allocated with the nearest power of two size, it is possible for a single new member variable in a display item to double the memory used for all instances of that class.

This has potentially huge performance benefits and supports MemShrink project.
(In reply to Miko Mynttinen [:miko] from comment #0)
> Since memory for display items is allocated with the nearest power of two
> size, it is possible for a single new member variable in a display item to
> double the memory used for all instances of that class.

Nit: power-of-two rounding takes place at 512 bytes and above.  (Page aligning kicks in for page-sized things, but ideally we don't have display items that large!)  Smaller quantities are rounded to 16-byte boundaries.

> This has potentially huge performance benefits and supports MemShrink
> project.

Size improvements are always welcome!
I ran comparison for sizeof and the allocator bucket size for the types in nsDisplayList.h:

nsDisplayItem: size: 160, bucketSize: 256, waste: 96
nsCharClipDisplayItem: size: 176, bucketSize: 256, waste: 80
nsDisplayBackgroundColor: size: 224, bucketSize: 256, waste: 32
nsDisplayBackgroundImage: size: 264, bucketSize: 512, waste: 248
nsDisplayBlendContainer: size: 288, bucketSize: 512, waste: 224
nsDisplayBlendMode: size: 288, bucketSize: 512, waste: 224
nsDisplayBorder: size: 176, bucketSize: 256, waste: 80
nsDisplayBoxShadowInner: size: 184, bucketSize: 256, waste: 72
nsDisplayBoxShadowOuter: size: 208, bucketSize: 256, waste: 48
nsDisplayCaret: size: 184, bucketSize: 256, waste: 72
nsDisplayClearBackground: size: 160, bucketSize: 256, waste: 96
nsDisplayCompositorHitTestInfo: size: 216, bucketSize: 256, waste: 40
nsDisplayEffectsBase: size: 304, bucketSize: 512, waste: 208
nsDisplayEventReceiver: size: 160, bucketSize: 256, waste: 96
nsDisplayFilters: size: 320, bucketSize: 512, waste: 192
nsDisplayFixedPosition: size: 376, bucketSize: 512, waste: 136
nsDisplayForeignObject: size: 288, bucketSize: 512, waste: 224
nsDisplayGeneric: size: 192, bucketSize: 256, waste: 64
nsDisplayImageContainer: size: 160, bucketSize: 256, waste: 96
nsDisplayMasksAndClipPaths: size: 312, bucketSize: 512, waste: 200
nsDisplayOpacity: size: 296, bucketSize: 512, waste: 216
nsDisplayOutline: size: 160, bucketSize: 256, waste: 96
nsDisplayOwnLayer: size: 352, bucketSize: 512, waste: 160
nsDisplayPerspective: size: 448, bucketSize: 512, waste: 64
nsDisplayResolution: size: 376, bucketSize: 512, waste: 136
nsDisplaySVGWrapper: size: 288, bucketSize: 512, waste: 224
nsDisplayScrollInfoLayer: size: 312, bucketSize: 512, waste: 200
nsDisplaySolidColor: size: 192, bucketSize: 256, waste: 64
nsDisplaySolidColorBase: size: 168, bucketSize: 256, waste: 88
nsDisplaySolidColorRegion: size: 200, bucketSize: 256, waste: 56
nsDisplayStickyPosition: size: 360, bucketSize: 512, waste: 152
nsDisplaySubDocument: size: 376, bucketSize: 512, waste: 136
nsDisplayTableBackgroundColor: size: 240, bucketSize: 256, waste: 16
nsDisplayTableBackgroundImage: size: 280, bucketSize: 512, waste: 232
nsDisplayTableBlendContainer: size: 304, bucketSize: 512, waste: 208
nsDisplayTableBlendMode: size: 304, bucketSize: 512, waste: 208
nsDisplayTableFixedPosition: size: 392, bucketSize: 512, waste: 120
nsDisplayTableThemedBackground: size: 216, bucketSize: 256, waste: 40
nsDisplayThemedBackground: size: 200, bucketSize: 256, waste: 56
nsDisplayTransform: size: 728, bucketSize: 1024, waste: 296
nsDisplayWrapList: size: 288, bucketSize: 512, waste: 224
nsDisplayZoom: size: 384, bucketSize: 512, waste: 128
Depends on: 1526941
Status: ASSIGNED → NEW
Depends on: 1540785
Depends on: 1541426
Attached file 1502049.patch
Alias: small-dl
Summary: Optimize display item memory alignment and padding → [meta] Optimize display item memory alignment and padding
Depends on: 1542913
Depends on: 1542914
Depends on: 1542923
Depends on: 1542929

I did a hacky test (removing FLB specific members) to reduce the size of nsDisplayBackgroundColor < 128 bytes so that it went down to the smaller bucket, and ran displaylist_mutate with WebRender.

Display list building time, and total main thread time went down by around 30%. That's a pretty massive change, so I think it's well worth pushing on this metabug as much as possible.

Attached file bg-color-layout.txt

I found this command, to dump the layout of structs. Attached is the layout for nsDisplayBackgroundColor (with my hacky patch), showing it with a sizeof of 120.

clang++ -cc1 -fdump-record-layouts -I objdir-opt/dist/include -I nsprpub/pr/include -std=c++14 -stdlib=libc++ -I config/external/nspr -I nsprpub/lib/libc/include/ layout/painting/nsDisplayList.cpp -D PR_BYTES_PER_BYTE=1 -D PR_BYTES_PER_SHORT=2 -D PR_BYTES_PER_INT=4 -D PR_BYTES_PER_LONG=8 -D XP_DARWIN -D MOZILLA_INTERNAL_API -D HAVE_64BIT_BUILD -I objdir-opt/ipc/ipdl/_ipdlheaders/ -I ipc/chromium/src/ -I layout/generic/ -I layout/style -I dom/base -I layout/xul -I layout/svg -I layout/tables/

I think it might be feasible to eventually get to a state where background color is less than 64 bytes.

My theoretical layout is:

0 - vptr
8 - mAbove
16 - uint32 type, index (per-frame-key) and flags
20 - uint32 old-list-index
24 - mFrame
32 - mClipChain index uint32
36 - mASR index uint32
40 - old-list-32bit-ident
44 - mRect
60 - nscolor bg color

Size = 64, rounds to 64, buckets to 64

That's stripping everything that WebRender doesn't need, and tighly packing the rest. It also switches away from pointers, and uses 32bit indices where possible.

It also seems possible to drop the vptr, and hand-roll virtual functions using the type, to save a further 8 bytes, though that's very hard change to write.

There's also 8 bytes of temporary RDL state there (old-list-index and old-list-32bit-ident), which would be nice to avoid as part of the permanent item size. I'll think about that some more.

Assignee: mikokm → nobody
Depends on: 1544583
You need to log in before you can comment on or make changes to this bug.