Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jankeromnes opened this issue Mar 13, 2018 · 8 comments

Comments

@jankeromnes
Copy link
Member

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!

  • Can Chromium and content_shell be built side-by-side, e.g. by adding content_shell to our build instruction ninja -C out/Default chrome -j18?
  • Why would pre-running layout tests during the Chromium image build be useful? Does it produce some artifacts that make subsequent "incremental layout test runs" faster?
@phistuck
Copy link

  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.
  1. 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
Copy link

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
Copy link

dpranke commented Mar 19, 2018

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
Copy link
Member Author

jankeromnes commented Mar 22, 2018

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:

  • Use DISPLAY=":98" ./your-cmd to run your test command via our long-running xvfb server (you can view what's happening live by opening the noVNC tab)
  • Use xvfb-run ./your-cmd to run your test command in a separate temporary xvfb server (you'd have to produce JPG screenshots to view what happened in the virtual X server)

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
Copy link

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
Copy link
Member Author

jankeromnes commented Mar 22, 2018

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
Copy link

dpranke commented Mar 22, 2018

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants