Pass VM instead of JSGlobalObject to function constructors.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Andreas Kling]

Pass VM instead of JSGlobalObject to function constructors.

[Andreas Kling]

Created attachment 212943 [details]
Patch

[Andreas Kling]

Benchmark report for SunSpider on foie (MacBookPro10,1).

VMs tested:
"TipOfTree" at /Users/akling/Source/Safari/Reference-OpenSource/WebKitBuild/Release/jsc
"MyChanges" at /Users/akling/Source/Safari/OpenSource/WebKitBuild/Release/jsc

Collected 4 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between
sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific
preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95%
confidence intervals in milliseconds.

                                  TipOfTree                 MyChanges                                     

3d-cube                         5.0504+-0.2404     ?      5.0685+-0.4292        ?
3d-morph                        5.3848+-0.1364     ?      5.4618+-0.2715        ? might be 1.0143x slower
3d-raytrace                     6.4196+-0.6557            6.3090+-0.3647          might be 1.0175x faster
access-binary-trees             1.3939+-0.1756            1.3300+-0.0646          might be 1.0480x faster
access-fannkuch                 5.6747+-1.8268            5.0225+-0.1727          might be 1.1299x faster
access-nbody                    2.4979+-0.0869     ?      2.5365+-0.1272        ? might be 1.0155x slower
access-nsieve                   3.2899+-0.0775            3.2593+-0.1950        
bitops-3bit-bits-in-byte        1.3797+-0.0327     ?      1.4056+-0.0422        ? might be 1.0188x slower
bitops-bits-in-byte             1.8750+-0.1168     ?      1.9258+-0.2800        ? might be 1.0271x slower
bitops-bitwise-and              1.9222+-0.0419            1.9171+-0.0297        
bitops-nsieve-bits              3.0391+-0.2831            2.8948+-0.0700          might be 1.0498x faster
controlflow-recursive           1.8852+-0.0740            1.8689+-0.0443        
crypto-aes                      3.6846+-0.3304            3.6338+-0.0604          might be 1.0140x faster
crypto-md5                      2.2400+-0.1685            2.1845+-0.0904          might be 1.0254x faster
crypto-sha1                     2.1714+-0.2661     ?      2.1838+-0.2136        ?
date-format-tofte               7.3279+-0.2435     ?      7.6221+-1.3069        ? might be 1.0401x slower
date-format-xparb               5.2873+-0.0333     ?      5.5544+-0.3916        ? might be 1.0505x slower
math-cordic                     2.9060+-0.4604            2.7918+-0.0209          might be 1.0409x faster
math-partial-sums               5.4099+-0.2110     ?      5.8085+-0.6211        ? might be 1.0737x slower
math-spectral-norm              1.7714+-0.1938            1.7349+-0.0814          might be 1.0210x faster
regexp-dna                      7.7466+-0.3649     ?      7.8943+-0.3119        ? might be 1.0191x slower
string-base64                   3.5917+-0.2942     ?      3.8972+-0.7940        ? might be 1.0851x slower
string-fasta                   11.7495+-0.4235           11.2756+-0.4031          might be 1.0420x faster
string-tagcloud                 9.7996+-0.7279     ?     10.4869+-0.7872        ? might be 1.0701x slower
string-unpack-code             20.3339+-1.4194     ?     20.3427+-1.1601        ?
string-validate-input           4.4093+-0.7144            4.1877+-0.0550          might be 1.0529x faster

<arithmetic> *                  4.9324+-0.1621     ?      4.9461+-0.1401        ? might be 1.0028x slower
<geometric>                     3.8557+-0.0692            3.8526+-0.0870          might be 1.0008x faster
<harmonic>                      3.1461+-0.0199            3.1327+-0.0499          might be 1.0043x faster

[Darin Adler]

Comment on attachment 212943 [details]
Patch

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

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:601
>                          

Why keep this blank line?

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:604
> +                        VM& vm = exec->vm();
> +                        JSObject* o = JSCallbackFunction::create(vm, thisObj->globalObject(), callAsFunction, name);
> +                        thisObj->putDirect(vm, propertyName, o, entry->attributes);

Do local variables pay back in cases like this?

[Andreas Kling]

(In reply to comment #3)
> (From update of attachment 212943 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212943&action=review
> 
> > Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:604
> > +                        VM& vm = exec->vm();
> > +                        JSObject* o = JSCallbackFunction::create(vm, thisObj->globalObject(), callAsFunction, name);
> > +                        thisObj->putDirect(vm, propertyName, o, entry->attributes);
> 
> Do local variables pay back in cases like this?

I used to think that it doesn't. Then I looked at assembly output and saw a lot more loads than I expected.

For example, JSGlobalObject::reset() shrunk ~4kB when I cached the VM in a local. Since then I've become very suspicious of the compiler's ability to see which values aren't changing.

I checked now, and this function does not benefit from the local variable. However in this same patch, JSPromisePrototypeFuncCatch does benefit. It shrinks 14 bytes on x86_64 from caching the VM for a single re-use.

(Return of an old cargo cult?)

[Andreas Kling]

Committed r156624: <http://trac.webkit.org/changeset/156624>