| Summary: | LegacyProfiler: remove redundant ProfileNode members and other cleanup | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brian Burg <burg> | ||||
| Component: | JavaScriptCore | Assignee: | Brian Burg <burg> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, graouts, joepeck, mark.lam, timothy | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Bug Depends on: | 136462 | ||||||
| Bug Blocks: | 136352, 136381 | ||||||
| Attachments: |
|
||||||
|
Description
Brian Burg
2014-08-29 12:10:35 PDT
Created attachment 237418 [details]
Patch
Comment on attachment 237418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237418&action=review > Source/JavaScriptCore/profiler/ProfileNode.h:145 > - Vector<Call, 1> m_calls; > + Vector<Call> m_calls; Why this change? There will always be a minimum of 1 call, so it seems best to have storage for it instead of another malloc right away. Comment on attachment 237418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237418&action=review > Source/WebCore/inspector/ScriptProfile.idl:33 > - readonly attribute ScriptProfileNode head; > - readonly attribute unrestricted double idleTime; > + readonly attribute ScriptProfileNode rootNode; Will we need to change LegacyJavaScriptProfileView.js which still makes use of "profileNode.head" which no longer exists? (In reply to comment #3) > (From update of attachment 237418 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237418&action=review > > > Source/WebCore/inspector/ScriptProfile.idl:33 > > - readonly attribute ScriptProfileNode head; > > - readonly attribute unrestricted double idleTime; > > + readonly attribute ScriptProfileNode rootNode; > > Will we need to change LegacyJavaScriptProfileView.js which still makes use of "profileNode.head" which no longer exists? Maybe this was only used by tests? Comment on attachment 237418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237418&action=review >> Source/JavaScriptCore/profiler/ProfileNode.h:145 >> + Vector<Call> m_calls; > > Why this change? There will always be a minimum of 1 call, so it seems best to have storage for it instead of another malloc right away. For some reason, the compiler couldn't figure out how to pass a reference to m_calls if it had the default inline size. Maybe Vector was forward-declared instead of defined? I'll try again. >>> Source/WebCore/inspector/ScriptProfile.idl:33 >>> + readonly attribute ScriptProfileNode rootNode; >> >> Will we need to change LegacyJavaScriptProfileView.js which still makes use of "profileNode.head" which no longer exists? > > Maybe this was only used by tests? The tests were changed to use rootNode too. My understanding is that the legacy profiler views are only used when the inspected page speaks a legacy protocol. But, I have no way of testing this. Tim? Comment on attachment 237418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237418&action=review > Source/JavaScriptCore/profiler/ProfileNode.h:99 > - Vector<Call> calls() const { return m_calls; } > + const Vector<Call>& calls() const { return m_calls; } Oh, I see. This is what required the change of no default size. Hmm... LegacyJavaScriptProfile*.{js,css} can be removed. I punted on making it work after the profile migration to Timeline.
Committed r173199: <http://trac.webkit.org/changeset/173199> |