Closed Bug 1760920 Opened 2 years ago Closed 2 years ago

Fix multiple problems with counting memory for about:performance

Categories

(Toolkit :: Performance Monitoring, defect, P1)

defect

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox108 --- fixed

People

(Reporter: pbone, Assigned: pbone)

References

Details

Attachments

(16 files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

I think that there's a problem with how the about:performance page collects memory information. It collects from a browsing context's wrapper. Which doesn't always exist meaning any JS memory is not counted. I think it's better to find the document's global object and the JS zone associated with that.

I want to remove CollectMemoryInfo(BrowsingContext*, _) and have all callers
use the one interface with document groups. I'm not sure how correct this
WorkerDebugger code is, hopefully this is no worse.

Remove this 2nd unnecessary code path.

Depends on D146674

Since the caller usually needs to know the zone anyway, this seems simpler
if we query based on the zone rather than the object.

Depends on D146676

Summary: Capture JS memory usage from the document global → Fix multiple problems with counting memory for about:performance
Attachment #9277072 - Attachment description: Bug 1760920 - Don't count shared memory more than once r=jonco → Bug 1760920 - pt 7. Don't count shared memory more than once r=jonco

This function is only called from this .cpp file so it can be static.

Depends on D146894

Attachment #9277066 - Attachment description: Bug 1760920 - pt 1. Worker debugger can query the doc group r=asuth → Bug 1760920 - pt 1. Worker debugger can query the doc group r=asuth,jonco
Attachment #9277450 - Attachment description: Bug 1760920 - pt 8. Count only the media resources belonging to the document r=nika → Bug 1760920 - pt 8. Count only the media resources belonging to the document r=padenot
Attachment #9277066 - Attachment description: Bug 1760920 - pt 1. Worker debugger can query the doc group r=asuth,jonco → Bug 1760920 - pt 2. Worker debugger can query the doc group r=asuth,jonco
Attachment #9277066 - Attachment description: Bug 1760920 - pt 2. Worker debugger can query the doc group r=asuth,jonco → Bug 1760920 - pt 2b. Worker debugger can query the doc group r=asuth
Attachment #9277067 - Attachment description: Bug 1760920 - pt 2. Fix potential double-counting windows r=nika → Bug 1760920 - pt 3. Fix potential double-counting windows r=nika
Attachment #9277068 - Attachment description: Bug 1760920 - pt 3. Remove CollectMemoryInfo(RefPtr<BrowsingContext> &, _) r=nika → Bug 1760920 - pt 4. Remove CollectMemoryInfo(RefPtr<BrowsingContext> &, _) r=nika
Attachment #9277069 - Attachment description: Bug 1760920 - pt 4. Include memory usage from JS's malloc and JIT usage r=jonco,nika → Bug 1760920 - pt 5. Include memory usage from JS's malloc and JIT usage r=jonco,nika
Attachment #9277070 - Attachment description: Bug 1760920 - pt 5. Capture memory usage from the document global r=nika → Bug 1760920 - pt 6. Capture memory usage from the document global r=nika
Attachment #9277071 - Attachment description: Bug 1760920 - pt 6. Query memory per zone r=jonco → Bug 1760920 - pt 8. Query memory per zone r=jonco
Attachment #9277072 - Attachment description: Bug 1760920 - pt 7. Don't count shared memory more than once r=jonco → Bug 1760920 - pt 9. Don't count shared memory more than once r=jonco
Attachment #9277450 - Attachment description: Bug 1760920 - pt 8. Count only the media resources belonging to the document r=padenot → Bug 1760920 - pt 10. Count only the media resources belonging to the document r=padenot
Attachment #9277451 - Attachment description: Bug 1760920 - pt 9. Remove dead GetTabSizes function r=nika → Bug 1760920 - pt 11. Remove dead GetTabSizes function r=nika
Attachment #9277452 - Attachment description: Bug 1760920 - pt 10. Make AddWindowTabSizes static r=nika → Bug 1760920 - pt 12. Make AddWindowTabSizes static r=nika
Attachment #9283073 - Attachment description: Bug 1760920 - pt 11. Count all the JS memory for web workers r=jonco → Bug 1760920 - pt 13. Count all the JS memory for web workers r=jonco

Sometimes this entry can be at the bottom of the table and therefore there's
no nextSibling.

Depends on D160341

Pushed by pbone@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b589f7b55a27
pt 1. Test about:performance with web workers r=florian
https://hg.mozilla.org/integration/autoland/rev/274a277dbb73
pt 2a. Provide GetGCHeapUsage r=jonco
https://hg.mozilla.org/integration/autoland/rev/1400576fd80d
pt 2b. Worker debugger can query the doc group r=dom-worker-reviewers,asuth,smaug
https://hg.mozilla.org/integration/autoland/rev/9ad856e68af9
pt 3. Fix potential double-counting windows r=nika
https://hg.mozilla.org/integration/autoland/rev/41f28233dff1
pt 4. Remove CollectMemoryInfo(RefPtr<BrowsingContext> &, _) r=nika
https://hg.mozilla.org/integration/autoland/rev/34aeb07a6f67
pt 5. Include memory usage from JS's malloc and JIT usage r=jonco,nika
https://hg.mozilla.org/integration/autoland/rev/7311c284f09f
pt 6. Capture memory usage from the document global r=nika
https://hg.mozilla.org/integration/autoland/rev/09290cd6713d
pt 7. Add a test for memory in about:performance r=florian
https://hg.mozilla.org/integration/autoland/rev/822684f1df76
pt 8. Query memory per zone r=jonco
https://hg.mozilla.org/integration/autoland/rev/711128371fc0
pt 9. Don't count shared memory more than once r=jonco
https://hg.mozilla.org/integration/autoland/rev/b7cd3ed1d21b
pt 10. Count only the media resources belonging to the document r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/d702485e2dbc
pt 11. Remove dead GetTabSizes function r=nika
https://hg.mozilla.org/integration/autoland/rev/e6635f916ac9
pt 12. Make AddWindowTabSizes static r=nika
https://hg.mozilla.org/integration/autoland/rev/cebb14585723
pt 13. Count all the JS memory for web workers r=jonco
https://hg.mozilla.org/integration/autoland/rev/9d30f7b5bce0
pt 14. Test that workers memory usage is reported r=florian
https://hg.mozilla.org/integration/autoland/rev/8f02fab27335
pt 15. Fix timing in a test leading to a failure r=florian
Regressions: 1800302
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: