[webkit-reviews] review granted: [Bug 111417] [V8] Add machinery for generating specialized bindings for the main world : [Attachment 193131] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Marja Hölttä
<marja at chromium.org>'s request for review:
Bug 111417: [V8] Add machinery for generating specialized bindings for the main
world
https://bugs.webkit.org/show_bug.cgi?id=111417

Attachment 193131: Patch
https://bugs.webkit.org/attachment.cgi?id=193131&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193131&action=review


Looks great.

> Source/WebCore/ChangeLog:10
> +	   The new specialized bindings will be faster, because they don't need
to
> +	   do the "main world, isolated world or a worker" check, but can right

> +	   away assume that we're in the main world.

As jochen pointed out, you can measure the performance impact by using
PerformanceTests/Bindings/first-child.html, which would be the most sensitive
micro-benchmark to your change. Please paste the results here.

(Actually I won't be much surprised if you cannot observe any performance
improvement. When I tried the optimization last time, I couldn't observe any
perf win.)

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2799
> +	   if (!$attrExt->{"V8EnabledPerContext"}) {

Why do you want to exclude attributes with [V8EnabledPerContext]? (Either way
you can fix it in a separate patch.)

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2809
> +	       if (!$attrExt->{"V8EnabledPerContext"}) {

Ditto.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3062
> -    if (!${enable_function}())
> +   if (!${enable_function}())

Nit: Drop this change.