performance.mark() is ignored in Timeline if it happens too fast
Reported by
dan.abra...@gmail.com,
Aug 24 2016
|
||||||||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36
Steps to reproduce the problem:
Run this code with Timeline recording:
function doSomething() { }
for (var i = 0; i < 10000; i++) {
var startName = i + '-start'
var endName = i + '-end'
performance.mark(startName)
doSomething()
performance.mark(endName)
performance.measure(i, startName, endName)
}
What is the expected behavior?
Each event should appear separately without any overlapping between events.
What went wrong?
When there is too little time before performance.mark()ers that are used as beginning and end in performance.measure(), the end marker appears ignored, and the event “lasts forever”.
Did this work before? N/A
Chrome version: 52.0.2743.116 Channel: stable
OS Version: OS X 10.10.5
Flash Version: Shockwave Flash 22.0 r0
,
Aug 24 2016
Good bug, thanks so much for reporting. Turned out an unstable sort was at fault here. Fix is on the way.
,
Aug 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7465824d77013dd37242713c7a2520be2b718345 commit 7465824d77013dd37242713c7a2520be2b718345 Author: alph <alph@chromium.org> Date: Thu Aug 25 19:27:25 2016 DevTools: Use stableSort for async events Async events may have identicatl start and end time. Also make sure async events go the the right strip, not to input one. BUG= 640652 Review-Url: https://codereview.chromium.org/2276743004 Cr-Commit-Position: refs/heads/master@{#414506} [modify] https://crrev.com/7465824d77013dd37242713c7a2520be2b718345/third_party/WebKit/Source/devtools/front_end/sdk/TracingModel.js [modify] https://crrev.com/7465824d77013dd37242713c7a2520be2b718345/third_party/WebKit/Source/devtools/front_end/timeline/TimelineFlameChart.js
,
Aug 25 2016
,
Aug 25 2016
We're very close to M53 stable launch. We can take this change only if it is well baked/verified in canary, safe to merge and critical for M53.
,
Aug 26 2016
[Automated comment] Less than 2 weeks to go before stable on M53, manual review required.
,
Aug 26 2016
The fix is quite safe. It basically changes the sort to stableSort. I verified that it works in todays Canary.
,
Aug 26 2016
Approving merge to M53 branch 2785 based on comment #7. Please merge ASAP (Merge has to happen before 4:00 PM PT Monday (08/29) in order to make into the desktop Stable final build cut). Thank you
,
Aug 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6cf21189f1f10bf531d8f91f1ac484ae22b4e32f commit 6cf21189f1f10bf531d8f91f1ac484ae22b4e32f Author: Alexei Filippov <alph@chromium.org> Date: Fri Aug 26 21:41:00 2016 DevTools: Use stableSort for async events Async events may have identicatl start and end time. Also make sure async events go the the right strip, not to input one. BUG= 640652 Review-Url: https://codereview.chromium.org/2276743004 Cr-Commit-Position: refs/heads/master@{#414506} (cherry picked from commit 7465824d77013dd37242713c7a2520be2b718345) Review URL: https://codereview.chromium.org/2283143002 . Cr-Commit-Position: refs/branch-heads/2785@{#765} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/6cf21189f1f10bf531d8f91f1ac484ae22b4e32f/third_party/WebKit/Source/devtools/front_end/sdk/TracingModel.js [modify] https://crrev.com/6cf21189f1f10bf531d8f91f1ac484ae22b4e32f/third_party/WebKit/Source/devtools/front_end/timeline/TimelineFlameChart.js
,
Aug 26 2016
,
Aug 31 2016
While trying to verify the merge on the latest M-53(53.0.2785.89) on Mac OS 10.11.6 and comparing with the M-53 builds(53.0.2785.6) without the Fix, didn't observe much of difference. Attached screenshot from 53.0.2785.6(build without the Fix) Steps tried: ============ 1. Timeline recording started. 2. Pasted the above code in the the console and ENTER. Not seeing the PerformanceMark UI on M-53 as attached in the above screenshots. alph@: Could you please help in verifying the merge on the latest M-53(53.0.2785.89) and provide detailed steps to verify this. Appreciate your help!
,
Aug 31 2016
We've merged this to 53 which we'll see as stable next week. |
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by igrigo...@chromium.org
, Aug 24 2016