WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
127039
Remove the CSS selector profiler.
https://bugs.webkit.org/show_bug.cgi?id=127039
Summary
Remove the CSS selector profiler.
Andreas Kling
Reported
2014-01-15 02:45:04 PST
The selector profiler was painting a mostly fictional picture of what selectors we were spending time on. It never really grokked the fast path selectors, nor did it understand recent additions like the extra cascading pass or the selector JIT. Somewhat ironically, this may end up making some selectors run faster since it removes a number of brances in hot code. FWIW, Blink removed theirs here <
https://codereview.chromium.org/21049007
>
Attachments
Patch
(40.14 KB, patch)
2014-01-15 02:50 PST
,
Andreas Kling
no flags
Details
Formatted Diff
Diff
Patch with removed files
(54.32 KB, patch)
2014-01-15 02:56 PST
,
Andreas Kling
sam
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2014-01-15 02:45:26 PST
<
rdar://problem/15823608
>
Andreas Kling
Comment 2
2014-01-15 02:50:10 PST
Created
attachment 221253
[details]
Patch
Andreas Kling
Comment 3
2014-01-15 02:56:54 PST
Created
attachment 221255
[details]
Patch with removed files
Antti Koivisto
Comment 4
2014-01-15 03:06:58 PST
Comment on
attachment 221255
[details]
Patch with removed files Selector profiles are somewhere between useless and actively harmful unless you know a ton about engine internals. Removal is a good idea.
Timothy Hatcher
Comment 5
2014-01-15 11:26:36 PST
Sounds good. I never liked it either.
Joseph Pecoraro
Comment 6
2014-01-15 11:34:08 PST
Comment on
attachment 221255
[details]
Patch with removed files View in context:
https://bugs.webkit.org/attachment.cgi?id=221255&action=review
> Source/WebInspectorUI/UserInterface/CSSSelectorProfileType.js:-61 > - return WebInspector.UIString("CSS selector profiles show how long the selector matching has taken in total and how many times a certain selector has matched DOM elements (the results are approximate due to matching algorithm optimizations.)");
Nit: This patch removes localized strings but does not update localizedStrings.js. To automatically update the localized strings you can run: shell> ./Tools/Scripts/extract-localizable-js-strings ./Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js ./Source/WebInspectorUI/UserInterface There is a basic script you can run, something like ./Tools/Scripts/update-localizable-strings, but the command above just targets WebInspectorUI strings and takes ~1 second. I'll update the strings now. Good job on this removal! I think you covered everything!
Joseph Pecoraro
Comment 7
2014-01-15 11:35:39 PST
(In reply to
comment #6
)
> I'll update the strings now.
Actually, this hasn't landed yet! So there is time for you to update the strings =)
Andreas Kling
Comment 8
2014-01-15 12:05:14 PST
Committed
r162084
: <
http://trac.webkit.org/changeset/162084
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug