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

Issue 179343002: Revert 167680 "Move popular DOM attributes to prototype chains" (Closed)

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

Description

Revert 167680 "Move popular DOM attributes to prototype chains" This CL caused unacceptable perf regression, because I landed the CL before landing a V8 side CL that fixes the perf regression. I'll land the CL again after landing the V8 side CL. > 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 two 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 1086 runs/sec => 1077 runs/sec (-0.8%) > Dromaeo/dom-modify 522 runs/sec => 525 runs/sec (+0.5%) > Dromaeo/dom-query 26150 runs/sec => 26200 runs/sec (+0.2%) > Dromaeo/dom-traverse 564 runs/sec => 544 runs/sec (-3.6%) > > I don't understand the regression in dom-traverse. The regression is reproducible but I cannot reproduce the regression in a super-micro benchmark that just repeats accessing div.firstChild. Probably the regression might be a noise. > > 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 > > Review URL: https://codereview.chromium.org/158713002 TBR=haraken@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167722

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, 9 months ago (2014-02-25 05:44:32 UTC) #1
haraken
6 years, 9 months ago (2014-02-25 05:44:51 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r167722.

Powered by Google App Engine
This is Rietveld 408576698