Closed Bug 1519454 Opened 6 years ago Closed 6 years ago

Use the proper MallocSizeOf machinery for WebRender

Categories

(Core :: Graphics: WebRender, enhancement)

Product:
Core
Component:
Graphics: WebRender
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla66
Tracking Flags:
Tracking Status
firefox66 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(4 files, 1 obsolete file)

Bug 1519454 - Put malloc_size_of_derive on crates.io, and add a reduced fork of malloc_size_of for WebRender.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1519454 - Hook up MallocSizeOf and use it to replace some manual reporting.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1519454 - Implement MallocSizeOf for Interner and DataStore.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1519454 - Use a macro to declare interners and hook up memory reporters.
47 bytes, text/x-phabricator-request
Details | Review

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.

Attachment

General

Creator:
Created:
Updated:
Size: