Closed Bug 1377575 Opened 8 years ago Closed 8 years ago

Add telemetry for MemoryBlockCache memory usage watermark

Categories

(Core :: Audio/Video: Playback, enhancement)

56 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: mozbugz, Assigned: mozbugz)

References

Details

Attachments

(3 files)

MEDIACACHE_MEMORY_WATERMARK will record the highest memory usage of MemoryBlockCache per session.
Comment on attachment 8882713 [details] Bug 1377575 - No need to explicitly print 'this' from MemoryBlockCache logging - https://reviewboard.mozilla.org/r/153800/#review159026
Attachment #8882713 - Flags: review?(cpearce) → review+
Comment on attachment 8882714 [details] Bug 1377575 - Rename MemoryBlockCache's mCombinedSizes to gCombinedSizes - https://reviewboard.mozilla.org/r/153802/#review159028
Attachment #8882714 - Flags: review?(cpearce) → review+
Comment on attachment 8882715 [details] Bug 1377575 - MEDIACACHE_MEMORY_WATERMARK records the MemoryBlockCache memory use watermark - https://reviewboard.mozilla.org/r/153804/#review159030 ::: dom/media/MemoryBlockCache.cpp:81 (Diff revision 3) > + mozilla::services::GetObserverService(); > + if (observerService) { > + observerService->AddObserver( > + gMemoryBlockCacheTelemetry, "profile-change-teardown", true); > + } > + } Do you need to do a ClearOnShutdown(gMemoryBlockCacheTelementry) here so that the singleton is considered freed by the leak tracker? ::: dom/media/MemoryBlockCache.cpp:83 (Diff revision 3) > + observerService->AddObserver( > + gMemoryBlockCacheTelemetry, "profile-change-teardown", true); > + } > + } > + > + // Udpate watermark if needed, report current watermark. s/Udpate/Update/
Attachment #8882715 - Flags: review?(cpearce) → review+
Comment on attachment 8882715 [details] Bug 1377575 - MEDIACACHE_MEMORY_WATERMARK records the MemoryBlockCache memory use watermark - https://reviewboard.mozilla.org/r/153804/#review159184 datareview+
Attachment #8882715 - Flags: review?(francois) → review+
Comment on attachment 8882715 [details] Bug 1377575 - MEDIACACHE_MEMORY_WATERMARK records the MemoryBlockCache memory use watermark - https://reviewboard.mozilla.org/r/153804/#review159030 > Do you need to do a ClearOnShutdown(gMemoryBlockCacheTelementry) here so that the singleton is considered freed by the leak tracker? Good question. Looking at the documentation for ClearOnShutdown, I should indeed call it to ensure we don't leak gMemoryBlockCacheTelemetry. I was thinking of just resetting the pointer from Observe() when we get the "profile-change-teardown" notification, but we would risk re-creating it (and then leaking it) in case some MediaBlockCache work would racily happen after that notification. So I'll just add the ClearOnShutdown. > s/Udpate/Update/ Tahnks!
Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8b0acd667dc6 No need to explicitly print 'this' from MemoryBlockCache logging - r=cpearce https://hg.mozilla.org/integration/autoland/rev/5caa90b684f3 Rename MemoryBlockCache's mCombinedSizes to gCombinedSizes - r=cpearce https://hg.mozilla.org/integration/autoland/rev/4bdafa7daf7d MEDIACACHE_MEMORY_WATERMARK records the MemoryBlockCache memory use watermark - r=cpearce,francois
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: