Use the proper MallocSizeOf machinery for WebRender

RESOLVED FIXED in Firefox 66

Status

()

enhancement
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

I've been doing the memory reporters by hand to avoid trait coherency issues, and it's worked ok, but it starts to fall down on the interners, which are quite complex. We can solve this problem with a tiny bit of code duplication - making a reduced fork of malloc_size_of to bundle with WebRender, and I think that's the right tradeoff.

Patches coming shortly.

Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f3ef205b4f63
Put malloc_size_of_derive on crates.io, and add a reduced fork of malloc_size_of for WebRender. r=emilio
https://hg.mozilla.org/integration/autoland/rev/e9b1d04247de
Hook up MallocSizeOf and use it to replace some manual reporting. r=emilio
https://hg.mozilla.org/integration/autoland/rev/054028a8d4a7
Implement MallocSizeOf for Interner and DataStore. r=emilio
https://hg.mozilla.org/integration/autoland/rev/21d74c03c00a
Use a macro to declare interners and hook up memory reporters. r=emilio

Backed out 4 changesets (Bug 1519454) for api.rs build bustage.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=21d74c03c00a986914f5dc6ad44e5027b186f049&selectedJob=221513189

Backout link: https://hg.mozilla.org/integration/autoland/rev/5d2e7b3ecb63f0770d409abd397b93af5c0b3fd7

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=221512675&repo=autoland&lineNumber=183

[task 2019-01-12T19:32:44.724Z] Checking files for tidiness...
[task 2019-01-12T19:32:44.724Z]
[task 2019-01-12T19:32:44.724Z] Progress: 0% (2/862)
[task 2019-01-12T19:32:44.725Z] Progress: 0% (3/862)
[task 2019-01-12T19:32:44.725Z] Progress: 0% (4/862)
[task 2019-01-12T19:32:44.725Z] Progress: 0% (5/862)
[task 2019-01-12T19:32:44.725Z] Progress: 0% (6/862)
[task 2019-01-12T19:32:44.725Z] Progress: 0% (7/862)
[task 2019-01-12T19:32:44.725Z] Progress: 0% (8/862)
[task 2019-01-12T19:32:44.725Z] Progress: 1% (9/862)
[task 2019-01-12T19:32:44.725Z] Progress: 1% (10/862)
[task 2019-01-12T19:32:44.769Z] Progress: 1% (11/862)
[task 2019-01-12T19:32:44.782Z] Progress: 1% (12/862)
[task 2019-01-12T19:32:44.860Z] Progress: 1% (13/862)
[task 2019-01-12T19:32:44.873Z] Progress: 1% (14/862)
[task 2019-01-12T19:32:44.877Z] Progress: 1% (15/862)
[task 2019-01-12T19:32:44.909Z] Progress: 1% (16/862)
[task 2019-01-12T19:32:48.943Z] Progress: 1% (17/862)
[task 2019-01-12T19:32:48.947Z] Progress: 2% (18/862)
[task 2019-01-12T19:32:48.947Z] ./webrender_api/src/api.rs:835: Line is longer than 120 characters
[task 2019-01-12T19:32:49.065Z]
[task 2019-01-12T19:32:49.070Z] Progress: 2% (19/862)
[task 2019-01-12T19:32:49.078Z] Progress: 2% (20/862)
[task 2019-01-12T19:32:49.086Z] Progress: 2% (21/862)
[task 2019-01-12T19:32:49.206Z] Progress: 2% (22/862)
[task 2019-01-12T19:32:49.213Z] Progress: 2% (23/862)
[task 2019-01-12T19:32:49.214Z] Progress: 2% (24/862)
[task 2019-01-12T19:32:49.214Z] Progress: 2% (25/862)
[task 2019-01-12T19:32:49.214Z] Progress: 3% (26/862)
[task 2019-01-12T19:32:49.214Z] Progress: 3% (27/862)
[task 2019-01-12T19:32:49.215Z] Progress: 3% (28/862)
[task 2019-01-12T19:32:49.215Z] Progress: 3% (29/862)
[task 2019-01-12T19:32:49.215Z] Progress: 3% (30/862)

Flags: needinfo?(bobbyholley)

Hm, try push in comment 6 was green. Looks like the tidy job doesn't get run with the syntax suggested at [1]?

[1]

Flags: needinfo?(bobbyholley) → needinfo?(kats)
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d5e192fd022f
Put malloc_size_of_derive on crates.io, and add a reduced fork of malloc_size_of for WebRender. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ed1842f02fe2
Hook up MallocSizeOf and use it to replace some manual reporting. r=emilio
https://hg.mozilla.org/integration/autoland/rev/388b2d191167
Implement MallocSizeOf for Interner and DataStore. r=emilio
https://hg.mozilla.org/integration/autoland/rev/33ca54452d38
Use a macro to declare interners and hook up memory reporters. r=emilio

Your query was "webrender linux" but the tidy job name doesn't contain the "linux" string. The query on the wiki page does what it says, but it wasn't what you wanted/expected. We could change the job name to include the "linux" string somewhere if this will make life easier.

https://searchfox.org/mozilla-central/rev/7d7aca6a935f0725f35050fdaa3dd7a1b05d8d38/taskcluster/ci/webrender/kind.yml#27

Flags: needinfo?(kats)

(In reply to Kartikaya Gupta (away Jan 12-26; email:kats@mozilla.com) from comment #12)

Your query was "webrender linux" but the tidy job name doesn't contain the "linux" string. The query on the wiki page does what it says, but it wasn't what you wanted/expected. We could change the job name to include the "linux" string somewhere if this will make life easier.

https://searchfox.org/mozilla-central/rev/7d7aca6a935f0725f35050fdaa3dd7a1b05d8d38/taskcluster/ci/webrender/kind.yml#27

Ah I see - linux was busted, and so I re-ran just the linux jobs, but the tidy job isn't officially a linux job even though it busts when linux busts.

So that's probably pretty edge-casey, and not worth worrying about. Thanks for explaining!

(In reply to Bobby Holley (:bholley) from comment #14)

the tidy job isn't officially a linux job

In a manner of speaking, yes, since the things it's testing are not specific to any platform. This is the reason I didn't put "linux" in the job name. But on the other hand I have to have it in some row on TH and so I threw it under the "Linux x64 QuantumRender" row.

even though it busts when linux busts.

Just to be clear it busted in your try push in comment 1 because of the lint failure, not because the Linux build was busted. The error summary on TH doesn't show the lint error but it's in the full job log. I filed bug 1519657 for improving the error summary situation in this and other cases.

Oh I see - I missed that. So that's all on me, though bug 1519657 should certainly help.

Comment on attachment 9044595 [details]
Bug 1519454 - Make webrender_build publishable. r=kvark

Revision D20134 was moved to bug 1527884. Setting attachment 9044595 [details] to obsolete.

Attachment #9044595 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.