Bug 71427 - Inlined uses of the global object should use the right global object
Summary: Inlined uses of the global object should use the right global object
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-02 18:13 PDT by Filip Pizlo
Modified: 2011-11-02 19:59 PDT (History)
0 users

See Also:


Attachments
the patch (10.12 KB, patch)
2011-11-02 18:14 PDT, Filip Pizlo
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2011-11-02 18:13:30 PDT
The DFG backend was assuming that uses of the global object can use the global object associated with the machine code block.  But they should be the ones associated with the (potentially inlined) function instead.
Comment 1 Filip Pizlo 2011-11-02 18:14:44 PDT
Created attachment 113413 [details]
the patch
Comment 2 Oliver Hunt 2011-11-02 19:30:06 PDT
Comment on attachment 113413 [details]
the patch

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

r=me, but i don't believe that fix me is correct.

> Source/JavaScriptCore/dfg/DFGJITCompiler.h:439
> +        // FIXME: if we ever inline based on executable not function, this code will need to change.

I don't believe that this is correct -- all functions for a given executable must by definition share the same global object.
Comment 3 Filip Pizlo 2011-11-02 19:31:01 PDT
(In reply to comment #2)
> (From update of attachment 113413 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113413&action=review
> 
> r=me, but i don't believe that fix me is correct.
> 
> > Source/JavaScriptCore/dfg/DFGJITCompiler.h:439
> > +        // FIXME: if we ever inline based on executable not function, this code will need to change.
> 
> I don't believe that this is correct -- all functions for a given executable must by definition share the same global object.

If we inline based on executable, the inlineCallFrame->callee field will be 0, and there will be another field that tells you how to get the callee from the register file.  That's what I meant by the FIXME.
Comment 4 Filip Pizlo 2011-11-02 19:52:23 PDT
Looks performance neutral and all tests pass.



Benchmark report for SunSpider, V8, and Kraken.

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/tertiary/OpenSource/WebKitBuild/Release/jsc
"FixInlineGlobal" at /Volumes/Data/pizlo/secondary/OpenSource/WebKitBuild/Release/jsc

Collected 12 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            FixInlineGlobal                                 
SunSpider:
   3d-cube                                7.9055+-0.0250          7.9030+-0.0245       
   3d-morph                               8.4010+-0.0327          8.3968+-0.0273       
   3d-raytrace                            8.2108+-0.0663          8.1345+-0.0946       
   access-binary-trees                    1.7229+-0.0395          1.6849+-0.0058         might be 1.0226x faster
   access-fannkuch                        7.8228+-0.0070    ?     7.8424+-0.0260       ?
   access-nbody                           4.5250+-0.0182    ?     4.5274+-0.0092       ?
   access-nsieve                          3.1817+-0.0122    ?     3.2215+-0.0327       ? might be 1.0125x slower
   bitops-3bit-bits-in-byte               1.2934+-0.0156    ?     1.2998+-0.0149       ?
   bitops-bits-in-byte                    4.9984+-0.0076          4.9842+-0.0120       
   bitops-bitwise-and                     3.4312+-0.0617    ?     3.4583+-0.0728       ?
   bitops-nsieve-bits                     5.6541+-0.0357    ?     5.6871+-0.0369       ?
   controlflow-recursive                  2.3309+-0.0122    ?     2.3598+-0.0281       ? might be 1.0124x slower
   crypto-aes                             7.7440+-0.0641          7.7323+-0.0451       
   crypto-md5                             2.8935+-0.0283          2.8830+-0.0122       
   crypto-sha1                            2.6501+-0.0400          2.6337+-0.0210       
   date-format-tofte                     10.8381+-0.1213         10.7141+-0.0460         might be 1.0116x faster
   date-format-xparb                     10.0780+-0.1998    ^     9.7163+-0.1056       ^ definitely 1.0372x faster
   math-cordic                            7.2948+-0.0635          7.2656+-0.0419       
   math-partial-sums                     10.5843+-0.0257         10.5822+-0.0750       
   math-spectral-norm                     2.9087+-0.0334          2.8803+-0.0069       
   regexp-dna                            13.4231+-0.1595    ?    13.4949+-0.1883       ?
   string-base64                          4.3142+-0.0267          4.2876+-0.0140       
   string-fasta                           7.1573+-0.0114    ^     7.1321+-0.0136       ^ definitely 1.0035x faster
   string-tagcloud                       13.2550+-0.1296    ?    13.2607+-0.1008       ?
   string-unpack-code                    23.0323+-0.1087         22.9692+-0.1619       
   string-validate-input                  5.6430+-0.0265    ?     5.6622+-0.0404       ?

   <arithmetic> *                         6.9729+-0.0207          6.9505+-0.0238       
   <geometric>                            5.6266+-0.0158          5.6126+-0.0150       
   <harmonic>                             4.4490+-0.0168          4.4406+-0.0118       

                                            TipOfTree            FixInlineGlobal                                 
V8:
   crypto                                81.1349+-0.2988         80.8813+-0.2027       
   deltablue                            182.6440+-0.9418        182.6097+-1.9133       
   earley-boyer                         111.4420+-0.2287        111.3792+-0.4176       
   raytrace                              69.1373+-0.3317    !    70.0228+-0.5185       ! definitely 1.0128x slower
   regexp                               125.9373+-0.5356        125.8627+-0.1749       
   richards                             144.1570+-0.5759    ?   145.0371+-0.3479       ?
   splay                                 90.5705+-0.9146         90.4614+-1.1537       

   <arithmetic>                         115.0033+-0.1811    ?   115.1792+-0.2915       ?
   <geometric> *                        109.4122+-0.1859    ?   109.6156+-0.2958       ?
   <harmonic>                           104.2286+-0.2096    ?   104.4777+-0.3322       ?

                                            TipOfTree            FixInlineGlobal                                 
Kraken:
   ai-astar                             818.0541+-10.8419   ?   827.8603+-0.5665       ? might be 1.0120x slower
   audio-beat-detection                 209.9056+-1.0301        209.7902+-0.8383       
   audio-dft                            262.9032+-1.8822        260.7352+-2.4678       
   audio-fft                            136.9000+-0.5795    ?   137.1937+-0.7243       ?
   audio-oscillator                     317.5338+-30.9262       290.2611+-1.1621         might be 1.0940x faster
   imaging-darkroom                     338.8633+-5.6019        338.4315+-5.8906       
   imaging-desaturate                   240.9064+-0.0799    ?   240.9227+-0.1222       ?
   imaging-gaussian-blur                621.3248+-0.9010        621.1927+-0.3395       
   json-parse-financial                  70.4761+-0.1563    ?    70.5841+-0.1556       ?
   json-stringify-tinderbox              79.2334+-0.2014    ?    79.5120+-0.6848       ?
   stanford-crypto-aes                  116.6959+-0.7287        116.5538+-0.6824       
   stanford-crypto-ccm                  114.7526+-0.5635    !   115.8885+-0.5538       ! definitely 1.0099x slower
   stanford-crypto-pbkdf2               234.7142+-0.5188    ?   235.8879+-1.0809       ?
   stanford-crypto-sha256-iterative      97.5235+-0.2353    ?    97.6753+-0.1444       ?

   <arithmetic> *                       261.4134+-2.5696        260.1778+-0.3637       
   <geometric>                          200.4197+-1.3682        199.6099+-0.2592       
   <harmonic>                           159.7703+-0.5276        159.6421+-0.2686       

                                            TipOfTree            FixInlineGlobal                                 
All benchmarks:
   <arithmetic>                          98.8533+-0.7571         98.4991+-0.1172       
   <geometric>                           25.3745+-0.0676         25.3164+-0.0442       
   <harmonic>                             7.8349+-0.0288          7.8206+-0.0204       

                                            TipOfTree            FixInlineGlobal                                 
Geomean of preferred means:
   <scaled-result>                       58.4237+-0.1967         58.3067+-0.1016
Comment 5 Filip Pizlo 2011-11-02 19:59:13 PDT
Landed in http://trac.webkit.org/changeset/99132