JanitorTechnology / dockerfiles

popular development environments as containers
https://hub.docker.com/u/janitortechnology/
53 stars 20 forks source link

[chromium] consider pre-compiling content_shell and pre-running layout tests #164

Open jankeromnes opened 6 years ago

jankeromnes commented 6 years ago

I see that [Chromium] is built, but content_shell is not built and the layout tests were not run. Could those be factored into the resulting container as well?

Thank you. :)

Thank you @phistuck for suggesting this idea!

phistuck commented 6 years ago
  1. I guess so, I am not nearly knowledgeable enough in the Chromium build process to answer. @dpranke (hopefully this is the right dpranke) - can you assist? (Perhaps we should move this to blink-dev or chromium-dev, though)

  2. Incentives -

    • I would know that the base commit is a good one (if it is), that passes all (or practically all) of the tests.
    • I would know that my changes might have broken some test (and not the base commit). Other than that, I do not think it would affect incremental layout test runs, but maybe, @dpranke (sorry for picking on you) might know.
  3. The layout tests ran from ~17:20 until ~2:10 (including retries) on the old (8 core) machine. I guess it would be faster on the new (gazillion core) machine. I will report back once I finished running them.

phistuck commented 6 years ago

Perhaps building "all" instead of only Chromium would be the right thing to do here, as it would also compile JavaScript with Closure Compiler and other (debatable) goodies.

Regarding the graphical environment, Tom Anderson mentioned xvfb on blink-dev -

Try running the layout tests under xvfb. Should be $ tools/xvfb.py <what you'd normally use to run layout tests>

dpranke commented 6 years ago

Yes, you can change ninja chrome to ninja chrome content_shell just fine.

Running the layout tests might help establish that this is a Known Good Revision, just like running any test would, but apart from that it wouldn't be particularly useful.

jankeromnes commented 6 years ago

Perhaps building "all" instead of only Chromium would be the right thing to do here, as it would also compile JavaScript with Closure Compiler and other (debatable) goodies.

Yes, you can change ninja chrome to ninja chrome content_shell just fine.

Thanks! So I guess we have two options to pre-build content_shell:

  1. ninja -C out/Default all -j`nproc`
  2. ninja -C out/Default chrome content_shell -j`nproc`

@phistuck or @beaufortfrancois any preference?

Regarding the graphical environment, Tom Anderson mentioned xvfb on blink-dev -

Interesting suggestion. I think @beaufortfrancois does this as well. I believe you have two options:

Running the layout tests might help establish that this is a Known Good Revision, just like running any test would, but apart from that it wouldn't be particularly useful.

@dpranke Thanks! But doesn't Chromium already have a "Last Known Good Revision"? Or is that just for "it compiles" and not "it passes all tests"?

phistuck commented 6 years ago

If it is scalable, I prefer all (I, for one, will probably work more on the Developer Tools, for example) and layout tests.

I managed to run almost all of the layout tests without even using xvfb, so I guess it is a non issue at the moment. The few (4) tests that eventually failed were due to the new Ubuntu the virtual machine is using, apparently (see the blink-dev thread that confirms it).

Chromium used to maintain Last Known Good Revision, but if it still exists, it is rarely used for anything nowadays, as far as I remember reading. Last Know Compilable Revision is used instead, I believe. Even if the former was maintained, I believe it did not run the layout tests, but this is just a speculation at this point.

jankeromnes commented 6 years ago

If it is scalable, I prefer all (I, for one, will probably work more on the Developer Tools, for example) and layout tests.

Noted, thanks. Let's try switching to all then.

Currently, pre-building Chrome on Janitor takes us ~3h in the background (see Image update) stats, which is why we build it on-site instead of via CircleCI (their free tier has weaker hardware and ~2h build timeouts). If building all can be done in under 8 hours, that'd be great because then we can run it overnight when fewer people are bothered by the high server load.

I managed to run almost all of the layout tests without even using xvfb, so I guess it is a non issue at the moment.

Thanks, that's great to know. Note that any graphical layout tests might still pick up on our running xvfb server (e.g. via the DISPLAY=:98 environment variable), so there is a chance that they run on xvfb behind your back (just a theory though).

Chromium used to maintain Last Known Good Revision, but if it still exists, it is rarely used for anything nowadays, as far as I remember reading. Last Know Compilable Revision is used instead, I believe. Even if the former was maintained, I believe it did not run the layout tests, but this is just a speculation at this point.

Thanks. So by running all layout tests as part of image build, Janitor would become sort of a new LKGR tracker for Chromium I guess.

Note that Janitor is entirely community-funded. Would Google be interested in helping us fund Janitor's hosting in order to boost Chromium development on there?

dpranke commented 6 years ago

Chromium's LKGR is gone, and LKCR is almost gone. What we'd found is that a generic definition of either didn't make all that much sense given the complexity of all of the different build configs (e.g., do you want to block the linux build if mac doesn't compile?), and so we have generally switched to specific checks for specific needs.

Note that building all will build a lot of stuff you probably don't need or care about (like all of the test binaries).

beaufortfrancois commented 6 years ago

I think* you don't need to specify -j as ninja will use the max available by default.