Summary: | Web Inspector: Canvas tab: Determine isFunction by looking at the prototype | ||
---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> |
Component: | Web Inspector | Assignee: | Devin Rousso <hi> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | bburg, commit-queue, ews-watchlist, inspector-bugzilla-changes, rniwa, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | All | ||
OS: | All | ||
Bug Depends on: | 182995 | ||
Bug Blocks: | 173807 | ||
Attachments: |
Description
Devin Rousso
2018-04-25 12:51:17 PDT
Created attachment 338771 [details]
Patch
Comment on attachment 338771 [details] Patch Attachment 338771 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7530624 New failing tests: inspector/canvas/recording-2d.html Created attachment 339248 [details]
Archive of layout-test-results from ews103 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 338771 [details] Patch Attachment 338771 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7530960 New failing tests: inspector/canvas/recording-2d.html Created attachment 339251 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 338771 [details] Patch Attachment 338771 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7531248 New failing tests: inspector/canvas/recording-2d.html Created attachment 339258 [details]
Archive of layout-test-results from ews114 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 339276 [details]
Patch
Comment on attachment 339276 [details] Patch Attachment 339276 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7534477 New failing tests: http/tests/preload/onload_event.html Created attachment 339287 [details]
Archive of layout-test-results from ews200 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 339287 [details]
Archive of layout-test-results from ews200 for win-future
Unrelated failure
Comment on attachment 339276 [details]
Patch
r=me
How often is this code called? Does it affect performance to dynamically look up for every call, rather than using a fixed Set lookup?
(In reply to Brian Burg from comment #12) > How often is this code called? Does it affect performance to dynamically > look up for every call, rather than using a fixed Set lookup? `WI.RecoringAction.isFunctionForType` is called once per `WI.RecordingAction` (assuming it has been `swizzle()`d), and is called for each value in the `WI.Recording`'s `initialState.attributes`. I think a good middle-ground would be to keep the `WI.RecordingAction._functionNames` object, but have it be filled with values as each action is processed (sort of like a memoization). (In reply to Devin Rousso from comment #13) > (In reply to Brian Burg from comment #12) > > How often is this code called? Does it affect performance to dynamically > > look up for every call, rather than using a fixed Set lookup? > `WI.RecoringAction.isFunctionForType` is called once per > `WI.RecordingAction` (assuming it has been `swizzle()`d), and is called for > each value in the `WI.Recording`'s `initialState.attributes`. I think a > good middle-ground would be to keep the `WI.RecordingAction._functionNames` > object, but have it be filled with values as each action is processed (sort > of like a memoization). If you can capture a profile during the initial setup, and you can't see this code, then don't bother optimizing it. (In reply to Brian Burg from comment #14) > (In reply to Devin Rousso from comment #13) > > (In reply to Brian Burg from comment #12) > > > How often is this code called? Does it affect performance to dynamically > > > look up for every call, rather than using a fixed Set lookup? > > `WI.RecoringAction.isFunctionForType` is called once per > > `WI.RecordingAction` (assuming it has been `swizzle()`d), and is called for > > each value in the `WI.Recording`'s `initialState.attributes`. I think a > > good middle-ground would be to keep the `WI.RecordingAction._functionNames` > > object, but have it be filled with values as each action is processed (sort > > of like a memoization). > > If you can capture a profile during the initial setup, and you can't see > this code, then don't bother optimizing it. I was able to get a profile of it while recording the canvas on <http://acko.net> and it only took 0.1ms. The worst I've seen so far (100k actions) was 0.9ms. I think it's fine as is. Comment on attachment 339276 [details] Patch Clearing flags on attachment: 339276 Committed r231368: <https://trac.webkit.org/changeset/231368> All reviewed patches have been landed. Closing bug. |