Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(139)

Issue 292743003: Revert 174142 "Move popular DOM attributes to prototype chains" (Closed)

Created:
6 years, 7 months ago by haraken
Modified:
6 years, 7 months ago
Reviewers:
haraken
CC:
blink-reviews
Visibility:
Public.

Description

Revert 174142 "Move popular DOM attributes to prototype chains" It caused unacceptable regression in Dromaeo/xxx-traverse. https://chromeperf.appspot.com/report?masters=ChromiumPerf&bots=chromium-rel-mac6%2Cchromium-rel-mac8%2Cchromium-rel-win7-dual%2Cchromium-rel-win7-single%2Cchromium-rel-win7-x64-dual%2Cchromium-rel-win8-dual%2Cchromium-rel-xp-dual%2Clinux-release&tests=dromaeo.jslibtraverseprototype%2Fjslib&checked=core > Move popular DOM attributes to prototype chains > > Design document: https://docs.google.com/a/google.com/document/d/1yeHTCHhulVIlrKyx9_gCguAhLfcefVOa9uxxfW2LVG0/edit > > Intent-to-ship-and-implement in blink-dev: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/t0XiZuMey7M/9-5AuhoFyisJ > > This CL moves DOM attributes used in Dromaeo to prototype chains. When I landed the CL four months ago (https://codereview.chromium.org/94403002), it caused 20+% regression in benchmarks. However, now that a V8 side fix is landed, there should be no overhead in moving DOM attributes to prototype chains in theory. > > Dromaeo/dom-attr 1145 runs/sec => 1130 runs/sec (-1.3%) > Dromaeo/dom-modify 492 runs/sec => 496 runs/sec (+0.8%) > Dromaeo/dom-query 25580 runs/sec => 25107 runs/sec (-1.8%) > Dromaeo/dom-traverse 581 runs/sec => 579 runs/sec (+0.3%) > > In particular, I cannot observe any regression in a super micro benchmark that repeats div.firstChild. > > I'm going to land this CL and see how much regression is observed in perf bots (I'll email to perf-sheriff@). If I find the regression is unacceptable, I'll revert the CL. > > See the design document and the Intent-to-ship email for justification for the regression. > > BUG=43394 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167680 > > R=jochen@chromium.org > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168383 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170941 > > Review URL: https://codereview.chromium.org/158713002 TBR=haraken@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174278

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -54 lines) Patch
D trunk/LayoutTests/fast/dom/dom-attribute-on-prototype-chain.html View 1 chunk +0 lines, -29 lines 0 comments Download
D trunk/LayoutTests/fast/dom/dom-attribute-on-prototype-chain-expected.txt View 1 chunk +0 lines, -15 lines 0 comments Download
M trunk/Source/core/dom/Document.idl View 1 chunk +1 line, -1 line 0 comments Download
M trunk/Source/core/dom/Element.idl View 2 chunks +2 lines, -2 lines 0 comments Download
M trunk/Source/core/dom/Node.idl View 1 chunk +7 lines, -7 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
haraken
6 years, 7 months ago (2014-05-19 13:19:46 UTC) #1
haraken
6 years, 7 months ago (2014-05-19 13:19:57 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r174278.

Powered by Google App Engine
This is Rietveld 408576698