Re: [webkit-dev] Deployment of new EWS Non-Unified builder
> On Sep 8, 2022, at 5:41 AM, Adrian Perez de Castro via webkit-dev > wrote: > On Thu, 08 Sep 2022 02:27:53 + Alexey Proskuryakov via webkit-dev > wrote: >> Without non-unified EWS, or anyone fixing non-unified build manually: >> - a smaller number of patches gets rejected for breaking existing EWS >> builders; >> - it's sometimes harder to fix, because the errors could be in unfamiliar >> code (although how hard can it be to add an include, on average). > > Not fixing the build ever would not work. If we don't land patches which are > technically correct there will be always situations in which the build will > randomly break. > > Example: Buildbot (both EWS and post-commit) build with DEVELOPER_MODE=ON and > ENABLE_EXPERIMENTAL_FEATURES=ON, while end user builds disable those. The > changed options can result in shifted sources in the unified build compilation > units. What passes as "yeah, it builds" in the bots could still fail later > when producing releases. This applies to *all* ports. This is not true. Build flags are if-def’s inside each file, not a condition on whether a given file is unified or not. Enabling or disabling any feature should not affect which set of cpp files are unified together. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Thu, Sep 8 2022 at 03:00:00 PM +0300, Adrian Perez de Castro wrote: My laptop has 20 GiB of memory and a debug build in non-unified mode links just fine with either LLD or Mold (I haven't used the GNU linker for months). Something smells fishy with your setup. I haven't changed the default linker, so it's using ld.bfd. We used to prefer ld.gold by default, but don't anymore. That's probably why the RAM required has ballooned. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Hi list, On Thu, 08 Sep 2022 02:27:53 + Alexey Proskuryakov via webkit-dev wrote: > Ross, I didn't mean any disrespect. > I absolutely agree that this issue is not about the project supporting > multiple platforms. > What I disagree with is the statement that omitting includes necessary for > non-unified build is a mistake, or that it needs to be made visible. Fixing > all of these is very time consuming, whether it's done pre- or post-commit. > In my mind, it is a logical conclusion that this shouldn't be done at all, > neither pre- nor post-commit. The downside is that patches are more likely > to break the build, but overall it's less work, because only a small > percentage of "missing" includes will ever cause problems. So, ignoring > non-unified build saves work over time, and saves the 6 days per month that > its maintenance has been costing. > > Perhaps that's incorrect, and the cost situation is the opposite? So that > ignoring non-unified builds is costlier overall? It is a real cost, in > particular because it sometimes requires fixing issues in unfamiliar code, > but that cost hasn't been quantified AFAICT. My personal experience is that building locally with non-unified builds while working on making a patch practically does not add any overhead -- most of the time I always work with non-unified builds. What takes more time is to fix the non-unified build caused by other people's patches if the non-unified build has been broken since the last time I pulled from the repository. More precisely: sometimes it takes a lot of time, and often it does not take much to fix the build itself; but even for the simple ones having to write commit logs, waiting for the EWS, and then some more time while commit-queue does its thing easily adds 30 to 40 minutes before I can go on with what I initially planned to work on. > In other words, the choices are: > > With non-unified EWS: > - many patches get rejected for breaking it; > - it's easy for the patch author to add the includes. - ensures that patches we land are as correct as possible Patches rejected for breaking it are patches which are technically wrong. I am sure we all agree that unified builds are a clever, but disgusting workaround to speed up builds. If went back to 2017 for a moment, when we did not have unified builds [1], We would be rejecting these patches anyway, because they do break the build. And nobody would disagree. > Without non-unified EWS, or anyone fixing non-unified build manually: > - a smaller number of patches gets rejected for breaking existing EWS > builders; > - it's sometimes harder to fix, because the errors could be in unfamiliar > code (although how hard can it be to add an include, on average). Not fixing the build ever would not work. If we don't land patches which are technically correct there will be always situations in which the build will randomly break. Example: Buildbot (both EWS and post-commit) build with DEVELOPER_MODE=ON and ENABLE_EXPERIMENTAL_FEATURES=ON, while end user builds disable those. The changed options can result in shifted sources in the unified build compilation units. What passes as "yeah, it builds" in the bots could still fail later when producing releases. This applies to *all* ports. > Without non-unified EWS, but someone fixes non-unified build manually: > - an even smaller number of patches gets rejected due to missing includes; > - it's a huge investment on the part of those who keep fixing the non-unified > build. Cheers, —Adrián --- [1] https://bugs.webkit.org/show_bug.cgi?id=177190 signature.asc Description: PGP signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Hi Michael, On Wed, 07 Sep 2022 12:41:00 -0500 Michael Catanzaro via webkit-dev wrote: > On Sat, May 21 2022 at 09:43:06 AM -0500, Michael Catanzaro > wrote: > > I would go even further and consider enabling unified builds only in > > DEVELOPER_MODE (for CMake ports). For non-developer builds, > > compilation time is much less important than limiting RAM usage to > > reasonable levels. Using ninja's default parallelization level, I > > recently started hitting OOM failures even on a machine with 64 GB > > RAM! We have many people complaining that they cannot build on more > > normal machines with 16 GB RAM. If we have an EWS to ensure the > > non-unified build actually works, then it should be safe enough to > > make it the normal supported path for non-developers, rather than > > just a "best effort, let's hope it works today" thing. > > I withdraw this proposal. > > I thought that non-unified builds would significantly reduce peak RAM > usage, but I was wrong [...] The main reason for having working non-unified builds has never been memory usage. I would expect that with a reasonable linker memory usage is similar because the amount of code, relocations, symbol resolutions, etc. to handle is about the same. The main difference is file descriptor usage and that there will be more debuginfo copies that the linker has to deduplicate in non-unified mode--some linkers may handle that worse than others. > [...] In fact, non-unified builds seem to require substantially more RAM at > link time, perhaps because there are more object files to link together. On > a desktop with 64 GB of RAM, I'm not able to link a non-unified build > successfully without running out of RAM, but using a unified build it works > fine. So my proposal was just totally off base. My laptop has 20 GiB of memory and a debug build in non-unified mode links just fine with either LLD or Mold (I haven't used the GNU linker for months). Something smells fishy with your setup. Cheers, —Adrián signature.asc Description: PGP signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
If I may add, On Thu, 2022-09-08 at 02:27 +, Alexey Proskuryakov via webkit-dev wrote: > With non-unified EWS: > - many patches get rejected for breaking it; > - it's easy for the patch author to add the includes. - over time, by force of habit /less patches break/, as contributors become more aware of the need to check they add includes properly, in the same way that developers get used to follow coding style to avoid style errors. Claudio ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Ross, I didn't mean any disrespect. I absolutely agree that this issue is not about the project supporting multiple platforms. What I disagree with is the statement that omitting includes necessary for non-unified build is a mistake, or that it needs to be made visible. Fixing all of these is very time consuming, whether it's done pre- or post-commit. In my mind, it is a logical conclusion that this shouldn't be done at all, neither pre- nor post-commit. The downside is that patches are more likely to break the build, but overall it's less work, because only a small percentage of "missing" includes will ever cause problems. So, ignoring non-unified build saves work over time, and saves the 6 days per month that its maintenance has been costing. Perhaps that's incorrect, and the cost situation is the opposite? So that ignoring non-unified builds is costlier overall? It is a real cost, in particular because it sometimes requires fixing issues in unfamiliar code, but that cost hasn't been quantified AFAICT. In other words, the choices are: With non-unified EWS: - many patches get rejected for breaking it; - it's easy for the patch author to add the includes. Without non-unified EWS, or anyone fixing non-unified build manually: - a smaller number of patches gets rejected for breaking existing EWS builders; - it's sometimes harder to fix, because the errors could be in unfamiliar code (although how hard can it be to add an include, on average). Without non-unified EWS, but someone fixes non-unified build manually: - an even smaller number of patches gets rejected due to missing includes; - it's a huge investment on the part of those who keep fixing the non-unified build. - Alexey 7 сент. 2022 г., в 6:20 PM, Kirsling, Ross via webkit-dev mailto:webkit-dev@lists.webkit.org> > написал(а): This conversation has had some diverging threads (which is what makes mailing list-based communication so difficult), but I'm disappointed that this should mean that the crux of the conversation was lost. There is no such thing as "not maintaining the non-unified build"; there never has been. We have covered that this is an *inherent* problem in a unified build mechanism and that this would be an issue even if Mac were the only platform. The *singular* question at play is whether contributors have visibility into the repercussions of their mistakenly omitted includes. The proposal is to have a bot to provide this visibility, and Igalia already has one ready to offer. No single platform can provide complete visibility because no single platform compiles everything, but incomplete visibility would still save a huge amount of labor regularly performed by the community. The fact that this labor can not only be overlooked but outright *defended* is, quite frankly, appalling—it is the most disrespectful behavior I've witnessed in my time contributing to this community. Ross Get Outlook for Android <https://aka.ms/AAb9ysg> From: Alexey Proskuryakov via webkit-dev mailto:webkit-dev@lists.webkit.org> > Sent: Wednesday, September 7, 2022 5:05:40 PM To: Michael Catanzaro mailto:mcatanz...@gnome.org> > Cc: webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> mailto:webkit-dev@lists.webkit.org> > Subject: Re: [webkit-dev] Deployment of new EWS Non-Unified builder I can't speak for everyone, but the reason why I haven't been responding was that the discussion went in circles, and didn't address the concerns raised. There is new evidence showing that maintaining the non-unified build is very hard. We knew that from the start, which is why the plan was to not maintain it. Seems like this newly posted evidence only reinforces the decision. - Alexey > 7 сент. 2022 г., в 10:41 AM, Michael Catanzaro via webkit-dev > mailto:webkit-dev@lists.webkit.org> > > написал(а): > > > At this point I would just go ahead and create the EWS bot. Even if it's not > going to be a default build configuration, we're still wasting a bunch of > time and effort to keep it working, and the EWS would help fix that. > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> > https://urldefense.com/v3/__https://lists.webkit.org/mailman/listinfo/webkit-dev__;!!JmoZiZGBv3RvKRSx!_-YAPcPo_76ayjWZ1XeMQ8GeKGm4GosQLa_S2VyNAFELCChziGVCya-6PVjS2uwD27UxfA_DnSX4FpDUJ7rwqxwrjek$ > > <https://urldefense.com/v3/__https://lists.webkit.org/mailman/listinfo/webkit-dev__;!!JmoZiZGBv3RvKRSx!_-YAPcPo_76ayjWZ1XeMQ8GeKGm4GosQLa_S2VyNAFELCChziGVCya-6PVjS2uwD27UxfA_DnSX4FpDUJ7rwqxwrjek$> > ___ webkit-dev mailing list webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> https://urldefense.com/v3
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
This conversation has had some diverging threads (which is what makes mailing list-based communication so difficult), but I'm disappointed that this should mean that the crux of the conversation was lost. There is no such thing as "not maintaining the non-unified build"; there never has been. We have covered that this is an *inherent* problem in a unified build mechanism and that this would be an issue even if Mac were the only platform. The *singular* question at play is whether contributors have visibility into the repercussions of their mistakenly omitted includes. The proposal is to have a bot to provide this visibility, and Igalia already has one ready to offer. No single platform can provide complete visibility because no single platform compiles everything, but incomplete visibility would still save a huge amount of labor regularly performed by the community. The fact that this labor can not only be overlooked but outright *defended* is, quite frankly, appalling—it is the most disrespectful behavior I've witnessed in my time contributing to this community. Ross Get Outlook for Android<https://aka.ms/AAb9ysg> From: Alexey Proskuryakov via webkit-dev Sent: Wednesday, September 7, 2022 5:05:40 PM To: Michael Catanzaro Cc: webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] Deployment of new EWS Non-Unified builder I can't speak for everyone, but the reason why I haven't been responding was that the discussion went in circles, and didn't address the concerns raised. There is new evidence showing that maintaining the non-unified build is very hard. We knew that from the start, which is why the plan was to not maintain it. Seems like this newly posted evidence only reinforces the decision. - Alexey > 7 сент. 2022 г., в 10:41 AM, Michael Catanzaro via webkit-dev > написал(а): > > > At this point I would just go ahead and create the EWS bot. Even if it's not > going to be a default build configuration, we're still wasting a bunch of > time and effort to keep it working, and the EWS would help fix that. > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://urldefense.com/v3/__https://lists.webkit.org/mailman/listinfo/webkit-dev__;!!JmoZiZGBv3RvKRSx!_-YAPcPo_76ayjWZ1XeMQ8GeKGm4GosQLa_S2VyNAFELCChziGVCya-6PVjS2uwD27UxfA_DnSX4FpDUJ7rwqxwrjek$ ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://urldefense.com/v3/__https://lists.webkit.org/mailman/listinfo/webkit-dev__;!!JmoZiZGBv3RvKRSx!_-YAPcPo_76ayjWZ1XeMQ8GeKGm4GosQLa_S2VyNAFELCChziGVCya-6PVjS2uwD27UxfA_DnSX4FpDUJ7rwqxwrjek$ ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
I can't speak for everyone, but the reason why I haven't been responding was that the discussion went in circles, and didn't address the concerns raised. There is new evidence showing that maintaining the non-unified build is very hard. We knew that from the start, which is why the plan was to not maintain it. Seems like this newly posted evidence only reinforces the decision. - Alexey > 7 сент. 2022 г., в 10:41 AM, Michael Catanzaro via webkit-dev > написал(а): > > > At this point I would just go ahead and create the EWS bot. Even if it's not > going to be a default build configuration, we're still wasting a bunch of > time and effort to keep it working, and the EWS would help fix that. > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
At this point I would just go ahead and create the EWS bot. Even if it's not going to be a default build configuration, we're still wasting a bunch of time and effort to keep it working, and the EWS would help fix that. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Sat, May 21 2022 at 09:43:06 AM -0500, Michael Catanzaro wrote: I would go even further and consider enabling unified builds only in DEVELOPER_MODE (for CMake ports). For non-developer builds, compilation time is much less important than limiting RAM usage to reasonable levels. Using ninja's default parallelization level, I recently started hitting OOM failures even on a machine with 64 GB RAM! We have many people complaining that they cannot build on more normal machines with 16 GB RAM. If we have an EWS to ensure the non-unified build actually works, then it should be safe enough to make it the normal supported path for non-developers, rather than just a "best effort, let's hope it works today" thing. I withdraw this proposal. I thought that non-unified builds would significantly reduce peak RAM usage, but I was wrong. In fact, non-unified builds seem to require substantially more RAM at link time, perhaps because there are more object files to link together. On a desktop with 64 GB of RAM, I'm not able to link a non-unified build successfully without running out of RAM, but using a unified build it works fine. So my proposal was just totally off base. Michael ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Hi again, On Fri, 02 Sep 2022 15:02:05 +0300 Adrian Perez de Castro wrote: > I have picked this message from Ryosuke because I want to explicitly address > a misjudged metric on the effort it takes to address non-unified build issues > after patches that break them have landed -- but my comments apply to the > discussion in general. > > On Wed, 01 Jun 2022 16:39:39 -0700 Ryosuke Niwa via webkit-dev > wrote: > > On Wed, Jun 1, 2022 at 16:10 Kirsling, Ross via webkit-dev < > > webkit-dev@lists.webkit.org> wrote: > > > > [...] > > > > One day per month for one beginner sounds like a really low maintenance > > cost compared to having every WebKit developer fix non-unified builds at > > all times. > > I have recorded at least 23 patches fixing non-unified builds in the last > couple of weeks. Not all of them from Sony/Igalia folks. Assuming that each > patch took around 1h to solve on average, this means we are investing a > whopping *six days per month* fixing non-unified builds. Let me re-state > that: > > We have collectively spent 1 out of every 4 weeks fixing build issues. I forgot to add the list of patches I gathered. Here it is: https://github.com/WebKit/WebKit/pull/3241 https://github.com/WebKit/WebKit/pull/3301 https://github.com/WebKit/WebKit/pull/3307 https://github.com/WebKit/WebKit/pull/3335 https://github.com/WebKit/WebKit/pull/3336 https://github.com/WebKit/WebKit/pull/3337 https://github.com/WebKit/WebKit/pull/3338 https://github.com/WebKit/WebKit/pull/3339 https://github.com/WebKit/WebKit/pull/3403 https://github.com/WebKit/WebKit/pull/3495 https://github.com/WebKit/WebKit/pull/3499 https://github.com/WebKit/WebKit/pull/3524 https://github.com/WebKit/WebKit/pull/3642 https://github.com/WebKit/WebKit/pull/3699 https://github.com/WebKit/WebKit/pull/3732 https://github.com/WebKit/WebKit/pull/3753 https://github.com/WebKit/WebKit/pull/3756 https://github.com/WebKit/WebKit/pull/3767 https://github.com/WebKit/WebKit/pull/3785 https://github.com/WebKit/WebKit/pull/3803 https://github.com/WebKit/WebKit/pull/3820 https://github.com/WebKit/WebKit/pull/3905 https://github.com/WebKit/WebKit/pull/3930 Cheers, —Adrián signature.asc Description: PGP signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Hello, I have picked this message from Ryosuke because I want to explicitly address a misjudged metric on the effort it takes to address non-unified build issues after patches that break them have landed -- but my comments apply to the discussion in general. On Wed, 01 Jun 2022 16:39:39 -0700 Ryosuke Niwa via webkit-dev wrote: > On Wed, Jun 1, 2022 at 16:10 Kirsling, Ross via webkit-dev < > webkit-dev@lists.webkit.org> wrote: > > > I feel like this has been discussed adequately in the past, but one more > > time for good measure: > > > > Any two platforms which don't build the exact same set of files will > > undergo unification differently. That means that unification shifts are an > > inherent part of working on WebKit, embedder or otherwise. > > > > The only way to be certain that includes are done correctly in a given > > patch is to perform a non-unified build. This would be an unreasonable > > burden for local development, but that's exactly why an EWS builder is > > desirable. > > > > To have this appear like a non-issue is to ignore the work that Sony and > > Igalia have continually performed through the 5(?) years since unified > > builds were introduced. From experience, I know that it can take a person > > about a day per month to clean up includes on behalf of the entire WebKit > > community. > > One day per month for one beginner sounds like a really low maintenance > cost compared to having every WebKit developer fix non-unified builds at > all times. I have recorded at least 23 patches fixing non-unified builds in the last couple of weeks. Not all of them from Sony/Igalia folks. Assuming that each patch took around 1h to solve on average, this means we are investing a whopping *six days per month* fixing non-unified builds. Let me re-state that: We have collectively spent 1 out of every 4 weeks fixing build issues. Let that sink in for a moment. Sure, some patches take less than an hour, but I *know* from working on them that some take definitely more than one or two hours. One hour average seems reasonable, knowing that it's not only applying the fix, but also testing whether it worked, making sure we don't introduce unneeded #includes, writing commit log entries, and waiting that EWS runs over the fix to be minimally sure that nothing else got broken. The above 1h per patch is time from *experienced* developers. Even if we could offload these patches to beginners, it would take them *even more time* and in my opinion it would be *extremely unfair* towards them. Not to mention it would be counter-productive for the WebKit project at large to burn out newcomers with a responsibility that, as others already pointer, should be shared among all of us. > Each patch should be responsible for getting its own includes correct. > > > > It's unclear that this makes sense given that we can already fix build > failures caused by different set of translation units getting unified for > WebKit ports that have their own EWS bots. > > One day per person per month sounds like a totally reasonable cost for us > to ask of ports that don't yet to have EWS setup in the upstream WebKit > project. > > Inevitably, such ports already face other more complex build failures > whenever we refactor WebKit or WebCore platform by, say, introducing new > client interface or pure virtual member function that has to be overridden > in each platform / port. This is a straw man argument. The fact that other changes like refactors may break other ports is not a justification to push more build failures (even if simpler) to other ports *unnecessarily*. We are not only pointing the issues with non-unified builds, but we are also putting the money^W effort where our mouth is. Not only we had made building WebKit more robust for everybody with our continued trickle of patches, we have spent time and hardware resources setting up a post-commit builder, and now we are offering to commit further hardware to have pre-commit EWS builders. We want to see the EWS block from merging patches which break the non-unified builds *that* badly. Cheers, —Adrián signature.asc Description: PGP signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On 6/8/22 02:40, Elliott Williams via webkit-dev wrote: > >> On Jun 7, 2022, at 10:27, Olmstead, Don via webkit-dev >> wrote: >> >> If we wanted to try any tooling around identifying when an include or >> forward declaration should be used we need a functioning non-unified build. >> We could try IWYU on the codebase, >> https://github.com/include-what-you-use/include-what-you-use , and see what >> happens. > I’ve been thinking about this, too. For Mac builds, we already have the > infrastructure to emit the compile database needed to run IWYU as a > post-build step. We could limit it to the files changed in a commit, which > ought to be very fast, and the verification would be higher-quality than a > non-unified builder since it’ll catch unused includes. In theory, we could > run it during local builds, too. > > But any meaningful integration would require figuring out how to build and > deploy it reliably, which seems annoying since it requires libclang, and > needs to be rebuilt whenever we change compilers. This sounds great. Any solution that helps to solve or miminize the current issues related to unified builds has my support. -- Diego ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On 6/7/22 01:39, Geoffrey Garen via webkit-dev wrote: >> As such, I also think that the non-unified EWS being green should not be a >> blocker to landing a patch. But I think having it there for information will >> help the situation. At minimum, even if every engineer simply ignores the >> non-unified EWS, it also makes it easier for someone trying to fix a trim >> missing include build issue to scan through PRs to look for this EWS failure >> in order to narrow down on which patches (and therefore possible includes) >> to focus on. > > Is this the proposal on the table -- to have an EWS bot, but also not block > patches on it? > > That's surprising to me, and not how EWS bots usually work. If we just want > an optional record of where a particular build configuration started failing, > Isn't that just... a not-EWS bot? The post-commit Non-Unified build bot already covers this use case: - https://build.webkit.org/#/builders/133 This bot is useful to pinpoint which commit introduced a change that broke non-unified builds. Although if it's not kept in good shape it becomes less and less useful over time, as errors accumulate and mask one another. So the proposal is that the EWS Non-Unified bot should work as all the other EWS bots do. -- Diego___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
> On Jun 7, 2022, at 10:27, Olmstead, Don via webkit-dev > wrote: > > If we wanted to try any tooling around identifying when an include or forward > declaration should be used we need a functioning non-unified build. We could > try IWYU on the codebase, > https://github.com/include-what-you-use/include-what-you-use , and see what > happens. I’ve been thinking about this, too. For Mac builds, we already have the infrastructure to emit the compile database needed to run IWYU as a post-build step. We could limit it to the files changed in a commit, which ought to be very fast, and the verification would be higher-quality than a non-unified builder since it’ll catch unused includes. In theory, we could run it during local builds, too. But any meaningful integration would require figuring out how to build and deploy it reliably, which seems annoying since it requires libclang, and needs to be rebuilt whenever we change compilers. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
I agree with everything Mark said in his reply I just wanted to add another benefit of having a non-unified build specifically to support build time optimization. > > 2) In my contributions, I don’t just want to add missing includes, I want > > to remove unnecessary ones taking full advantage of forward declarations > > and moving code out of headers. Too many includes and too much dependency > > has a dramatic bad effect on the project, making colossal project build > > times even worse. > I too try to do this when writing my patches. I would argue that a > non-unified EWS gives me greater confidence that a forward declaration is > sufficient, and that I’m not just mistakenly thinking that it is only because > some other file in the same unified unit also happen to include the header. > So, again, a non-unified EWS is helping here. If we wanted to try any tooling around identifying when an include or forward declaration should be used we need a functioning non-unified build. We could try IWYU on the codebase, https://github.com/include-what-you-use/include-what-you-use , and see what happens. There's also tools for analyzing where the build is spending its time. For Clang users the ClangBuildAnalyzer could be used, https://github.com/aras-p/ClangBuildAnalyzer . For Visual Studio a project that looks interesting is CompileScore, https://github.com/Viladoman/CompileScore . ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
If it's useful as a data point, in Gecko we used not to care about non-unified builds. This worked kind of ok, because the file arrangements were mostly deterministic by directory. However, folks running with weird build configurations always ended up hitting these issues (and they might not know that they're not their fault). Furthermore, various IDEs and tooling rely on dependencies being right / files building in non-unified mode to provide useful diagnostics while editing the files (e.g, we have a code review bot that runs clang-tidy on the files you touched, and if the dependencies are not right it just fails). To that end, we have a bot that builds a "hybrid" build configuration, and runs on CI. It's "hybrid" in the sense there's a per-directory opt out; there are some directories that still haven't been fixed to build without unified builds, but the goal is that eventually everything should build in non-unified builds. See [1] for the details. In my experience, the occasional fix-up for non-unified builds is better / simpler than having to untangle the mess after the fact (see [2] for a recent example). Cheers, -- Emilio [1]: https://firefox-source-docs.mozilla.org/build/buildsystem/unified-builds.html#building-outside-of-the-unified-environment [2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1772513 On 5/21/22 06:17, dpino via webkit-dev wrote: Hi, Last year we started a thread to discuss the possibility of deploying a new EWS bot that builds WebKit with Non-Unified sources [1]. This thread explains the technical reasons why a non-unified build bot is desirable. If you're aware of the problems introduced by unified sources compilation, skip the next paragraph. Unified sources compilation consists of merging several source files together into one big file to improve building times. Usually the same sources files are grouped together, but compilation flags or downstream changes can create different source file groups. When that happens, you may hit a build error. Since unified sources group files together, missing headers in one file can be satisfied by another file in the same group. What's happening in WebKit development currently is that source code that breaks non-unified compilation frequently lands without even being noticed. The breaks are usually related to missing headers. Embedders that need to support WebKit builds with different flags or maintain downstream changes are more likely to hit these compilation errors. Last year's attempt to deploy an EWS Non-Unified builder ended up in a deployment of a post-commit bot plus two workers running in the UAT. It was actually hard to get the Non-Unified builder working [2], something that we achieved at the beginning of this year (kudos to Adrián and Lauro). Since then we have been maintaining the post-commit bot very closely. There are periods of time where the bot has remained green for a long period of time. But lately maintaining the bot green has become harder and harder, specially due to the big refactorizations that have happened in WebKit source code lately. The bot very rarely stays green longer than 2 or 3 days without close maintenance. We believe that we all should share the responsibility of keeping the non-unified build working, and therefore we want to proceed with the deployment of a EWS Non-Unified builder. Initially, we'll provide two workers on a modern host with 8 cores assigned each. We have found this setup faster for most patches than many of the existing EWS queues, as it only runs a build in non-unified mode, without test or upload steps. If this were not fast enough, we would consider increasing the number of workers in the queue. We set up a page[3] with a rough comparison to the regular (unified) bot and the build times are pretty similar. Diego [1] https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html [2] https://bugs.webkit.org/show_bug.cgi?id=226088 [3] https://people.igalia.com/lmoura/webkit-bots-dashboard/unified.html ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
> On Jun 6, 2022, at 1:09 PM, Olmstead, Don wrote: > > If it isn’t it should be considering how many times I’ve had a cq- come from > an AppleWin build that is in no way affected by my patch. Yes, we have a problem with that AppleWin bot, and it’s one that bothers me quite a bit, but I don’t want to design our policy around that problem. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
If it isn’t it should be considering how many times I’ve had a cq- come from an AppleWin build that is in no way affected by my patch. From: Geoffrey Garen via webkit-dev Sent: Monday, June 6, 2022 10:40 AM To: Mark Lam Cc: Darin Adler ; webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] Deployment of new EWS Non-Unified builder As such, I also think that the non-unified EWS being green should not be a blocker to landing a patch. But I think having it there for information will help the situation. At minimum, even if every engineer simply ignores the non-unified EWS, it also makes it easier for someone trying to fix a trim missing include build issue to scan through PRs to look for this EWS failure in order to narrow down on which patches (and therefore possible includes) to focus on. Is this the proposal on the table — to have an EWS bot, but also not block patches on it? That’s surprising to me, and not how EWS bots usually work. If we just want an optional record of where a particular build configuration started failing, Isn’t that just… a not-EWS bot? Thanks, Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
> As such, I also think that the non-unified EWS being green should not be a > blocker to landing a patch. But I think having it there for information will > help the situation. At minimum, even if every engineer simply ignores the > non-unified EWS, it also makes it easier for someone trying to fix a trim > missing include build issue to scan through PRs to look for this EWS failure > in order to narrow down on which patches (and therefore possible includes) to > focus on. Is this the proposal on the table — to have an EWS bot, but also not block patches on it? That’s surprising to me, and not how EWS bots usually work. If we just want an optional record of where a particular build configuration started failing, Isn’t that just… a not-EWS bot? Thanks, Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
I think Ross’ point about supported ports being bitten by missing includes is valid. I also agree that it can take more time (possibly a lot more time) for an engineer stepping on the issue later in time to fix the missing includes vs the original author fixing it if a non-unified EWS points out where the issue is. I have personally encountered this myself on Mac builds. The build error seemed non-sensical at first as it had nothing to do with the patch I was developing. This wasted some time for me to figure out that it wasn’t because I accidentally made a bad edit somewhere (especially when my patch is necessarily large). Once I realized that it was due to skew in the unification boundaries, figuring out what include needed to be added also wasn’t obvious (at least at the time where I encountered this years ago). I will grant that it appears to not happen very often anymore (at least in my experience, I hasn’t happened to me since), but there is an argument to be made that this may be thanks to Italia’s effort of fixing them on a regular basis, and preventing it from becoming a problem. Anecdotal data aside, I would like to make a few points: 1. Build problems with skew in the unified files will only occur when one adds new .cpp files into the unified mix. This means: a. It will not affect most engineers who are just modifying code in existing files. b. It may affect engineers who are building features that require adding new .cpp files (not .h files). c. It may affect engineers who are refactoring code when it requires adding new .cpp files. d. It may affect engineers working on non-supported ports that are not using unified ports. The activities in (a) and (b) are common. (c) is more rare. (d), we agree we shouldn’t care about (in the sense that it should not be a burden on engineers of supported ports). As someone who does a lot of (b) and (c) activities, I appreciate having this non-unified EWS. Note that (a), (b), and (c) applies to supported ports working with unified ports. This is not a port-specific issue, nor is just an unsupported port issue. 2. Unlike build issues due platform specific code, build issues due to missing includes tend to be easy to fix if identified by a non-unified EWS because: a. The error will point exactly at the .cpp file that is failing to compile which need the missing include (as opposed to a unified file with many .cpp files). b. The missing include is directly relevant to the patch being worked by the engineer at the time. Hence, it is very likely the author of the patch will immediately recognize the type / variable declaration that is missing, and know which header file to include. There is a rare chance that the non-unified build error involves some platform specific code. In this case, I think it is fair to just ping the platform maintainers on slack to give them a heads up about it, and ask them to help resolve it. It does not need to block the patch from landing. Even so, this makes it way easier for the platform maintainers to figure out the issue rather than unsuspectingly stepping on it much later after hundreds of patches have landed. Some more thoughts in response to Darin below ... > On Jun 4, 2022, at 5:42 AM, Darin Adler via webkit-dev > wrote: > > Yes, I don’t oppose adding another EWS bot, and I was not trying to argue > against that proposal. I did intend to express my disagreement with many > points made in follow-up replies that seem quite wrong to me. > > Three other thoughts: > > 1) Even though I don’t object to adding a new bot, I will say that the idea > that a single “non-unified” bot will add a lot of value at very little cost > to the WebKit project doesn’t sound right to me. These arguments about how > long things will take don’t seem correct. My experience is that it’s quite > difficult to find, understand, and resolve errors seen in bot builds, far > more difficult than resolving errors I can see locally as I make code > changes. In my view every EWS bot we add helps weigh down the project, making > each change more difficult. I agree that this is true in general. However, it the build issue is due to a missing include, I think this is not true (see my point (2) above). > 2) In my contributions, I don’t just want to add missing includes, I want to > remove unnecessary ones taking full advantage of forward declarations and > moving code out of headers. Too many includes and too much dependency has a > dramatic bad effect on the project, making colossal project build times even > worse. I too try to do this when writing my patches. I would argue that a non-unified EWS gives me greater confidence that a forward declaration is sufficient, and that I’m not just mistakenly thinking that it is only because some other file in the same unified unit also happen to include the header. So, again, a non-unified EWS is helping here. > 3) We
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Yes, I don’t oppose adding another EWS bot, and I was not trying to argue against that proposal. I did intend to express my disagreement with many points made in follow-up replies that seem quite wrong to me. Three other thoughts: 1) Even though I don’t object to adding a new bot, I will say that the idea that a single “non-unified” bot will add a lot of value at very little cost to the WebKit project doesn’t sound right to me. These arguments about how long things will take don’t seem correct. My experience is that it’s quite difficult to find, understand, and resolve errors seen in bot builds, far more difficult than resolving errors I can see locally as I make code changes. In my view every EWS bot we add helps weigh down the project, making each change more difficult. 2) In my contributions, I don’t just want to add missing includes, I want to remove unnecessary ones taking full advantage of forward declarations and moving code out of headers. Too many includes and too much dependency has a dramatic bad effect on the project, making colossal project build times even worse. 3) We had many, many problems with platform-specific include differences before we had unified builds, with frustrated contributors if they worked with any configuration that lacked an EWS bot. It may seem that the unified-build-specific problems are something fundamentally different, but this is not how I see it. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
"Supported configurations" may indeed be too tricky of a concept to pin down, but I believe Diego explained quite thoroughly how the central issue here would be every bit as relevant in a world where we only care about standard Mac builds. If FooBar.cpp and FooBaz.cpp need the same 10 includes, then we may be able to omit all 10 of them from FooBaz.cpp and still get the unified build to pass. Now, a complete lack of includes might cause a reviewer to raise an eyebrow, but missing a few is likely to go unnoticed...until months later, when someone working on an totally unrelated patch gets bitten (e.g. by trying to add a Blah.cpp that shifts everything after it). The fact is that "oops, I missed an include" is a one-second fix for the original patch author, while it could well be a one-hour fix for someone lacking the appropriate context (just think about the inscrutable error output that comes from missing an *Inlines.h). This would be quite a frustrating hour too, since it has nothing to do with the work they meant to be doing. Without a bot to verify includes on each patch, an author cannot be blamed for missing a few; this has been our status quo, but it is not a state we are in by choice—on the contrary, it is a technical debt that this community has carried for five whole years. We should be excited to see this debt paid at last. Ross On 6/3/22, 4:19 PM, "Darin Adler via webkit-dev" wrote: Here’s my view: Long ago we agreed that we’ll ask WebKit contributors to keep builds working that have EWS bots, and not other configurations. As far as I can tell, nothing has changed that invalidates that strategy and we should stick with it. I do not agree that the statement that “all projects must build under all supported configurations” applies to WebKit. We don’t even have a concept of “supported configurations” to build that policy on. This has not been a project goal in the past and I suggest we do not add this project goal. We should continue to use the Early Warning System to define which configurations must be kept working by all contributors, with anything beyond being treated as a stretch goal. And we should continue to accept patches to fix various configurations that people want to keep working that are not checked by EWS. But we absolutely should not ask contributors to keep all possible combinations working. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://urldefense.com/v3/__https://lists.webkit.org/mailman/listinfo/webkit-dev__;!!JmoZiZGBv3RvKRSx!_0KKQpKHxvBc_6M9_Y1X-umnEdbZbpx72USGbZhbAXJN3qd9s3G_WOz4R-8KPEGDPvekm30vuZhwS6ae1IoL_Iz3Eko$ ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Here’s my view: Long ago we agreed that we’ll ask WebKit contributors to keep builds working that have EWS bots, and not other configurations. As far as I can tell, nothing has changed that invalidates that strategy and we should stick with it. I do not agree that the statement that “all projects must build under all supported configurations” applies to WebKit. We don’t even have a concept of “supported configurations” to build that policy on. This has not been a project goal in the past and I suggest we do not add this project goal. We should continue to use the Early Warning System to define which configurations must be kept working by all contributors, with anything beyond being treated as a stretch goal. And we should continue to accept patches to fix various configurations that people want to keep working that are not checked by EWS. But we absolutely should not ask contributors to keep all possible combinations working. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Thu, Jun 2, 2022 at 4:28 AM Claudio Saavedra via webkit-dev wrote: > > On Wed, 2022-06-01 at 16:39 -0700, Ryosuke Niwa via webkit-dev wrote: > > One day per month for one beginner sounds like a really low > > maintenance cost compared to having every WebKit developer fix non- > > unified builds at all times. > > I'm sorry, but this is not about having "every WK developer fix non- > unified builds at all times", it's about everyone making sure that > their patches are correct. A patch that uses API in one file without > making sure the required headers are included is not a correct patch > and unified builds are only hiding the issues. I don't see how this can > be controversial. We probably disagree on what we mean by "correct". As far as I'm concerned, a patch is correct if all relevant ports can be built. There is theoretical correctness of cpp or headers files including all the necessary headers but that's merely theoretical outside of non-unified builds. I don't think we necessarily want to spend all our time thinking about & fixing theoretical problems until they actually surface. > Also, there is no pool of beginner developers who can be fixing missing > includes all the time; even if we had spare manpower, it's more > beneficial for the project (and themselves) if they spend that time > doing gardening or fixing actual bugs. Sorry, that was a typo / bad auto correction of the word "engineer". - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Wed, 2022-06-01 at 16:39 -0700, Ryosuke Niwa via webkit-dev wrote: > One day per month for one beginner sounds like a really low > maintenance cost compared to having every WebKit developer fix non- > unified builds at all times. I'm sorry, but this is not about having "every WK developer fix non- unified builds at all times", it's about everyone making sure that their patches are correct. A patch that uses API in one file without making sure the required headers are included is not a correct patch and unified builds are only hiding the issues. I don't see how this can be controversial. Also, there is no pool of beginner developers who can be fixing missing includes all the time; even if we had spare manpower, it's more beneficial for the project (and themselves) if they spend that time doing gardening or fixing actual bugs. Claudio ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Hi Alexey, On 6/2/22 06:26, Alexey Proskuryakov wrote: > Hi, > > I'm not sure if we have a consensus on whether it is a project goal to keep > non-unified build working at all times. As discussed last year, setting up > post-commit bots is a pre-requisite for having EWS, so this part is resolved. > But proactively maintaining the non-unified build is strictly more work than > fixing it on demand. So the immediate response continues to be that we > shouldn't be doing more work when we can do less. Sorry, I disagree with your point of view. Now that we have the post-commit non-unified build it has become evident to me that actively maintaining non-unified builds is less work than doing it on demand. When the post-commit non-unified is green and a build fails is trivia to spot which commit introduced a regression and what it might be the causes. When maintaining the bot on demand, errors eventually accumulate and becomes less trivia how to fix them. A non-unified EWS bot has the same benefits as the post-commit non-unified EWS, plus is the developer who attempts to land the patch who is in charge of fixing the build. This is more efficient that offloading this work to other developer. Take for instance, this non-unified build fix I recently landed: https://github.com/webkit/webkit/compare/3a5a57d060ee%5E..3a5a57d060ee It took me a long time to figure out I had to include "JSNodeCustom.h" in some files, but possibly for the developer authorizing the patch that introduced the regression, this build error was trivial. > You mention that embedders who build with non-default flags are more likely > to hit issues. Building with non-default flags would be resulting in missing > includes for non-unified builds too, do you have an estimate of how much this > problem grows due to unified builds? I don't have stats to answer how often it happens satisfactorily, but I know that both in WebKitGTK, WPE and other WebKit ports build errors will eventually happen while building with unified sources. IMHO the earlier you can find them the better. We have even made WebKitGTK releases that failed due to unified sources build errors https://stackoverflow.com/questions/64744576/incomplete-webkitgtk-build. > How do we decide if everyone is responsible for the convenience of downstream > embedders? I think there's a wrong perception that non-unified build errors are errors happening in the WebKit ports part of the code. Most of the times they are build errors in WebCore, JavaScriptCore and many other parts of the codebase. If you build Mac with non-unified compilation probably you'll discover a bunch of build errors. I have fixed a bunch of non-unified build errors in the Mac port while building the Playwright's version of WebKit for Mac: - [macOS] Unreviewed, non-unified build fixes https://bugs.webkit.org/show_bug.cgi?id=239889 - [macOS] Unreviewed, non-unified build fixes https://bugs.webkit.org/show_bug.cgi?id=237586 - [macOS] Non-unified build fixes https://bugs.webkit.org/show_bug.cgi?id=236752 These patches landed upstream, they're not errors in the downstream code of Playwright. So it's not a downstream embedder who introduces a build error in the codebase, simply they're more likely to discover it because the introduced downstream changes causes the UnifiedSource files to be arranged in a different way. Who is the responsible for the build error? The developer who introduced the build error obviously, not the person who discovered it. >From my point of view, a project's code should be guaranteed to be always buildable under any supported configuration and it shouldn't rely on collateral effects to build successful (which is the current situation with unified sources). I understand the convenience of unified sources and the advantage they bring. A EWS non-unified bot will guarantee the entire codebase is always buildable while letting us keep using unified sources. From time to time, the EWS non-unified bot will report a build error, and the most suitable person to fix that error will be in charge of it. And we know, thanks to the post-commit non-unified bot we deployed, the non-unified bot won't introduce delays as many times it's even faster than the equivalent unified sources build bot, and of course it's always faster than the EWS LayoutTests bots. > It sounds like none of actively supported ports builds non-unified by > default, which I understand from the fact that a special post-commit queue > had to be set up for that. > > Perhaps part of the argument is that even though proactively maintaining the > non-unified build is more work, this work is somehow easier than fixing > builds on demand. If so, can you elaborate on why this is the case? Exactly, this how I see it. If we agree that a person authorizing a patch is always in a better position to fix a build error than somebody else stranger to that patch, and if we value the time of everybody equally, then we will
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Sat, May 21 2022 at 09:43:06 AM -0500, Michael Catanzaro wrote: I would go even further and consider enabling unified builds only in DEVELOPER_MODE (for CMake ports). For non-developer builds, compilation time is much less important than limiting RAM usage to reasonable levels. Using ninja's default parallelization level, I recently started hitting OOM failures even on a machine with 64 GB RAM! We have many people complaining that they cannot build on more normal machines with 16 GB RAM. If we have an EWS to ensure the non-unified build actually works, then it should be safe enough to make it the normal supported path for non-developers, rather than just a "best effort, let's hope it works today" thing. Any objections to this proposal? I would additionally request that the non-unified EWS also disable DEVELOPER_MODE, so we can additionally make sure we don't break the build when experimental features tied to that (e.g. WebRTC or LFC) are disabled. All the other EWS have DEVELOPER_MODE enabled, and this has been a regular problem in the past. Michael ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
> One day per person per month sounds like a totally reasonable cost for us to > ask of ports that don't yet to have EWS setup in the upstream WebKit project. I don’t follow; I am *purely* concerned with well-established platforms that do have EWS set up. The work that is proactively done by Sony and Igalia is intended to benefit all platforms. The result of this work is that folks seldom encounter errors on their patches that are purely due to unified build shifts and not due to anything in the content of their patch. The fact that people are able to unaware of these issues occurring is a testament to all the work that has been done. Since these errors would occur on *all* platforms, there’s no justification for this labor to be offloaded to any particular group; the problem is that we’ve not had a good solution until now (particularly due to the lack of a fully working Mac Cmake build). This bot means that people can know their patches are free of include errors without having to worry about how to verify that locally. Ross From: Ryosuke Niwa Date: Wednesday, June 1, 2022 at 4:39 PM To: "Kirsling, Ross" Cc: Alexey Proskuryakov , "webkit-dev@lists.webkit.org" Subject: Re: [webkit-dev] Deployment of new EWS Non-Unified builder On Wed, Jun 1, 2022 at 16:10 Kirsling, Ross via webkit-dev mailto:webkit-dev@lists.webkit.org>> wrote: I feel like this has been discussed adequately in the past, but one more time for good measure: Any two platforms which don't build the exact same set of files will undergo unification differently. That means that unification shifts are an inherent part of working on WebKit, embedder or otherwise. The only way to be certain that includes are done correctly in a given patch is to perform a non-unified build. This would be an unreasonable burden for local development, but that's exactly why an EWS builder is desirable. To have this appear like a non-issue is to ignore the work that Sony and Igalia have continually performed through the 5(?) years since unified builds were introduced. From experience, I know that it can take a person about a day per month to clean up includes on behalf of the entire WebKit community. One day per month for one beginner sounds like a really low maintenance cost compared to having every WebKit developer fix non-unified builds at all times. Each patch should be responsible for getting its own includes correct. It's unclear that this makes sense given that we can already fix build failures caused by different set of translation units getting unified for WebKit ports that have their own EWS bots. One day per person per month sounds like a totally reasonable cost for us to ask of ports that don't yet to have EWS setup in the upstream WebKit project. Inevitably, such ports already face other more complex build failures whenever we refactor WebKit or WebCore platform by, say, introducing new client interface or pure virtual member function that has to be overridden in each platform / port. - R. Niwa -- - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Wed, Jun 1, 2022 at 16:10 Kirsling, Ross via webkit-dev < webkit-dev@lists.webkit.org> wrote: > I feel like this has been discussed adequately in the past, but one more > time for good measure: > > Any two platforms which don't build the exact same set of files will > undergo unification differently. That means that unification shifts are an > inherent part of working on WebKit, embedder or otherwise. > > The only way to be certain that includes are done correctly in a given > patch is to perform a non-unified build. This would be an unreasonable > burden for local development, but that's exactly why an EWS builder is > desirable. > > To have this appear like a non-issue is to ignore the work that Sony and > Igalia have continually performed through the 5(?) years since unified > builds were introduced. From experience, I know that it can take a person > about a day per month to clean up includes on behalf of the entire WebKit > community. > One day per month for one beginner sounds like a really low maintenance cost compared to having every WebKit developer fix non-unified builds at all times. Each patch should be responsible for getting its own includes correct. > It's unclear that this makes sense given that we can already fix build failures caused by different set of translation units getting unified for WebKit ports that have their own EWS bots. One day per person per month sounds like a totally reasonable cost for us to ask of ports that don't yet to have EWS setup in the upstream WebKit project. Inevitably, such ports already face other more complex build failures whenever we refactor WebKit or WebCore platform by, say, introducing new client interface or pure virtual member function that has to be overridden in each platform / port. > - R. Niwa -- - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
I feel like this has been discussed adequately in the past, but one more time for good measure: Any two platforms which don't build the exact same set of files will undergo unification differently. That means that unification shifts are an inherent part of working on WebKit, embedder or otherwise. The only way to be certain that includes are done correctly in a given patch is to perform a non-unified build. This would be an unreasonable burden for local development, but that's exactly why an EWS builder is desirable. To have this appear like a non-issue is to ignore the work that Sony and Igalia have continually performed through the 5(?) years since unified builds were introduced. From experience, I know that it can take a person about a day per month to clean up includes on behalf of the entire WebKit community. Each patch should be responsible for getting its own includes correct. This has not been possible in the past, but Igalia's generosity in providing an EWS builder means that it now could be. They deserve our gratitude. Ross From: Alexey Proskuryakov Sent: Wednesday, June 1, 2022 3:26:50 PM To: Kirsling, Ross Cc: dpino ; webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] Deployment of new EWS Non-Unified builder Hi, I'm not sure if we have a consensus on whether it is a project goal to keep non-unified build working at all times. As discussed last year, setting up post-commit bots is a pre-requisite for having EWS, so this part is resolved. But proactively maintaining the non-unified build is strictly more work than fixing it on demand. So the immediate response continues to be that we shouldn't be doing more work when we can do less. You mention that embedders who build with non-default flags are more likely to hit issues. Building with non-default flags would be resulting in missing includes for non-unified builds too, do you have an estimate of how much this problem grows due to unified builds? How do we decide if everyone is responsible for the convenience of downstream embedders? It sounds like none of actively supported ports builds non-unified by default, which I understand from the fact that a special post-commit queue had to be set up for that. Perhaps part of the argument is that even though proactively maintaining the non-unified build is more work, this work is somehow easier than fixing builds on demand. If so, can you elaborate on why this is the case? - Alexey 21 мая 2022 г., в 12:10 AM, Kirsling, Ross via webkit-dev mailto:webkit-dev@lists.webkit.org>> написал(а): This is wonderful news―thanks Diego! Ross From: dpino via webkit-dev mailto:webkit-dev@lists.webkit.org>> Sent: Friday, May 20, 2022 9:17:56 PM To: webkit-dev@lists.webkit.org<mailto:webkit-dev@lists.webkit.org> mailto:webkit-dev@lists.webkit.org>> Subject: [webkit-dev] Deployment of new EWS Non-Unified builder Hi, Last year we started a thread to discuss the possibility of deploying a new EWS bot that builds WebKit with Non-Unified sources [1]. This thread explains the technical reasons why a non-unified build bot is desirable. If you're aware of the problems introduced by unified sources compilation, skip the next paragraph. Unified sources compilation consists of merging several source files together into one big file to improve building times. Usually the same sources files are grouped together, but compilation flags or downstream changes can create different source file groups. When that happens, you may hit a build error. Since unified sources group files together, missing headers in one file can be satisfied by another file in the same group. What's happening in WebKit development currently is that source code that breaks non-unified compilation frequently lands without even being noticed. The breaks are usually related to missing headers. Embedders that need to support WebKit builds with different flags or maintain downstream changes are more likely to hit these compilation errors. Last year's attempt to deploy an EWS Non-Unified builder ended up in a deployment of a post-commit bot plus two workers running in the UAT. It was actually hard to get the Non-Unified builder working [2], something that we achieved at the beginning of this year (kudos to Adrián and Lauro). Since then we have been maintaining the post-commit bot very closely. There are periods of time where the bot has remained green for a long period of time. But lately maintaining the bot green has become harder and harder, specially due to the big refactorizations that have happened in WebKit source code lately. The bot very rarely stays green longer than 2 or 3 days without close maintenance. We believe that we all should share the responsibility of keeping the non-unified build working, and therefore we want to proceed with the deployment of a EWS Non-Unified builder. Initially, we'll provide two workers
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
Hi, I'm not sure if we have a consensus on whether it is a project goal to keep non-unified build working at all times. As discussed last year, setting up post-commit bots is a pre-requisite for having EWS, so this part is resolved. But proactively maintaining the non-unified build is strictly more work than fixing it on demand. So the immediate response continues to be that we shouldn't be doing more work when we can do less. You mention that embedders who build with non-default flags are more likely to hit issues. Building with non-default flags would be resulting in missing includes for non-unified builds too, do you have an estimate of how much this problem grows due to unified builds? How do we decide if everyone is responsible for the convenience of downstream embedders? It sounds like none of actively supported ports builds non-unified by default, which I understand from the fact that a special post-commit queue had to be set up for that. Perhaps part of the argument is that even though proactively maintaining the non-unified build is more work, this work is somehow easier than fixing builds on demand. If so, can you elaborate on why this is the case? - Alexey 21 мая 2022 г., в 12:10 AM, Kirsling, Ross via webkit-dev mailto:webkit-dev@lists.webkit.org> > написал(а): This is wonderful news—thanks Diego! Ross From: dpino via webkit-dev mailto:webkit-dev@lists.webkit.org> > Sent: Friday, May 20, 2022 9:17:56 PM To: webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> mailto:webkit-dev@lists.webkit.org> > Subject: [webkit-dev] Deployment of new EWS Non-Unified builder Hi, Last year we started a thread to discuss the possibility of deploying a new EWS bot that builds WebKit with Non-Unified sources [1]. This thread explains the technical reasons why a non-unified build bot is desirable. If you're aware of the problems introduced by unified sources compilation, skip the next paragraph. Unified sources compilation consists of merging several source files together into one big file to improve building times. Usually the same sources files are grouped together, but compilation flags or downstream changes can create different source file groups. When that happens, you may hit a build error. Since unified sources group files together, missing headers in one file can be satisfied by another file in the same group. What's happening in WebKit development currently is that source code that breaks non-unified compilation frequently lands without even being noticed. The breaks are usually related to missing headers. Embedders that need to support WebKit builds with different flags or maintain downstream changes are more likely to hit these compilation errors. Last year's attempt to deploy an EWS Non-Unified builder ended up in a deployment of a post-commit bot plus two workers running in the UAT. It was actually hard to get the Non-Unified builder working [2], something that we achieved at the beginning of this year (kudos to Adrián and Lauro). Since then we have been maintaining the post-commit bot very closely. There are periods of time where the bot has remained green for a long period of time. But lately maintaining the bot green has become harder and harder, specially due to the big refactorizations that have happened in WebKit source code lately. The bot very rarely stays green longer than 2 or 3 days without close maintenance. We believe that we all should share the responsibility of keeping the non-unified build working, and therefore we want to proceed with the deployment of a EWS Non-Unified builder. Initially, we'll provide two workers on a modern host with 8 cores assigned each. We have found this setup faster for most patches than many of the existing EWS queues, as it only runs a build in non-unified mode, without test or upload steps. If this were not fast enough, we would consider increasing the number of workers in the queue. We set up a page[3] with a rough comparison to the regular (unified) bot and the build times are pretty similar. Diego [1] https://urldefense.com/v3/__https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSWjtVsXQ$ <https://urldefense.com/v3/__https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSWjtVsXQ$> [2] https://urldefense.com/v3/__https://bugs.webkit.org/show_bug.cgi?id=226088__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSqxngQkY$ <https://urldefense.com/v3/__https://bugs.webkit.org/show_bug.cgi?id=226088__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSqxngQkY$> [3] https://urldefense.com/v
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
On Sat, May 21 2022 at 07:10:30 AM +, "Kirsling, Ross via webkit-dev" wrote: This is wonderful news—thanks Diego! Agreed. I would go even further and consider enabling unified builds only in DEVELOPER_MODE (for CMake ports). For non-developer builds, compilation time is much less important than limiting RAM usage to reasonable levels. Using ninja's default parallelization level, I recently started hitting OOM failures even on a machine with 64 GB RAM! We have many people complaining that they cannot build on more normal machines with 16 GB RAM. If we have an EWS to ensure the non-unified build actually works, then it should be safe enough to make it the normal supported path for non-developers, rather than just a "best effort, let's hope it works today" thing. Michael ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Deployment of new EWS Non-Unified builder
This is wonderful news—thanks Diego! Ross From: dpino via webkit-dev Sent: Friday, May 20, 2022 9:17:56 PM To: webkit-dev@lists.webkit.org Subject: [webkit-dev] Deployment of new EWS Non-Unified builder Hi, Last year we started a thread to discuss the possibility of deploying a new EWS bot that builds WebKit with Non-Unified sources [1]. This thread explains the technical reasons why a non-unified build bot is desirable. If you're aware of the problems introduced by unified sources compilation, skip the next paragraph. Unified sources compilation consists of merging several source files together into one big file to improve building times. Usually the same sources files are grouped together, but compilation flags or downstream changes can create different source file groups. When that happens, you may hit a build error. Since unified sources group files together, missing headers in one file can be satisfied by another file in the same group. What's happening in WebKit development currently is that source code that breaks non-unified compilation frequently lands without even being noticed. The breaks are usually related to missing headers. Embedders that need to support WebKit builds with different flags or maintain downstream changes are more likely to hit these compilation errors. Last year's attempt to deploy an EWS Non-Unified builder ended up in a deployment of a post-commit bot plus two workers running in the UAT. It was actually hard to get the Non-Unified builder working [2], something that we achieved at the beginning of this year (kudos to Adrián and Lauro). Since then we have been maintaining the post-commit bot very closely. There are periods of time where the bot has remained green for a long period of time. But lately maintaining the bot green has become harder and harder, specially due to the big refactorizations that have happened in WebKit source code lately. The bot very rarely stays green longer than 2 or 3 days without close maintenance. We believe that we all should share the responsibility of keeping the non-unified build working, and therefore we want to proceed with the deployment of a EWS Non-Unified builder. Initially, we'll provide two workers on a modern host with 8 cores assigned each. We have found this setup faster for most patches than many of the existing EWS queues, as it only runs a build in non-unified mode, without test or upload steps. If this were not fast enough, we would consider increasing the number of workers in the queue. We set up a page[3] with a rough comparison to the regular (unified) bot and the build times are pretty similar. Diego [1] https://urldefense.com/v3/__https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSWjtVsXQ$ [2] https://urldefense.com/v3/__https://bugs.webkit.org/show_bug.cgi?id=226088__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSqxngQkY$ [3] https://urldefense.com/v3/__https://people.igalia.com/lmoura/webkit-bots-dashboard/unified.html__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOSPFziSrI$ ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://urldefense.com/v3/__https://lists.webkit.org/mailman/listinfo/webkit-dev__;!!JmoZiZGBv3RvKRSx!6YGOGRuK21OCDj8_MKm9gtNOYvBg9Ry10BMna8HXDHWUcUpVeDPBeA4QBT-nC3xj3j8wgt_mjpqbrrOsbTOS8gKEMAU$ ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Deployment of new EWS Non-Unified builder
Hi, Last year we started a thread to discuss the possibility of deploying a new EWS bot that builds WebKit with Non-Unified sources [1]. This thread explains the technical reasons why a non-unified build bot is desirable. If you're aware of the problems introduced by unified sources compilation, skip the next paragraph. Unified sources compilation consists of merging several source files together into one big file to improve building times. Usually the same sources files are grouped together, but compilation flags or downstream changes can create different source file groups. When that happens, you may hit a build error. Since unified sources group files together, missing headers in one file can be satisfied by another file in the same group. What's happening in WebKit development currently is that source code that breaks non-unified compilation frequently lands without even being noticed. The breaks are usually related to missing headers. Embedders that need to support WebKit builds with different flags or maintain downstream changes are more likely to hit these compilation errors. Last year's attempt to deploy an EWS Non-Unified builder ended up in a deployment of a post-commit bot plus two workers running in the UAT. It was actually hard to get the Non-Unified builder working [2], something that we achieved at the beginning of this year (kudos to Adrián and Lauro). Since then we have been maintaining the post-commit bot very closely. There are periods of time where the bot has remained green for a long period of time. But lately maintaining the bot green has become harder and harder, specially due to the big refactorizations that have happened in WebKit source code lately. The bot very rarely stays green longer than 2 or 3 days without close maintenance. We believe that we all should share the responsibility of keeping the non-unified build working, and therefore we want to proceed with the deployment of a EWS Non-Unified builder. Initially, we'll provide two workers on a modern host with 8 cores assigned each. We have found this setup faster for most patches than many of the existing EWS queues, as it only runs a build in non-unified mode, without test or upload steps. If this were not fast enough, we would consider increasing the number of workers in the queue. We set up a page[3] with a rough comparison to the regular (unified) bot and the build times are pretty similar. Diego [1] https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html [2] https://bugs.webkit.org/show_bug.cgi?id=226088 [3] https://people.igalia.com/lmoura/webkit-bots-dashboard/unified.html ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
