Startup regression in 59.0.3037.0 canary in Omnibox code |
|||
Issue descriptionThere was a small startup regression in 59.0.3037.0 canary in Omnibox code. I found this by trying to analyze improvements from my resource reordering changes. Looking at some graphs[1], it looked like there was a slight regression in 3037 canary and so I opened the sampling profiler data for this using the new "diff view" and indeed there was a clear change there[2]. Looks like OmniboxViewViews::EmphasizeURLComponents() takes 30ms more (due to calling into RenderText code). [1] https://uma.googleplex.com/p/chrome/timeline_v2/?sid=76c6d1b0134742e9f97be54634aa2840 [2] https://uma.googleplex.com/p/chrome/callstacks?sid=d2a69462294a0a4051cfb817b7a7025a Mark, assigning to you to take a look. Let me know if you don't have cycles for this and I can dig in instead.
,
Mar 27 2017
Thanks for reporting Alexei. This function and code sounds really familiar. I think something went by recently touching it. Aha, https://codereview.chromium.org/2734783007 is on the blamelist. Assigning to elawrence@, the author of that change. Is it expected that the painting takes this long? P.S. I also spotted https://codereview.chromium.org/2373773002 submitted at around the same time and touches related code, so if elawrence@ says no, that's my next best guess.
,
Mar 27 2017
https://codereview.chromium.org/2734783007 causes us to do *less* work, by skipping the ApplyColor/ApplyStyle operation when it isn't necessary. (I'd be shocked if the difference was noticeable either way).
,
Mar 27 2017
This is actually a 30ms improvement rather than a regression, for the omnibox code at least. (The sampling profiler UI still needs some polish to make the direction of the diff clearer.) So, congratulations on speeding things up! :) |
|||
►
Sign in to add a comment |
|||
Comment 1 Deleted