Open Bug 1333433 Opened 8 years ago Updated 3 years ago

Consider making the wpt manifest a product of the build system

Categories

(Testing :: web-platform-tests, defect)

Product:
Testing
Component:
web-platform-tests
Type:
defect

Tracking

(Not tracked)

People

(Reporter: jgraham, Unassigned)

References

Details

At the moment we check in the wpt test manifest. The format recently changed to make it easier to perform incremental updates, but it has the disadvantage that changing any file under testing/web-platform/tests requires a rebuild. But the problem could be entirely avoided by not checking the manifest in, but putting it in the objdir and making |mach wpt| and |mach build| (re-)generate it. The latter would be useful in CI since we could ship the prebuilt manifest to test machines along with e.g. certutil instead of building it for each test run.
s/requires a rebuild/requires the manifest is updated/
I sort of suspect that the alternate option here is just to stop linting for manifest correctness and always have the test harness do a rebuild so that we can end up out of sync with what's checked in but we always run the right thing. That would cost a few seconds per job in CI though so it's not ideal.
If the manifest is derived from files under source control, then checking in a manifest seems less than ideal. If we're only talking about say <5s to dynamically build up the manifest in a test job, I'm not too worried. Even if it is longer, my guess is there are ways to make it faster than what it is today.
Heh 5s. jgraham@luna:~/develop/web-platform-tests$ time ./manifest DEBUG:manifest:Creating new manifest at /home/jgraham/develop/web-platform-tests/MANIFEST.json INFO:manifest:Updating manifest real 1m14.754s user 1m6.212s sys 0m2.648s That's for a complete rebuild, a noop rebuild is now relatively fast: jgraham@luna:~/develop/web-platform-tests$ time ./manifest DEBUG:manifest:Opening manifest at /home/jgraham/develop/web-platform-tests/MANIFEST.json INFO:manifest:Updating manifest real 0m2.361s user 0m2.028s sys 0m0.336s Which is basically why it's checked in at the moment; a 1+ minute build process isn't much fun. It could be faster but probably not 10x faster (the slowness is html parsing all the test files to extract data).
(In reply to James Graham [:jgraham] from comment #4) > jgraham@luna:~/develop/web-platform-tests$ time ./manifest > DEBUG:manifest:Creating new manifest at > /home/jgraham/develop/web-platform-tests/MANIFEST.json > INFO:manifest:Updating manifest > > real 1m14.754s > user 1m6.212s > sys 0m2.648s :( > It could be faster but probably not 10x faster (the > slowness is html parsing all the test files to extract data). Ewww. Fastest solution is likely to rewrite that HTML parsing in Rust so we have no Python overhead and can leverage multiple CPU cores easily. That /may/ get a 10x speedup, even on test machines (which tend to only have 2-4 vCPUs). Not sure how practical that is or how willing you are to reinvent wheels in !Python.
It's difficult enough to convince other browser vendors to put time into setting up wpt in their automation without a rust dependency, so I imagine we'd at least want to make it optional.
Yeah, I would love to write the entire infrastructure in Rust; I'm pretty sure it would solve a number of problems with the Python, not just performance issues. But getting buy in to a compiled test harness seems rather unlikely.
So another suggestion that came up is to use an artifact-build like approach. m-c builds could generate the manifest and upload to S3. Then when running |mach wpt| it would first check for a MANIFEST.json in the source directory and if none is found pull one from S3 and then run an update pass on it (|mach test| might have to do something similar). In the case of no network it would obviously have to do a full local build. Does that sound sensible?
So :cactusmachete is now working on this, using the artifact approach; i.e. whenever tc runs on a push that changes testing/web-platform/tests it schedules a task to generate the manifest and upload it as an artifact. Locally we can successfully locate the manifest from the most recent wpt-changing commit in the tree which has an associated pushhead with a tc taskgroup containing a manifest upload artifact, download that to the objdir, update it for subsequent changes, and run tests. The open question is the best approach to use in CI. The steps described above depend on VCS access to find the latest wpt-changing commits that are ancestors of the current rev. Pending bug 1286900 we don't have such VCS access in CI, so it's not clear how to figure out the right manifest to download. Some possible solutions: * Hack the package-tests build step to download a manifest during packaging and add it to the zip as today. This runs in the build job which does have VCS access and so can find the right manifest artifact. It also means that no changes are required to the rest of the system (i.e. no mozharness changes). I imagine such a change would go in https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/action/test_archive.py#570 since we require custom python code to do the download and I don't know how to make this kind of thing declarative in a moz.build file. * Download the artifact as a mozharness step and get the relevant source information from hg.mozilla.org somehow without doing a clone. AFAICT we have access to the current tree and rev in the mozharness script, but I can't see a way to get the list of commits/pushes that changed a particualr directory from hg.mozilla.org. If that is actualy possible, it seems like the simplest approach to get working. * Get the source checkout on the test machines. This seems like the best long term option but not reasonable for the timeframe of this project. * Something else? :gps any preferences / suggestions?
Flags: needinfo?(gps)
One more open question here. Currently the build system knows about all tests (I think?). The `TestResolver` constructed from this is used in `mach test` and `mach try` to map paths to tests. So the question is how to replicate this behaviour with the downloaded manifest. I think there are two high level options, and a bunch of possible variants of detail. The high level options are: * Hack the test resolver to download the wpt manifest as required without really touching the build system at all. * Have the build system downoad and update the wpt manifest as part of the build so that things keep working as now, but with slightly different paths. The second option seems "right", but has a couple of problems: * It will add some time to each build to download & update the wpt manifest * I have no idea how to implement it THe first option seems like it runs the risk of playing whack-a-mole with cases where people expect the build metadata to contain the full list of tests (e.g. at some point treeherder used the list of all tests, although I think that might be gone now). (ted: you probably want to read the previous comment for more context).
Flags: needinfo?(ted)
Depends on: 1473915
OK, sorry for letting this sit, but I've read the whole bug now. Here are some thoughts: * For automation purposes we should just always build the test manifest during the build and upload it as part of the test package, and test jobs can use that manifest. That matches what we do for many other things and should be least prone to failure. * For local builds spending a minute to generate the manifest isn't great, but might not be a problem as long as it can run in parallel with other parts of the build. We have seen issues in the past where folks with access to large icecream compile clusters start to notice things like this in their build because the compilation parts of the build run so quickly. Until we get the ability to distribute Rust compilation it's unlikely that will be an issue. If rebuilds are fast then the common use case of "update to latest mozilla-central, rebuild" should not be too bad. If you've already got code to fetch the manifest artifact-style then we could certainly do that instead, but I'm not sure I'd want that to be part of the default build. We've had a pretty strict policy against network access during a default build and I don't know that this is compelling enough to change that. If you wanted to do the fetch during `mach wpt-test` that would be fine, I think. * Re: TestResolver, it might be possible to simply cheat and hack the code so that any paths under `testing/web-platform` are magically resolved as wpt, and then the wpt mach command could ensure that the manifest is up-to-date in whatever way makes the most sense (fetch an artifact, rebuild it locally). That would avoid the overhead of dealing with the manifest for people that aren't actually trying to run wpt tests. If you have any further questions let me know.
Flags: needinfo?(ted)
Importing tens of thousands of CSS tests didn't help the manifest build time; it just took 8 minutes to build from scratch on my machine. So I don't think that making it build as a normal part of the build system is a sensible idea until we make that a lot faster (which probably involves a fair amount of work; all the time is parsing HTML in python and porting that to Rust may be non-trivial). If doing something approximate in the test resolver is good enough then I think that the patch in 1473915 plus that approximation might be a reasonable way forward.
We discussed this on IRC and I think we're in agreement that the current approach of having a separate task to generate the manifest, hooking that up to the taskgraph in CI, and fetching it on-demand when developers run wpt tests should be sufficient.
CCing glandium since the WPT manifest file is causing problems for git-cinnabar. https://github.com/glandium/git-cinnabar/issues/192
What Ted said is pretty reasonable. In addition, I'd like to note: * Using the proposal of downloading a manifest artifact will require Internet connectivity to run WPT tests. That's less than ideal. We may want to consider an option to generate the manifest locally when Internet connectivity is not available. * Regarding version control, we do want VCS checkouts to be available on testers someday. We're mainly held back by high VCS operation times on the resource-constrained test workers. Partial clone support will help with this immeasurably. That is my primary area of focus for H2. * Please don't have high-volume CI tasks touching hg.mozilla.org in a manner that doesn't use `hg robustcheckout`. Doing so will likely lead to server capacity issues. If you need to resolve VCS data outside of a local clone (e.g. doing ancestry traversals, etc), it is best to perform that work in a task and then communicate the rests of that to dependent tasks via an artifact. * I don't understand the WPT architecture that well. That being said, I'm surprised that scanning for WPT tests takes >1 minute of CPU time. I'd really like to think we can find a solution to WPT test discovery that only requires a few seconds of CPU time. But if it truly is doing HTML parsing, then, yeah, I can understand why it is slow :/ It would be really nice if the metadata format could be changed so you only need to open a file and scan for a special syntax. Then we could scan thousands of files per second and the overhead wouldn't be a problem. I suppose our hands are tied :/
Flags: needinfo?(gps)
(In reply to Gregory Szorc [:gps] from comment #15) > What Ted said is pretty reasonable. > > In addition, I'd like to note: > > * Using the proposal of downloading a manifest artifact will require > Internet connectivity to run WPT tests. That's less than ideal. We may want > to consider an option to generate the manifest locally when Internet > connectivity is not available. Yes, the way this is intended to work is that we just ask for a manifest update, which does the following: 1. Checks if there is a recent existing manifest and if not downloads one 2. Performs an update on whatever manifest now exists In step 2 if there's no manifest then it just creates one from scratch which will be slow but doesn't require internet access. So this case is already handled. > * Regarding version control, we do want VCS checkouts to be available on > testers someday. We're mainly held back by high VCS operation times on the > resource-constrained test workers. Partial clone support will help with this > immeasurably. That is my primary area of focus for H2. Yes, I remember we tried this before with wpt and had to roll back. It's going to be *so* nice when it works, so I'm really excited ot hear that your working on the underlying issues :) > * Please don't have high-volume CI tasks touching hg.mozilla.org in a manner > that doesn't use `hg robustcheckout`. Doing so will likely lead to server > capacity issues. If you need to resolve VCS data outside of a local clone > (e.g. doing ancestry traversals, etc), it is best to perform that work in a > task and then communicate the rests of that to dependent tasks via an > artifact. OK, then it sounds like we might have to adjust the taskgraph after all to make wpt tests depend on the wpt manifest task. That's unfortunate in that I don't know apriori how to do it and it means that CI is a little less like local operation, but I understand that hg performance issues are a serious concern. > * I don't understand the WPT architecture that well. That being said, I'm > surprised that scanning for WPT tests takes >1 minute of CPU time. I'd > really like to think we can find a solution to WPT test discovery that only > requires a few seconds of CPU time. But if it truly is doing HTML parsing, > then, yeah, I can understand why it is slow :/ It would be really nice if > the metadata format could be changed so you only need to open a file and > scan for a special syntax. Then we could scan thousands of files per second > and the overhead wouldn't be a problem. I suppose our hands are tied :/ Right, the problem is that lots of the metadata is communicated by the HTML content in the files (presence of specific scripts or explicit <meta> or <link> elements). In principle this could be redesigned with parsing speed in mind, but that would be a lot of work. And upstream we implemented manifest download from GitHub (using releases to store the artifacts) which means that the performance outside mozilla-central is acceptable. Thanks gps and ted for the feedback, it's really helpful in making sure we end up building something that will actually work.
> OK, then it sounds like we might have to adjust the taskgraph after all to make wpt tests depend on the wpt manifest task. No, wait, thinking about how we did this, package-tests was adjusted to download a recent manifest and update from there. So it's not every wpt task that would be hitting hg.mozilla.org. Maybe we don't need to change things after all?
Depends on: 1505474
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.