Bug 96433

Summary: [V8] 8% regression in dom_perf
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: adamk, eric, haraken, jamesr, japhet, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Adam Barth
Reported 2012-09-11 15:26:30 PDT
[V8] 8% regression in dom_perf
Attachments
Patch (3.87 KB, patch)
2012-09-11 15:27 PDT, Adam Barth
no flags
Patch for landing (4.00 KB, patch)
2012-09-11 16:04 PDT, Adam Barth
no flags
Adam Barth
Comment 1 2012-09-11 15:27:59 PDT
Kentaro Hara
Comment 3 2012-09-11 16:01:42 PDT
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
Comment 4 2012-09-11 16:04:21 PDT
Created attachment 163467 [details] Patch for landing
Adam Barth
Comment 5 2012-09-11 16:04:32 PDT
Done
WebKit Review Bot
Comment 6 2012-09-11 16:48:47 PDT
Comment on attachment 163467 [details] Patch for landing Clearing flags on attachment: 163467 Committed r128242: <http://trac.webkit.org/changeset/128242>
WebKit Review Bot
Comment 7 2012-09-11 16:48:50 PDT
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 8 2012-09-11 17:44:19 PDT
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
Comment 9 2012-09-11 17:50:17 PDT
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
Comment 10 2012-09-11 17:59:29 PDT
Yeah, sorry. Just needs an update.
Adam Barth
Comment 11 2012-09-11 17:59:45 PDT
/me will fix
Adam Barth
Comment 12 2012-09-12 16:40:10 PDT
Fixed.
Note You need to log in before you can comment on or make changes to this bug.