Created attachment 252736 [details] [TEST] profile test page * SUMMARY No profile information is shown in Inspector. Recording a profile adds a profile parent record to the JavaScript & Events timeline, but it has no child records. * STEPS TO REPRODUCE 1. Inspect attached test page (profile-test.html) 2. Begin a new timeline recording from the Inspector. 3. Run the test in test page 4. Select the JavaScript & Events timeline, expand all records. => "Test" Profile Recorded is visible, but it has no child records (see attached screenshot).
<rdar://problem/20622499>
Created attachment 252737 [details] [IMAGE] missing profile records
Created attachment 252764 [details] [Patch] WIP Ready for feedback, not ready to land
Created attachment 252765 [details] [Image] timing mismatch When a profile is started after recording has already begun, there will be two entries for each profiled function call: one for the outer function that called console.profile, and a second for the profile itself. I noticed that the total, self, and average times reported for the profiled functions are different for the two cases described above (see screenshot). This seems like a bug, since the profiled code executed only once. The start times also differ.
Comment on attachment 252764 [details] [Patch] WIP Ugh, I regret messing with this code :) It seems to me that we aren't gaining much by using independent stopwatches at all. The only way it would make sense in the UI is if console profiles created a new Timeline recording, but I'm not sure the tiny number of users for this function justifies the complexity. The start time doesn't show up as "0" even with how the code is now. Is it possible to always use the shared stopwatch?
Comment on attachment 252764 [details] [Patch] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=252764&action=review Patch is OK, but there are two issues we should fix. > Source/JavaScriptCore/profiler/Profile.h:32 > +#include <wtf/Stopwatch.h> Should not include the header. Just need a forward declaration here. > Source/JavaScriptCore/profiler/Profile.h:39 > + static PassRefPtr<Profile> create(const String& title, unsigned uid, PassRefPtr<Stopwatch>); New code should not use PassRefPtr. See <http://www.webkit.org/coding/RefPtr.html>. In this case, the argument should be a Stopwatch& because the function does not take ownership of it and it can’t be null.
> > Source/JavaScriptCore/profiler/Profile.h:39 > > + static PassRefPtr<Profile> create(const String& title, unsigned uid, PassRefPtr<Stopwatch>); > > New code should not use PassRefPtr. See > <http://www.webkit.org/coding/RefPtr.html>. In this case, the argument > should be a Stopwatch& because the function does not take ownership of it > and it can’t be null. Now I'm thinking ProfileGenerator should just sample its stopwatch and pass the start time to Profile::create, eliminating the need for a Stopwatch reference entirely.
Created attachment 252832 [details] [Patch] Proposed Fix
Comment on attachment 252832 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=252832&action=review Looks OK. > Source/WebCore/inspector/InspectorTimelineAgent.cpp:260 > + startProfiling(exec, title, m_instrumentingAgents->inspectorEnvironment().executionStopwatch()); This should change from a double to a std::chrono at some point.
Comment on attachment 252832 [details] [Patch] Proposed Fix Clearing flags on attachment: 252832 Committed r184117: <http://trac.webkit.org/changeset/184117>
All reviewed patches have been landed. Closing bug.