We need to track V8DOMWindowShell::clearForNavigation time to be able to quickly detect such performance issues as http://code.google.com/p/chromium/issues/detail?id=155270
Looking at the callers, I wonder if we actually want to instrument ScriptController::clearForNavigation. That seems to cover more of the code that we call pre navigating. I don't know this code at all though.
Digging a bit further, it looks like V8DOMWindowShell::clearForNavigation is only ever called by ScriptController::clearForNavigation, which is only ever called by ScriptController::clearWindowShell. Can we kill ScriptController::clearForNavigation?
Oh, I see. There's also a clearForClose. I suppose it's better self-documenting code?
Created attachment 170685 [details] Patch
Comment on attachment 170685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170685&action=review > Source/WebCore/bindings/v8/ScriptController.cpp:119 > + WebKit::Platform::current()->histogramCustomCounts("ScriptController::~ScriptController", (currentTime() - start) * 1000, 0, 10000, 50); We have helper functions in HistogramSupport.h so that other ports can add their histogram backends if they desire.
Comment on attachment 170685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170685&action=review > Source/WebCore/bindings/v8/ScriptController.cpp:162 > + WebKit::Platform::current()->histogramCustomCounts("ScriptController::reset", (currentTime() - start) * 1000, 0, 10000, 50); Is reset called from places other than clearForClose and clearWindowShell? It's a bit strange that you'll have several of these timers running at once. For example, in ~ScriptController, it looks like you'll have three running.
Comment on attachment 170685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170685&action=review >> Source/WebCore/bindings/v8/ScriptController.cpp:119 >> + WebKit::Platform::current()->histogramCustomCounts("ScriptController::~ScriptController", (currentTime() - start) * 1000, 0, 10000, 50); > > We have helper functions in HistogramSupport.h so that other ports can add their histogram backends if they desire. Oh right. I'm a bit braindead this morning clearly. >> Source/WebCore/bindings/v8/ScriptController.cpp:162 >> + WebKit::Platform::current()->histogramCustomCounts("ScriptController::reset", (currentTime() - start) * 1000, 0, 10000, 50); > > Is reset called from places other than clearForClose and clearWindowShell? It's a bit strange that you'll have several of these timers running at once. For example, in ~ScriptController, it looks like you'll have three running. It's true. I could drop the reset one for now as it is a little redundant. Should I drop ~ScriptController too? The only extra thing that really measures is windowShell()->destroyGlobal().
(In reply to comment #7) > (From update of attachment 170685 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=170685&action=review > > >> Source/WebCore/bindings/v8/ScriptController.cpp:162 > >> + WebKit::Platform::current()->histogramCustomCounts("ScriptController::reset", (currentTime() - start) * 1000, 0, 10000, 50); > > > > Is reset called from places other than clearForClose and clearWindowShell? It's a bit strange that you'll have several of these timers running at once. For example, in ~ScriptController, it looks like you'll have three running. > > It's true. I could drop the reset one for now as it is a little redundant. Should I drop ~ScriptController too? The only extra thing that really measures is windowShell()->destroyGlobal(). Now that I look, destroyGlobal literally just clears m_global. I'll remove this histogram too.
Created attachment 170713 [details] Patch
Comment on attachment 170713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170713&action=review > Source/WebCore/bindings/v8/ScriptController.cpp:162 > + HistogramSupport::histogramCustomCounts("ScriptController::clearForClose", (currentTime() - start) * 1000, 0, 10000, 50); We usually use names more like WebCore.ScriptController.ClearForClose for histograms.
Committed r132517: <http://trac.webkit.org/changeset/132517>