Cache manifest everywhere #14537
Cache manifest everywhere #14537
Conversation
|
|
||
| load_manifest = _init_manifest_cache() | ||
| def load_manifest(manifest_path=None, manifest_update=True): | ||
| from manifest import manifest |
There was a problem hiding this comment.
Switch to a delayed import here because otherwise Python will think there are two different manifest modules (because of different import paths) and we don't get the singleton.
There was a problem hiding this comment.
What do you say we document your rationale with a comment? I'm afraid of your
optimization being accidentally subverted by some future refactoring (the
corresponding performance degradation might not be noticed immediately).
This is beyond my experience with Python, though. Can you advise on any
longer-term goals that would obviate this kind of consideration?
There was a problem hiding this comment.
I added a comment here, but I don't really have great ideas how to make sure we always get a singleton.
There was a problem hiding this comment.
Did you verify that the previous code was broken? I know that foo.bar and bar are different even if they're the same bar.py file, but I'm slightly surprised if that also affects relative imports.
There was a problem hiding this comment.
Yes I did. It took me a while to realize what was going on.
|
On my machine, this reduces 2GB of memory usage (peak memory 5GB -> 3GB) when running ./wpt run --affected HEAD^1 --log-mach - --binary ~/tools/firefox-nightly/firefox firefox(with a dummy local commit) |
Hexcles
force-pushed
the
cache-manifest
branch
from
45679cc to
681f95b
Compare
Hexcles
force-pushed
the
cache-manifest
branch
from
681f95b to
5680792
Compare
|
I addressed the review comments, but am having trouble debugging the I'll try to dig deeper. |
Hexcles
force-pushed
the
cache-manifest
branch
from
3bbb06e to
4225541
Compare
|
So my working theory is that @jgraham PTAL again. |
foolip
force-pushed
the
cache-manifest
branch
from
4225541 to
754cf11
Compare
|
FYI, I've rebased this branch to trigger "Travis CI - Pull Request", manually setting things right after the travis-ci.com transition in #14499 changed the name of the required check, which applies retroactively. |
Move the cache layer of manifest objects from wpt.testfiles to manifest.manifest so that all users of the manifest module can benefit. This prevents us from having two in-memory copies of the manifest when running affected tests (one loaded by wpt.testfiles and the other loaded by wptrunner.testloader). Partly fixes #14421 (the memory usage part of it, but not the code health part).
The test module uses the same temporary manifest in all test cases, but some test cases modify the manifest, which causes inconsistency in the cache. This commit changes the test setup to initialize a persistent manifest on the module level, and each test case will then make a temporary copy of it. Also speed up tools/wpt tests on Azure by updating the manifest first.
Hexcles
force-pushed
the
cache-manifest
branch
from
754cf11 to
9cedce9
Compare
Move the cache layer of manifest objects from wpt.testfiles to
manifest.manifest so that all users of the manifest module can benefit.
This prevents us from having two in-memory copies of the manifest when
running affected tests (one loaded by wpt.testfiles and the other loaded
by wptrunner.testloader). Partly fixes #14421 (the memory usage part of
it, but not the code health part).