Bug 88003

Summary:

Chromium Console.time() and timeEnd() Should Support Async Traces

Product:

WebKit

Reporter:

Kevin Greer <kgr>

Component:

WebCore JavaScript

Assignee:

Kevin Greer <kgr>

Status:

RESOLVED FIXED

Severity:

Minor

CC:

danakj, jamesr, jbates, kgr, nduca, pfeldman, rbyers, rniwa, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description Kevin Greer 2012-05-31 12:21:57 PDT

When the Console.time/timerEnd() functions are called on the chromium platform, calls to chrome's TRACE_EVENT methods are called.
The versions being called only supported nested synchronous traces and should be switched to the versions that also support asynchronous un-nested traces.

Comment 1 Kevin Greer 2012-06-02 10:46:26 PDT

Created attachment 145456 [details]
Patch

Comment 2 Nat Duca 2012-06-04 12:33:41 PDT

Comment on attachment 145456 [details]
Patch

LGTM. +pfeldman for review.

Comment 3 WebKit Review Bot 2012-06-04 12:52:22 PDT

Comment on attachment 145456 [details]
Patch

Clearing flags on attachment: 145456

Committed r119418: <http://trac.webkit.org/changeset/119418>

Comment 4 WebKit Review Bot 2012-06-04 12:52:26 PDT

All reviewed patches have been landed.  Closing bug.

Comment 5 Dana Jansens 2012-06-04 15:48:32 PDT

This seems to have caused a failure in the browser_tests test GpuFeatureTest.RafNoDamage:

[12655:-1597528768:0604/134340:2263335561903:ERROR:trace_event_analyzer.cc(827)] Not enough events: 0
../../chrome/test/gpu/gpu_feature_browsertest.cc:425: Failure
Value of: trace_analyzer::GetRateStats(events, &stats, &stats_options)
  Actual: false
Expected: true
[12655:-1597528768:0604/134340:2263335777689:INFO:gpu_feature_browsertest.cc(427)] Number of RAFs: 0 Mean: 6.36069e-307 Min: -1.99068 Max: 2.56952e-256 StdDev: 2.57917e-256
../../chrome/test/gpu/gpu_feature_browsertest.cc:435: Failure
Expected: (stats.mean_us) > (15000.0), actual: 6.36069e-307 vs 15000


I bisected locally to find this change being the cause.

Comment 6 Ryosuke Niwa 2012-06-04 16:06:46 PDT

Could you look into this issue? I'm going to roll out the patch I don't hear back from you in the next half an hour or so.

Comment 7 James Robinson 2012-06-04 16:13:59 PDT

Looks like the chromium-side test should be updated to look for async trace events instead of normal matched ones.

Please don't revert, this patch is fine.

Comment 8 Ryosuke Niwa 2012-06-04 16:15:44 PDT

(In reply to comment #7)
> Looks like the chromium-side test should be updated to look for async trace events instead of normal matched ones.
> 
> Please don't revert, this patch is fine.

Okay. Could you make that change? I have no idea what this test is testing, and don't know how to do that.

Comment 9 John Bates 2012-06-04 16:21:58 PDT

Fix on the way:
https://code.google.com/p/chromium/issues/detail?id=131097

Comment 10 Ryosuke Niwa 2012-06-04 16:25:06 PDT

(In reply to comment #9)
> Fix on the way:
> https://code.google.com/p/chromium/issues/detail?id=131097

Great! Thank you.