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

Issue 230783002: Revert 170941 "Move popular DOM attributes to prototype chains" (Closed)

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

Description

Revert 170941 "Move popular DOM attributes to prototype chains" Regressed performance unacceptably. https://chromeperf.appspot.com/group_report?bug_id=361260 > 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 > > Review URL: https://codereview.chromium.org/158713002 TBR=haraken@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171141

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, 8 months ago (2014-04-09 12:37:47 UTC) #1
haraken
6 years, 8 months ago (2014-04-09 12:37:59 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r171141.

Powered by Google App Engine
This is Rietveld 408576698