Currently m_state is a member variable of StyleResolver. Make the m_state an on-stack object.
Created attachment 188509 [details] Patch
Created attachment 189257 [details] Patch
Comment on attachment 189257 [details] Patch r=me
Created attachment 189449 [details] Patch for landing
Comment on attachment 189449 [details] Patch for landing Clearing flags on attachment: 189449 Committed r143556: <http://trac.webkit.org/changeset/143556>
All reviewed patches have been landed. Closing bug.
This looks like 2-3% performance regression in Parser/html5-full-render: https://perf.webkit.org/#mode=charts&days=56&chartList=%5B%5B%22Mac%20Lion%22%2C%22Parser%2Fhtml5-full-render%3ATime%22%5D%5D&zoom=%5B1361218175932.3066%2C1361543074858.7913%5D
(In reply to comment #7) > This looks like 2-3% performance regression in Parser/html5-full-render: Thanks for the catch! Takashi is out until next week. Let me roll this out for now.
Rollout did indeed fix the regression.
Reopening (Rolled out http://trac.webkit.org/changeset/143885)
Created attachment 190754 [details] Patch(revised)
I revised the patch (State& -> const State&) and measured its performance: https://docs.google.com/spreadsheet/ccc?key=0AtdkWCxGaEc2dGRtWjlFejIyS1BBamtEcElzM1ZidGc&usp=sharing The spreadsheet compares master's html5-full-render test with patched one's html5-full-render test, i.e. for both master and patched DumpRenderTree, run xvfb-run out/Release/DumpRenderTree third_party/WebKit/PerformanceTests/Parser/html5-full-render.html --no-timeout The result looks almost the same performance. So can I land this patch?
Comment on attachment 190754 [details] Patch(revised) Attachment 190754 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16847211 New failing tests: css2.1/t1202-counter-01-b.html compositing/rtl/rtl-absolute-overflow.html css1/box_properties/margin_right.html accessibility/render-counter-text.html css1/box_properties/margin.html compositing/rtl/rtl-fixed-overflow.html http/tests/cache/subresource-failover-to-network.html css2.1/t1202-counter-02-b.html compositing/iframes/iframe-in-composited-layer.html accessibility/first-letter-text-transform-causes-crash.html compositing/rtl/rtl-fixed-overflow-scrolled.html compositing/layer-creation/overlap-transformed-and-clipped.html compositing/scrollbar-painting.html compositing/overflow/clip-content-under-overflow-controls.html compositing/generated-content.html compositing/overflow/remove-overflow-crash2.html compositing/overflow/content-loses-scrollbars.html css2.1/t1202-counter-03-b.html compositing/reflections/nested-reflection-on-overflow.html css1/pseudo/multiple_pseudo_elements.html css1/pseudo/firstline.html css1/pseudo/firstletter.html compositing/overflow/scrollbar-painting.html css1/color_and_background/background_attachment.html compositing/overflow/overflow-scroll.html compositing/rtl/rtl-absolute-overflow-scrolled.html css1/color_and_background/background_repeat.html css1/pseudo/pseudo_elements_in_selectors.html css2.1/t0510-c25-pseudo-elmnt-00-c.html css2.1/t1202-counter-00-b.html
Comment on attachment 190754 [details] Patch(revised) Attachment 190754 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16758390 New failing tests: css2.1/t1202-counter-01-b.html css2.1/t1202-counter-08-b.html css2.1/t1202-counters-06-b.html css2.1/t1202-counter-07-b.html css2.1/t1202-counter-02-b.html css2.1/t1202-counter-04-b.html css2.1/t1202-counter-15-b.html compositing/generated-content.html css2.1/t1202-counter-16-f.html css2.1/t1202-counter-13-b.html css2.1/t1202-counters-03-b.html css2.1/t1202-counter-12-b.html http/tests/cookies/third-party-cookie-relaxing.html css2.1/t1202-counter-09-b.html css2.1/t1202-counter-14-b.html css2.1/t1202-counter-11-b.html css2.1/t1202-counters-01-b.html css2.1/t1202-counters-04-b.html css2.1/t1202-counter-05-b.html css2.1/t1202-counter-06-b.html css2.1/t1202-counter-03-b.html css2.1/t1202-counters-02-b.html css2.1/t1202-counters-00-b.html css2.1/t1202-counter-00-b.html css2.1/t1202-counters-05-b.html
(In reply to comment #12) > I revised the patch (State& -> const State&) and measured its performance: > > https://docs.google.com/spreadsheet/ccc?key=0AtdkWCxGaEc2dGRtWjlFejIyS1BBamtEcElzM1ZidGc&usp=sharing FYI, http://trac.webkit.org/wiki/Performance%20Tests#AggregatingandComparingResults > The spreadsheet compares master's html5-full-render test with patched one's html5-full-render test, i.e. > for both master and patched DumpRenderTree, run xvfb-run out/Release/DumpRenderTree third_party/WebKit/PerformanceTests/Parser/html5-full-render.html --no-timeout > > The result looks almost the same performance. So can I land this patch? Do you observe a difference when you run the baseline and the one with the old patch? i.e. Can you reproduce the perf. regression we saw on the bots on your machine?
Comment on attachment 190754 [details] Patch(revised) Attachment 190754 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16758428 New failing tests: css2.1/t1202-counter-01-b.html css2.1/t1202-counter-08-b.html css2.1/t1202-counters-06-b.html css2.1/t1202-counter-07-b.html css2.1/t1202-counter-02-b.html css2.1/t1202-counter-04-b.html css2.1/t1202-counters-12-b.html css2.1/t1202-counter-15-b.html css2.1/t1202-counters-09-b.html compositing/generated-content.html css2.1/t1202-counters-11-b.html css2.1/t1202-counter-16-f.html css2.1/t1202-counters-04-b.html css2.1/t1202-counters-03-b.html css2.1/t1202-counter-12-b.html css2.1/t1202-counters-07-b.html http/tests/cookies/third-party-cookie-relaxing.html css2.1/t1202-counter-09-b.html css2.1/t1202-counters-08-b.html css2.1/t1202-counter-14-b.html css2.1/t1202-counter-11-b.html css2.1/t1202-counters-01-b.html css2.1/t1202-counter-13-b.html css2.1/t1202-counter-05-b.html css2.1/t1202-counter-06-b.html css2.1/t1202-counter-03-b.html css2.1/t1202-counters-02-b.html css2.1/t1202-counters-00-b.html css2.1/t1202-counter-00-b.html css2.1/t1202-counters-05-b.html
(In reply to comment #16) > New failing tests: > css2.1/t1202-counter-01-b.html > css2.1/t1202-counter-08-b.html > css2.1/t1202-counters-06-b.html > css2.1/t1202-counter-07-b.html > css2.1/t1202-counter-02-b.html > css2.1/t1202-counter-04-b.html > css2.1/t1202-counters-12-b.html > css2.1/t1202-counter-15-b.html > css2.1/t1202-counters-09-b.html > compositing/generated-content.html > css2.1/t1202-counters-11-b.html > css2.1/t1202-counter-16-f.html > css2.1/t1202-counters-04-b.html > css2.1/t1202-counters-03-b.html > css2.1/t1202-counter-12-b.html > css2.1/t1202-counters-07-b.html > http/tests/cookies/third-party-cookie-relaxing.html > css2.1/t1202-counter-09-b.html > css2.1/t1202-counters-08-b.html > css2.1/t1202-counter-14-b.html > css2.1/t1202-counter-11-b.html > css2.1/t1202-counters-01-b.html > css2.1/t1202-counter-13-b.html > css2.1/t1202-counter-05-b.html > css2.1/t1202-counter-06-b.html > css2.1/t1202-counter-03-b.html > css2.1/t1202-counters-02-b.html > css2.1/t1202-counters-00-b.html > css2.1/t1202-counter-00-b.html > css2.1/t1202-counters-05-b.html Are these failures real?
Created attachment 191396 [details] Patch
I rebase-lined and fixed cq- errors. (In reply to comment #15) > (In reply to comment #12) > > I revised the patch (State& -> const State&) and measured its performance: > Do you observe a difference when you run the baseline and the one with the old patch? i.e. Can you reproduce the perf. regression we saw on the bots on your machine? I compared master build with patched build again. The result was: https://docs.google.com/spreadsheet/ccc?key=0AtdkWCxGaEc2dGUwbWNEZm1iREQxNHE5WTl1dkVIMGc&usp=sharing My patch seems to cause about 1% performance regression. I think, this is almost the same performance regression as the old patch.
This code has been significantly refactored since this patch was proposed. There doesn't seem to be any action we can take here.
<rdar://problem/96978025>
It has indeed been refactored to be a stack object.