Bug 96433 – [V8] 8% regression in dom_perf

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

[V8] 8% regression in dom_perf

[Adam Barth]

Created attachment 163455 [details]
Patch

[Adam Barth]

http://code.google.com/p/chromium/issues/detail?id=147639

[Kentaro Hara]

Comment on attachment 163455 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163455&action=review

> Source/WebCore/bindings/v8/V8DOMWrapper.cpp:146
> +    if (document && document->frame())

Shall we add a comment that this code is for performance?

[Adam Barth]

Created attachment 163467 [details]
Patch for landing

[Adam Barth]

Done

[WebKit Review Bot]

Comment on attachment 163467 [details]
Patch for landing

Clearing flags on attachment: 163467

Committed r128242: <http://trac.webkit.org/changeset/128242>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[James Robinson]

I think this made bindings-tests on cr-mac fail:

http://build.webkit.org/builders/Chromium%20Mac%20Release%20%28Tests%29/builds/23809/steps/bindings-generation-tests/logs/stdio

FAIL: (V8) V8TestCustomNamedGetter.cpp
--- WebCore/bindings/scripts/test/V8/V8TestCustomNamedGetter.cpp	2012-09-11 02:59:54.000000000 -0700
+++ /var/folders/OH/OHv-vytAG2GCWI6iDiG7U++++Tg/-Tmp-/tmpj7R5NQ/V8TestCustomNamedGetter.cpp	2012-09-11 17:28:22.000000000 -0700
@@ -112,6 +112,8 @@
 v8::Handle<v8::Object> V8TestCustomNamedGetter::wrapSlow(PassRefPtr<TestCustomNamedGetter> impl, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
 {
     v8::Handle<v8::Object> wrapper;
+    Document* document = 0;
+    UNUSED_PARAM(document);
 
     v8::Handle<v8::Context> context;
     if (!creationContext.IsEmpty() && creationContext->CreationContext() != v8::Context::GetCurrent()) {
@@ -122,7 +124,7 @@
         context->Enter();
     }
 
-    wrapper = V8DOMWrapper::instantiateV8Object(&info, impl.get());
+    wrapper = V8DOMWrapper::instantiateV8Object(document, &info, impl.get());
 
     if (!context.IsEmpty())
         context->Exit();


etc....

[James Robinson]

Actually, bindings test seem to be failing everywhere that uses v8.  Probably just needs an expectation update, but I don't feel confident enough in the change to do it.  Could you take a look when you get a sec, Adam?

[Adam Barth]

Yeah, sorry.  Just needs an update.

[Adam Barth]

/me will fix

[Adam Barth]

Fixed.