Bug 102858 – [v8] 4% regression in dom_traverse tests

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Ojan Vafai]

haraken, abarth, looks to be one of your patches.

http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/dromaeo_domcoretraverse/report.html?rev=168181&graph=dom_traverse_previousSibling&trace=score&history=150

http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/dromaeo_domcoretraverse/report.html?rev=168181&graph=dom_traverse_nextSibling&trace=score&history=150

http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/dromaeo_domcoretraverse/report.html?rev=168181&graph=dom_traverse_previousSibling&history=150

http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/dromaeo_domcoretraverse/report.html?rev=168181&graph=dom_traverse_nextSibling&history=150

Regression range seems to be http://trac.webkit.org/log/?verbose=on&rev=134736&stop_rev=134728.

[Ojan Vafai]

ping

[Kentaro Hara]

(In reply to comment #0)
> Regression range seems to be http://trac.webkit.org/log/?verbose=on&rev=134736&stop_rev=134728.

abarth: Maybe the culprit is r134729 ? Other changes in the range don't change generated code at all.

[Adam Barth]

Sure.  We can revert that patch to see.  We'll need to do the revert manually though.

[Adam Barth]

Created attachment 176492 [details]
Patch

[Adam Barth]

Comment on attachment 176492 [details]
Patch

Clearing flags on attachment: 176492

Committed r136015: <http://trac.webkit.org/changeset/136015>

[Adam Barth]

I'm leaving this bug open to remind us to check where the bot is actually healed.

[Adam Barth]

No luck.

[Chris Dumez]

After the roll out, the following bindings generation test is failing:

FAIL: (V8) V8TestNode.h
--- WebCore/bindings/scripts/test/V8/V8TestNode.h	2012-11-19 18:49:35.208640094 -0800
+++ /tmp/tmpIYlIMK/V8TestNode.h	2012-11-28 09:23:10.983184745 -0800
@@ -88,10 +88,11 @@
     // in an isolated world. The fastest way we know how to do that is to check
     // whether the holder's inline wrapper is the same wrapper we see in the
     // v8::AccessorInfo.
-    v8::Handle<v8::Object> wrapper = (holder->wrapper() == info.Holder()) ? impl->wrapper() : DOMDataStore::getNode(impl, info.GetIsolate());
+    v8::Handle<v8::Object> holderWrapper = info.Holder();
+    v8::Handle<v8::Object> wrapper = (holder->wrapper() == holderWrapper) ? impl->wrapper() : DOMDataStore::getNode(impl, info.GetIsolate());
     if (!wrapper.IsEmpty())
         return wrapper;
-    return wrap(impl, info.Holder(), info.GetIsolate());
+    return wrap(impl, holderWrapper, info.GetIsolate());
 }
 
 inline v8::Handle<v8::Value> toV8(PassRefPtr< TestNode > impl, v8::Handle<v8::Object> creationContext = v8::Handle<v8::Object>(), v8::Isolate* isolate = 0)

[Kentaro Hara]

(In reply to comment #8)
> After the roll out, the following bindings generation test is failing:

Sorry, fixed in r136100.

[Ojan Vafai]

(In reply to comment #7)
> No luck.

Not only that, it may have caused https://bugs.webkit.org/show_bug.cgi?id=103791.

[Anders Carlsson]

V8 is gone.