Closed Bug 1479290 Opened 2 years ago Closed 2 years ago

wpt-update memory usage regressed

Categories

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

Version 3
defect
Not set

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jgraham, Assigned: jgraham)

References

Details

Attachments

(1 file)

Bug 1476053 was supposed to improve the speed and memory usage of wpt-udpate. Unfortunately a last minute change to the patch accidentially (but predictably) regressed the memory usage dramatically. Essentially the problem is that we stored the subtest names once per result rather than once per test, which turns out to be an awful lot of unicode data (eak memory usage was something over 20Gb on my system). This also crashed the wpt-sync host.

The previous memory usage was also problematically high, so some further optimisation rather than just fixing the regression would help.
The memory usage of wpt-update has always been high, and accidentially
regressed in Bug 1476053 to really problematic levels when doing a
full metadata update for all test results. This patch implements a
number of changes that reduce the peak memory usage to ~1Gb and the
heap after reading all results data to ~600Mb.

THere are several changes in this patch:

* Change from a dict {test: [(subtest, prop, run_info, result)]} to a
  nested dictionary {test: {subtest: [(prop, run_info,
  result)]}}. This fixes the silliness that caused the previous
  regression.

* Convert all unicode data to bytestrings and then intern() the
  bytestring. This allows reusing string allocations for repeated
  strings, of which there are many.

* Process the test manifests upfront and then allow the manifest data
  to be gc'd before starting to process the results files, so that we
  are not holding on to data that is no longer required.

* Stop storing status-type results as (status, default_expected); just
  look up the default_expected for a specific test if we have to
  update the expectations for that test.

* Add __slots__ to all frequently-allocated classes to avoid the
  overhead of constructing an __dict__ for each instance, which
  typically has size overhead (see
  e.g. http://book.pythontips.com/en/latest/__slots__magic.html )

* Move to using a custom compact representation for the list of
  per-subtest results i.e. [(prop, run_info, results)]. This uses an
  array.array of 2-byte ints to store the entries and effectively
  interns the values, bit packing the representations of property to
  be updated and result (if it's a test status) into 4 bits each in
  the first byte, and the run info into the second byte. Properties
  with a complex data representation (e.g. LSAN failures) are stored
  directly, with a 0 value of the status bits indicating that the
  complex value should be read.

MozReview-Commit-ID: IURU4TUfsei
Comment on attachment 8995817 [details]
Bug 1479290 - Reduce memory usage of wpt-update, r=ato

Andreas Tolfsen 「:ato」 has approved the revision.

https://phabricator.services.mozilla.com/D2496
Attachment #8995817 - Flags: review+
It’s very confusing that the commit message appears here on Bugzilla,
but not the review comments themselves or the actual .patch file,
but at least the r+ got set properly.  For what?  That’s hard to tell.
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12247 for changes under testing/web-platform/tests
https://hg.mozilla.org/mozilla-central/rev/ac688f61b766
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Upstream PR merged
Assignee: nobody → james
You need to log in before you can comment on or make changes to this bug.