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.
