The context snapshot is the default behavior that we ship and we should test. If there's issues with tests, we should just fix them, like anything else.
Using the snapshot exercises very different paths for something like e.g. bindings. IMHO, a removal should be treated like any other bigger removals: Someone comes up with a proposal and a Finch trial + reasons why it's not necessary anymore. Context snapshots are a performance feature in that they eagerly create objects that we then later need and as such benefit startup. -Michael On Tue, Jul 16, 2024 at 8:37 AM Daniel Cheng <[email protected]> wrote: > I'm guessing the root cause is this: > > --- > /b/s/w/ioajsmeisv/layout-test-results/fast/dom/Window/window-constructor-expected.txt > +++ > /b/s/w/ioajsmeisv/layout-test-results/fast/dom/Window/window-constructor-actual.txt > @@ -0,0 +1,5 @@ > +This is a testharness.js-based test. > +[FAIL] Test Window and its prototype chain's constructors. > + assert_equals: WindowProperties constructor is EventTarget. expected > function "function EventTarget() { [native code] }" but got function > "function WindowProperties() { [native code] }" > +Harness: the test ran to completion. > + > > Though *why* it's failing, I don't know off hand and will need to dig into > it. It does seem worth fixing that. > > Daniel > > On Mon, 15 Jul 2024 at 23:29, Leszek Swirski <[email protected]> wrote: > >> I'm not saying that it's subtle and hard to get right, any more so than >> any other code we ship at least, I'm saying that it's a full separate code >> path and behaviour that we would be shipping to users without running >> locally. The difficulties I'm having on android are because of build >> config/dependency issues. >> >> For "how much performance", I'm currently trying to enable it on Android >> because it would improve Search page load by what they estimate to be ~24 >> SWE-years worth of improvement (Kentaro has a calculation in this doc >> <https://docs.google.com/document/d/1QwzaNvSwkXeR2_Du5SnXNKPqA3ZTrh8bBK0kbC5T4BI/edit?tab=t.0>). >> Also +Kouhei Ueno <[email protected]> who has been looking into the >> impact of context setup on page load, and +Camillo Bruni >> <[email protected]> who looked at the performance with Scott. >> >> On Tue, 16 Jul 2024, 02:43 Nico Weber, <[email protected]> wrote: >> >>> If the code path behind use_v8_context_snapshot is subtle and hard to >>> get right, it sounds like there's our second reason for not having it, >>> right there :) >>> >>> How much performance does the blink snapshot buy us? Is it possible to >>> get some of that with less expensive techniques? >>> >>> (+sky who I think looked at this at some point.) >>> >>> On Monday, July 15, 2024 at 2:54:35 PM UTC-4 Leszek Swirski wrote: >>> >>>> +Michael Lippautz <[email protected]> >>>> >>>> I'm not sure we should do this for build time reasons alone. >>>> `use_v8_context_snapshot` is the default behaviour in shipping browsers, >>>> and it makes the renderer take quite a different path during page load / >>>> navigation. If we disable it in regular builds, then regular builds >>>> (including all the dchecks!) won't be running and texting the same code >>>> that we're shipping. >>>> >>>> The fact that tests are failing with this flag change should be a huge >>>> flashing red light about this proposal - if we make the tests not test the >>>> code we're shipping, then we risk it breaking without us noticing. Indeed, >>>> I've been trying to turn this flag on for Android, and have been struggling >>>> to do so. >>>> >>>> - Leszek >>>> >>>> >>>> On Mon, 15 Jul 2024, 20:25 Dave Tapuska, <[email protected]> wrote: >>>> >>>>> Have you thought about setting gn config v8_use_external_startup_data >>>>> on any bots? >>>>> >>>>> I don't know why they are failing. The window constructor one is weird >>>>> as well because the prototype chain is different... >>>>> >>>>> dave. >>>>> >>>>> On Mon, Jul 15, 2024 at 2:13 PM Nico Weber <[email protected]> >>>>> wrote: >>>>> >>>>>> Hello, >>>>>> >>>>>> use_v8_context_snapshot causes us to build lots of files (blink + >>>>>> dependencies) twice on bots that cross-compiler (win/arm64, android, >>>>>> etc). >>>>>> >>>>>> We'd like to turn off use_v8_context_snapshot in regular release >>>>>> builds by default, and only keep it enabled in is_official_build builds. >>>>>> >>>>>> However, a small number of tests, almost all of >>>>>> them inspector-protocol tests, fail if I try this: >>>>>> https://chromium-review.googlesource.com/c/chromium/src/+/5704136 >>>>>> >>>>>> That's surprising to me. I would've expected use_v8_context_snapshot >>>>>> to not change behavior. >>>>>> >>>>>> Does anyone have an idea why this might happen? >>>>>> >>>>>> Thanks, >>>>>> Nico >>>>>> >>>>>> -- >>>>>> 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 on the web visit >>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADZ1XibPV%3Dd448fckQeYFaO617_9aahsxysAjMEdQ4sy%3DQw4kQ%40mail.gmail.com >>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADZ1XibPV%3Dd448fckQeYFaO617_9aahsxysAjMEdQ4sy%3DQw4kQ%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 on the web visit >>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHgVhZU-er_dfh_esgBmfLQ%2BOVf24y2yN7K6W6yjGqNkH%3Df%3D6w%40mail.gmail.com >>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHgVhZU-er_dfh_esgBmfLQ%2BOVf24y2yN7K6W6yjGqNkH%3Df%3D6w%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 on the web visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGRskv_WMBbxoTeVLhK4Pst0tSk6gOnC2ijy6vdH86P1y_bAZA%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGRskv_WMBbxoTeVLhK4Pst0tSk6gOnC2ijy6vdH86P1y_bAZA%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 on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH%2BmL5DjurzZC7u2FOFc-vqq10tPjU1LoFUn-nq%3D%2BpHCWQhAHA%40mail.gmail.com.
