We should have memory usage logging alongside the CPU usage logging for telemetry.
Created attachment 299446 [details] Patch for EWS
Created attachment 299449 [details] Patch for EWS
Created attachment 299498 [details] Patch for EWS II
Created attachment 299501 [details] Patch for EWS 4
Created attachment 299540 [details] Patch
Does not build in Debug.
Created attachment 299545 [details] Patch
Comment on attachment 299545 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299545&action=review > Source/WebCore/page/PerformanceMonitor.cpp:177 > + m_postBackgroundingCPUTime = std::nullopt; muhahaha, you're resetting my CPU variables in your memory code. > Source/WebCore/page/PerformanceMonitor.cpp:273 > +#if !RELEASE_LOG_DISABLED This does not look needed? > Source/WebCore/page/cocoa/PerformanceLoggingCocoa.mm:39 > + if (err != KERN_SUCCESS) err != success look weird. How about we call it result? > Source/WebKit2/UIProcess/PerActivityStateMemoryUsageSampler.h:37 > +class PerActivityStateMemoryUsageSampler { I would have merged this class with mine. PerActivityStatePerformanceSampler? Why didn't you do it?
Comment on attachment 299545 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299545&action=review > Source/WebKit2/UIProcess/PerActivityStateMemoryUsageSampler.cpp:58 > +static inline String loggingKeyForActivityState(ActivityStateForPerformanceMonitoring state) This is duplicated from my code. > Source/WebKit2/UIProcess/PerActivityStateMemoryUsageSampler.cpp:88 > +WebPageProxy* PerActivityStateMemoryUsageSampler::pageForLogging() const This is duplicated from my code. > Source/WebKit2/UIProcess/WebPageProxy.cpp:5185 > + fprintf(stderr, "DLog: %s ~ %s ~ %s\n", message.ascii().data(), description.ascii().data(), value.ascii().data()); I guess you did not mean to leave this in? :)
Comment on attachment 299545 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299545&action=review >> Source/WebKit2/UIProcess/PerActivityStateMemoryUsageSampler.h:37 >> +class PerActivityStateMemoryUsageSampler { > > I would have merged this class with mine. PerActivityStatePerformanceSampler? Why didn't you do it? Or maybe WebPeformanceMetricsSampler
(In reply to comment #8) > Comment on attachment 299545 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299545&action=review > > > Source/WebCore/page/PerformanceMonitor.cpp:177 > > + m_postBackgroundingCPUTime = std::nullopt; > > muhahaha, you're resetting my CPU variables in your memory code. > > > Source/WebCore/page/PerformanceMonitor.cpp:273 > > +#if !RELEASE_LOG_DISABLED > > This does not look needed? Ugh, copy/pasting code gone too far. :| > > Source/WebCore/page/cocoa/PerformanceLoggingCocoa.mm:39 > > + if (err != KERN_SUCCESS) > > err != success look weird. How about we call it result? Sure! That would read nicer. > > Source/WebKit2/UIProcess/PerActivityStateMemoryUsageSampler.h:37 > > +class PerActivityStateMemoryUsageSampler { > > I would have merged this class with mine. > PerActivityStatePerformanceSampler? Why didn't you do it? I'll merge them. And take out that printf :)
Created attachment 299567 [details] Patch
Created attachment 299569 [details] Patch
Comment on attachment 299569 [details] Patch Attachment 299569 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2938633 New failing tests: media/modern-media-controls/tracks-support/tracks-support-show-panel-fullscreen.html
Created attachment 299571 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Okay now that I've had some sleep & coffee let me actually fix up this patch properly :|
Created attachment 299572 [details] Patch Okay, now without the WK2 part since that logging didn't even make sense (combined footprints looked goofy and what am I gonna do with them anyway) Also without the enum renames and stuff.
Comment on attachment 299572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299572&action=review r=me with comment > Source/WebCore/page/PerformanceMonitor.cpp:90 > + m_postPageLoadMemoryUsageTimer.startOneShot(memoryUsageMeasurementDelay); You should probably stop this timer if a new Page load starts.
Committed r211120: <http://trac.webkit.org/changeset/211120>