Bug 135467

Summary:

Web Inspector: message dispatch metrics should use high-resolution timing data

Product:

WebKit

Reporter:

Brian Burg <burg>

Component:

Web Inspector

Assignee:

Blaze Burg <bburg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, buildbot, graouts, joepeck, rniwa, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

42434

Bug Blocks:

147066

Attachments:

Description	Flags
Proposed Fix	none
Archive of layout-test-results from ews101 for mac-mavericks	none
Archive of layout-test-results from ews107 for mac-mavericks-wk2	none
Proposed Fix	timothy: review+

Brian Burg

Reported 2014-07-31 12:45:15 PDT

in particular, things controlled by InspectorBackend.dumpInspectorTimeStats

Attachments
Proposed Fix (9.71 KB, patch) 2015-08-25 15:14 PDT, Blaze Burg	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews101 for mac-mavericks (1.67 MB, application/zip) 2015-08-25 16:07 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (661.75 KB, application/zip) 2015-08-25 16:11 PDT, Build Bot	no flags	Details
Proposed Fix (9.26 KB, patch) 2015-08-25 16:16 PDT, Blaze Burg	timothy: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2014-07-31 12:46:35 PDT

<rdar://problem/17874162>

Blaze Burg

Comment 2 2015-08-25 15:14:52 PDT

Created attachment 259888 [details] Proposed Fix

Joseph Pecoraro

Comment 3 2015-08-25 15:46:39 PDT

Comment on attachment 259888 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=259888&action=review Looks good to me. cq- so you don't enable it by default. > Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:44 > - this.dumpInspectorTimeStats = false; > + this.dumpInspectorTimeStats = true; > this.dumpInspectorProtocolMessages = false; > - this.warnForLongMessageHandling = false; > + this.warnForLongMessageHandling = true; I don't think you want to commit these. Note, you could write a test for these if you wanted. You would just enable these dump/warn flags at the start then trigger some messages. But I'm not sure that would be too valuable.

Joseph Pecoraro

Comment 4 2015-08-25 15:48:12 PDT

I wonder how our jsmin.py is doing minifying with new syntax (template strings, arrow functions, etc). I wonder if the first time we have a newline in a template string if we will run into issues.

Build Bot

Comment 5 2015-08-25 16:07:23 PDT

Comment on attachment 259888 [details] Proposed Fix Attachment 259888 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/106488 Number of test failures exceeded the failure limit.

Build Bot

Comment 6 2015-08-25 16:07:25 PDT

Created attachment 259894 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5

Blaze Burg

Comment 7 2015-08-25 16:09:53 PDT

(In reply to comment #4) > I wonder how our jsmin.py is doing minifying with new syntax (template > strings, arrow functions, etc). I wonder if the first time we have a newline > in a template string if we will run into issues. We already have some, I think.

Build Bot

Comment 8 2015-08-25 16:11:30 PDT

Comment on attachment 259888 [details] Proposed Fix Attachment 259888 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/106480 Number of test failures exceeded the failure limit.

Build Bot

Comment 9 2015-08-25 16:11:33 PDT

Created attachment 259896 [details] Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5

Blaze Burg

Comment 10 2015-08-25 16:16:14 PDT

Created attachment 259897 [details] Proposed Fix

Timothy Hatcher

Comment 11 2015-08-25 17:10:12 PDT

Comment on attachment 259888 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=259888&action=review > Source/WebInspectorUI/UserInterface/Base/Utilities.js:1129 > +function timestampMS() Just timestamp()? > Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:68 > - if (this._dispatchTimeout) > + if (WebInspector._dispatchTimeout) I think we should use this. The other places should change.

Blaze Burg

Comment 12 2015-08-25 23:30:03 PDT

Committed r188946: <http://trac.webkit.org/changeset/188946>

Note You need to log in before you can comment on or make changes to this bug.