I want to clarify something about the "slower builds" (I originally reported the issue that led to this investigation/discussion). I was specifically interested in CQ (not local) jobs, as it related to web test rebaselining. The standard workflow for generating new web test baselines involves running a suite of CQ jobs. Of those, the win11-arm64 builders take 50% (a full hour) longer than any other job in the suite. That's a major workflow impact, and not addressed by changes that only apply to local builds. It sounds like Jeremy's suggestion would address this, but I know there are other proposals elsewhere (eg, increasing the builder bot dimensions to account for the extra work imposed on the windows-arm jobs).
On Wednesday, July 17, 2024 at 5:30:14 PM UTC-4 Jeremy Roman wrote: > On Wed, Jul 17, 2024 at 4:18 PM Erik Staab <[email protected]> wrote: > >> Let me try to summarize this thread and clarify a few points to help >> guide us to a decision. >> >> 1) The snapshot provides a significant performance improvement and there >> are currently no alternate strategies that would be better for build times >> 2) We need test coverage for the snapshot code path since it's what we >> ship to users and behavior can differ >> 3) Some tests are producing different results with the snapshot on and >> off, which we should fix regardless of the decision >> 4) Generating the snapshot has a significant impact on build times / >> costs, especially when cross compiling >> >> There are multiple lines of defense against shipping bugs to users in >> order of decreasing costs and increasing coverage: >> a) local debug edit test cycles >> b) CQ >> c) Gardened CI builders >> >> Right now we're paying a significant cost by enabling the snapshot >> everywhere. We could: >> i) disable it for (a) and the only difference is a CL author may see an >> occasional CQ failure due to the difference between snapshot on and off >> behavior >> ii) disable it for (a) and (b) and occasionally a CL will land and get >> reverted by gardeners >> >> In both of these scenarios we're not shipping more bugs to users. >> >> My suggestion would be to do (ii) and turn it off in critical paths for >> engineers knowing that we may need to revert a CL periodically. We should >> ensure that tests with both snapshot on and off code paths are healthy so >> that this is rare. I have a query to see the revert rate in chromium/src -- >> if we see more reverts due to this change then we can revisit and do (i) >> instead, or look into other options. >> >> If we aren't shipping more bugs to users and we monitor the revert rate >> does anyone have a problem with turning off snapshots in the critical >> development path? >> > > If the motivation mainly applies to cross-compiled configurations, why > change native build configurations (which I think includes most desktop > build configurations, at least) at all? > > Thanks, >> Erik >> >> On Tue, Jul 16, 2024 at 11:09 AM Jeremy Roman <[email protected]> >> wrote: >> >>> If we do so we could also only do so in build configurations that are >>> actually cross-compiling -- the build time overhead on, e.g., Linux x64 >>> targeting Linux x64 is much less. >>> >>> On Tue, Jul 16, 2024 at 11:28 AM Leszek Swirski <[email protected]> >>> wrote: >>> >>>> We could also disable for, e.g., developer debug builds, but keep >>>> enabled on bots (there's some precedent in V8 for "fast snapshots" for >>>> local dev builds). >>>> >>>> On Tue, Jul 16, 2024 at 5:26 PM Scott Violet <[email protected]> wrote: >>>> >>>>> The v8 snapshots provide substantial performance gains. Leszek >>>>> provided the data to back that up. We shouldn't remove them. The v8 team >>>>> is >>>>> in the best position to answer if there are other strategies that could >>>>> give the same performance wins without having the additional build time. >>>>> >>>>> While I believe we shouldn't remove the snapshots, I sympathize with >>>>> the desire for faster builds. Seems like the thing we care about is >>>>> ensuring we don't regress test coverage. So, if we were to disable the >>>>> snapshots on many bots, we need to ensure we have at least one bot (on >>>>> the >>>>> main waterfall) that is building the snapshots. >>>>> >>>>> -Scott >>>>> >>>>> On Mon, Jul 15, 2024 at 5:43 PM 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 >>>>>>> >>>>>>> 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-Nso4p3X-%3DEBPpi08h246gbjTRP8v7rVx%2BS3dH925%3D6A%40mail.gmail.com >>>> >>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGRskv-Nso4p3X-%3DEBPpi08h246gbjTRP8v7rVx%2BS3dH925%3D6A%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/e8d96e79-ba53-423e-87c8-22a438df7bf5n%40chromium.org.
