I guess I should have said something earlier, but when I first saw the CLs, I assumed that this had been discussed somewhere :)
There's going to be a bunch of new getters and setters (and Members) > showing up, but they already existed through template magic; they are > just becoming visible now and given names and types (as opposed to being > string-keyed, which risks type confusion). We expect a tiny further > decrease in binary size and no significant change in compilation time. > This isn't quite correct; supplements were never string keyed. It took me some time to track this down and verify, since the CL that changed this <https://chromium.googlesource.com/chromium/src/+/a0c6dba59914aec4a758d89508e39e0c524abad6> wasn't associated with any bug. But we don't define any specific traits for `const char*` so we just hashed the pointer value directly (though arguably this is somewhat of a footgun too :). While it's true that those getters and setters existed through template magic before, it still provided a useful abstraction to avoid embedding a bunch of arbitrary things into core classes. Now, we have 36 (!!) forward-declared members on LocalDOMWindow and another 28 on Navigator: that seems like a pretty big encapsulation failure. Are all 64 of these really performance critical? I think there's a balance to be struck here, and defaulting everything to a member of LocalDOMWindow/Document/LocalFrame/Navigator doesn't really seem like a maintainable approach, even if it's good for benchmarks. Daniel On Thu, 27 Nov 2025 at 15:40, Dave Tapuska <[email protected]> wrote: > I had the same thoughts that Jeremy had when I first saw this. And I > thought perhaps there was some intense discussion on it that I had missed. > This definitely seems like we now are just going all in on layering > violations. Chasing the highest benchmark you can get to isn't always the > best course of action. Things like this use to be discussed prior on > platform-dev. > > Dave > > > On Thu, Nov 27, 2025, 6:31 p.m. Jeremy Roman <[email protected]> wrote: > >> I guess I'm not sure I'm asking anything be done now that this is done, >> but I'm a bit surprised we unwound a longstanding technical decision so >> suddenly. >> >> On Thu, Nov 27, 2025 at 4:19 PM Steinar H. Gunderson <[email protected]> >> wrote: >> >>> On Thu, Nov 27, 2025 at 03:46:37PM -0500, Jeremy Roman wrote: >>> > Curious about the motivation here, beyond switching from a hashtable >>> to an >>> > array. Is the tracing cost that large? >>> >>> We won ~70 kB of APK size, and 2%+ style performance on some pages. The >>> tracing cost should be roughly the same. (The project was finished last >>> week, and Supplementable is gone.) >>> >>> > It seems like it forces a lot more boilerplate in very central classes >>> and >>> > may make some of them considerably larger because they need space for >>> > members, some of which are nearly always absent. >>> >>> Are you asking about the change from a hashtable to an array, or the >>> change >>> from an array to explicit members? >>> >> >> Explicit members. >> >> >>> The size (which changed from hashtable to array) diff was generally >>> small, >>> and often negative. The hash table was not free, even less so if you had >>> 1–2 members set and needed to go to a heap allocation. In any case, >>> unused >>> padding in these objects is probably much more significant if we care >>> about >>> the RAM usage in the 2–3 Document objects we have in a typical renderer >>> process. >>> >>> The boilerplate (which changed from array to explicit members) the same >>> as >>> every other getter and setter, so normal Google/Chromium/Blink style. >>> I don't think there's been a push for trying to e.g. add macros to reduce >>> the boilerplate for a typical class, or for that matter, making the >>> members >>> public? I mean, Supplementable comes purely from a desire to be able to >>> embed >>> classes across layers, right? >>> >> >> My question isn't about the particular way it's written (it's idiomatic, >> I have no doubt), just about core class headers being less readable because >> they need to contain potentially a lot (maybe it's less bad than in >> practice than I'd imagined) of code that isn't useful in understanding the >> class itself. >> >> ExecutionContext now has dozens of these simply because it's the scope of >> most "singletons" in the web that aren't specifically document-scoped. >> Several of them are even platform-specific in a class that's generally very >> platform-agnostic (for example, WebViewAndroid and CrosKiosk), as well as >> several that seem a bit mysterious without context from the module they >> belong to. >> >> > We have a similar concept outside Blink, in the form of >>> > base::SupportsUserData, for similar reasons (and we presumably wouldn't >>> > want WebContentsImpl to have members for every possible user data it >>> might >>> > have. >>> >>> There are valid reasons for having a sparse structure; in particular, >>> there are things like ElementRareData or NodeData where it genuinely >>> saves >>> memory (and has seen different implementations as we've been tuning >>> things). >>> But we have _lots_ of Elements, so the tradeoff is very different IMO. >>> >> >> content::WebContentsImpl is an example where we have fewer than we have >> blink::Elements, but certainly agreed that the tradeoffs are different. >> >> >>> /* Steinar */ >>> -- >>> Homepage: https://www.sesse.net/ >>> >> -- >> You received this message because you are subscribed to the Google Groups >> "blink-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACuR13ejLKx5hqSHVuX8CDBdzUOsGBn1v-HwGg-j%2Bw0R0JVebg%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACuR13ejLKx5hqSHVuX8CDBdzUOsGBn1v-HwGg-j%2Bw0R0JVebg%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHgVhZVfoweHqjL3bMDNvNrFOAYKBfmJTtARG2nEt9BRGG2OBQ%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHgVhZVfoweHqjL3bMDNvNrFOAYKBfmJTtARG2nEt9BRGG2OBQ%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKr%3Dyu0XG4%2B2QXyeVOUDU0_8_vq6wJzQwtcYvGEs1dzhMA%40mail.gmail.com.
