Bug 96433 - [V8] 8% regression in dom_perf
Summary: [V8] 8% regression in dom_perf
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-11 15:26 PDT by Adam Barth
Modified: 2012-09-12 16:40 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.87 KB, patch)
2012-09-11 15:27 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (4.00 KB, patch)
2012-09-11 16:04 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2012-09-11 15:26:30 PDT
[V8] 8% regression in dom_perf
Comment 1 Adam Barth 2012-09-11 15:27:59 PDT
Created attachment 163455 [details]
Patch
Comment 3 Kentaro Hara 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?
Comment 4 Adam Barth 2012-09-11 16:04:21 PDT
Created attachment 163467 [details]
Patch for landing
Comment 5 Adam Barth 2012-09-11 16:04:32 PDT
Done
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2012-09-11 16:48:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 James Robinson 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....
Comment 9 James Robinson 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?
Comment 10 Adam Barth 2012-09-11 17:59:29 PDT
Yeah, sorry.  Just needs an update.
Comment 11 Adam Barth 2012-09-11 17:59:45 PDT
/me will fix
Comment 12 Adam Barth 2012-09-12 16:40:10 PDT
Fixed.