[V8] Weave creationContext through toV8 and related functions
Created attachment 162618 [details] Patch
Created attachment 162619 [details] run-bindings-tests diffs
I put the run-bindings-tests diffs in a separate patch file to make it easier to read.
Comment on attachment 162618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162618&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:508 > +inline v8::Handle<v8::Value> toV8(${nativeType}* impl, v8::Handle<v8::Context> creationContext = v8::Handle<v8::Context>(), v8::Isolate* isolate = 0${forceNewObjectParameter}) Isn't there any perf concern about creating v8::Handle<v8::Context>() every time? toV8() should be super fast especially for node wrapper objects. If you cannot observe any perf regression in Binding/first-child, then it looks OK.
> Isn't there any perf concern about creating v8::Handle<v8::Context>() every time? toV8() should be super fast especially for node wrapper objects. If you cannot observe any perf regression in Binding/first-child, then it looks OK. I'll check before landing. At the moment, I'm just trying to get it working.
Looks like I might have made it marginally faster. The "before" numbers are before this patch and the "after" numbers are after this patch and the patch in Bug 96044. == Before == Running 1 tests Running Bindings/first-child.html (1 of 1) DESCRIPTION: This benchmark covers 'firstChild', 'lastChild', 'nextSibling' and 'previousSibling' in Dromaeo/dom-traverse.html, and other DOM attributes that return a Node object. RESULT Bindings: first-child= 812.341722521 runs/s median= 812.182741117 runs/s, stdev= 1.8782032458 runs/s, min= 807.061790668 runs/s, max= 815.286624204 runs/s RESULT Bindings: first-child: JSHeap= 1330116.8 bytes median= 1326596.0 bytes, stdev= 12326.5484934 bytes, min= 1319888.0 bytes, max= 1367520.0 bytes RESULT Bindings: first-child: Malloc= 0.0 bytes median= 0.0 bytes, stdev= 0.0 bytes, min= 0.0 bytes, max= 0.0 bytes == After == DESCRIPTION: This benchmark covers 'firstChild', 'lastChild', 'nextSibling' and 'previousSibling' in Dromaeo/dom-traverse.html, and other DOM attributes that return a Node object. RESULT Bindings: first-child= 813.735729057 runs/s median= 814.249363868 runs/s, stdev= 1.80420562007 runs/s, min= 810.126582278 runs/s, max= 816.326530612 runs/s RESULT Bindings: first-child: JSHeap= 1330162.4 bytes median= 1326696.0 bytes, stdev= 12300.6580897 bytes, min= 1319976.0 bytes, max= 1367520.0 bytes RESULT Bindings: first-child: Malloc= 0.0 bytes median= 0.0 bytes, stdev= 0.0 bytes, min= 0.0 bytes, max= 0.0 bytes
(In reply to comment #6) > Looks like I might have made it marginally faster. The "before" numbers are before this patch and the "after" numbers are after this patch and the patch in Bug 96044. Looks good:)
https://trac.webkit.org/changeset/127806 broke bindings tests on the bots. Could you check it, please?
(In reply to comment #8) > https://trac.webkit.org/changeset/127806 broke bindings tests on the bots. > Could you check it, please? Oddly, this bug is still in NEW. And, it looks like the second patch (rebaselining the bindings test) didn't get landed.
(In reply to comment #8) > https://trac.webkit.org/changeset/127806 broke bindings tests on the bots. > Could you check it, please? Oh, sorry. I forgot that I put the bindings results in a separate patch.
https://bugs.webkit.org/show_bug.cgi?id=96039
> Isn't there any perf concern about creating v8::Handle<v8::Context>() every time? toV8() should be super fast especially for node wrapper objects. If you cannot observe any perf regression in Binding/first-child, then it looks OK. Turns out you were right, but the issue was with calling CreationContext(). Binding/first-child didn't see the problem initially because that particular code path wasn't using CreationContext() in this patch. http://trac.webkit.org/changeset/127946 delays delays calling CreationContext() until we actually need it, which seems to solve the problem. http://trac.webkit.org/changeset/127955 then causes Binding/first-child to go through this new code path and doesn't show a perf regression.