Bug 73175 – DFG should not emit GetMethod node

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

The GetMethod node used to serve as an optimization for GetById, if the GetById was being syntactically used as the base of a method call.  But since then, we have replaced all of the profitable cases of GetMethod with a pair of CheckStructure nodes, and a pair of WeakJSConstant nodes.  Hence the GetMethod node is currently only being emitted when we know, based on profiling, that the GetMethod optimization had actually failed.

We should simply emit a normal GetById node, if we know that it is unprofitable to emit a pair of CheckStructure/WeakJSConstant nodes.

[Filip Pizlo]

Created attachment 116688 [details]
the patch

If anything, this is neutral.  Though it does seem to have hints of a win on V8.


Benchmark report for SunSpider, V8, and Kraken on nitroflex.local (MacBookPro8,2).

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (r101201)
"NoGetMethod" at /Volumes/Data/pizlo/OpenSource/WebKitBuild/Release/jsc (r101201)

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              NoGetMethod                                   
SunSpider:
   3d-cube                                7.1764+-0.1742          6.8968+-0.1244         might be 1.0405x faster
   3d-morph                               7.9859+-0.3041          7.8839+-0.1212         might be 1.0129x faster
   3d-raytrace                            7.2032+-0.2497    ?     7.3030+-0.1322       ? might be 1.0138x slower
   access-binary-trees                    1.5779+-0.0552          1.5116+-0.0364         might be 1.0439x faster
   access-fannkuch                        6.3811+-0.1948          6.1966+-0.0416         might be 1.0298x faster
   access-nbody                           3.5178+-0.0694    ?     3.5251+-0.0510       ?
   access-nsieve                          2.5911+-0.0593    ?     2.5923+-0.0493       ?
   bitops-3bit-bits-in-byte               1.2408+-0.0188    ?     1.2525+-0.0257       ?
   bitops-bits-in-byte                    2.4423+-0.0506          2.4300+-0.0808       
   bitops-bitwise-and                     3.4546+-0.0736          3.3714+-0.0364         might be 1.0247x faster
   bitops-nsieve-bits                     5.3373+-0.0818    ?     5.3556+-0.0673       ?
   controlflow-recursive                  2.0290+-0.0279    ?     2.0734+-0.0300       ? might be 1.0219x slower
   crypto-aes                             7.3307+-0.2431          7.2280+-0.1415         might be 1.0142x faster
   crypto-md5                             2.4306+-0.0582    ?     2.4351+-0.0529       ?
   crypto-sha1                            2.1583+-0.1186    ?     2.1933+-0.0843       ? might be 1.0162x slower
   date-format-tofte                     10.3251+-0.2437         10.1724+-0.2506         might be 1.0150x faster
   date-format-xparb                      9.5827+-0.1299    ?    10.2392+-0.5614       ? might be 1.0685x slower
   math-cordic                            6.3008+-0.0530    ?     6.3821+-0.0956       ? might be 1.0129x slower
   math-partial-sums                      7.6734+-0.1439          7.5819+-0.1422         might be 1.0121x faster
   math-spectral-norm                     2.3584+-0.0326    ?     2.3878+-0.0569       ? might be 1.0125x slower
   regexp-dna                            11.1242+-0.2322         11.1141+-0.2045       
   string-base64                          3.8605+-0.0873    ?     3.8903+-0.1043       ?
   string-fasta                           6.5896+-0.0792    !     6.8609+-0.1590       ! definitely 1.0412x slower
   string-tagcloud                       11.5641+-0.2413         11.4625+-0.3104       
   string-unpack-code                    20.4668+-0.3587         20.4124+-0.3204       
   string-validate-input                  5.5790+-0.1431          5.4070+-0.0983         might be 1.0318x faster

   <arithmetic> *                         6.0878+-0.0354          6.0830+-0.0295         might be 1.0008x faster
   <geometric>                            4.8526+-0.0254          4.8472+-0.0226         might be 1.0011x faster
   <harmonic>                             3.8238+-0.0239          3.8198+-0.0231         might be 1.0011x faster

                                            TipOfTree              NoGetMethod                                   
V8:
   crypto                                71.4757+-0.5945    ?    71.6543+-0.8676       ?
   deltablue                            152.1513+-1.2838        151.8126+-1.0527       
   earley-boyer                          85.1455+-0.9026    ?    87.0858+-1.3455       ? might be 1.0228x slower
   raytrace                              56.6661+-0.2108         56.6320+-0.4331       
   regexp                               106.8908+-1.4783    ^   104.3510+-0.3253       ^ definitely 1.0243x faster
   richards                             119.1301+-0.3283    ^   117.3293+-0.2406       ^ definitely 1.0153x faster
   splay                                 75.3844+-1.1653         73.4300+-0.9663         might be 1.0266x faster

   <arithmetic>                          95.2634+-0.3838         94.6136+-0.3198         might be 1.0069x faster
   <geometric> *                         90.6590+-0.3797         90.0993+-0.3332         might be 1.0062x faster
   <harmonic>                            86.4365+-0.3894         85.9689+-0.3435         might be 1.0054x faster

                                            TipOfTree              NoGetMethod                                   
Kraken:
   ai-astar                             491.0792+-0.4440    ?   494.3727+-4.1607       ?
   audio-beat-detection                 192.6858+-2.6961        192.5974+-1.3912       
   audio-dft                            276.7559+-5.9948        270.8880+-3.6428         might be 1.0217x faster
   audio-fft                            126.6115+-0.8848        125.7453+-0.7885       
   audio-oscillator                     252.6736+-3.4506        251.3422+-3.0310       
   imaging-darkroom                     305.5309+-4.4214        303.5602+-4.0916       
   imaging-desaturate                   217.7886+-1.3524        217.6962+-1.2634       
   imaging-gaussian-blur                559.9125+-3.2074        559.3110+-3.3036       
   json-parse-financial                  59.3956+-0.4921         59.1615+-0.4678       
   json-stringify-tinderbox              74.2847+-0.2681    !    76.2764+-1.0000       ! definitely 1.0268x slower
   stanford-crypto-aes                   99.3376+-1.1413    ?    99.9840+-1.1110       ?
   stanford-crypto-ccm                  103.0129+-1.2054    ?   104.6516+-1.4472       ? might be 1.0159x slower
   stanford-crypto-pbkdf2               193.9439+-0.7385    ^   190.5408+-1.1591       ^ definitely 1.0179x faster
   stanford-crypto-sha256-iterative      85.1716+-0.7458         84.8109+-0.3782       

   <arithmetic> *                       217.0132+-0.4951        216.4956+-0.5738         might be 1.0024x faster
   <geometric>                          173.9576+-0.3890        173.8070+-0.4777         might be 1.0009x faster
   <harmonic>                           140.9356+-0.2903    ?   141.2115+-0.4391       ? might be 1.0020x slower

                                            TipOfTree              NoGetMethod                                   
All benchmarks:
   <arithmetic>                          82.1981+-0.1766         81.9445+-0.1872         might be 1.0031x faster
   <geometric>                           21.7955+-0.0763         21.7565+-0.0628         might be 1.0018x faster
   <harmonic>                             6.7338+-0.0413          6.7265+-0.0398         might be 1.0011x faster

                                            TipOfTree              NoGetMethod                                   
Geomean of preferred means:
   <scaled-result>                       49.2923+-0.1406         49.1388+-0.0899         might be 1.0031x faster

[Gavin Barraclough]

Comment on attachment 116688 [details]
the patch

r+, keeping the GetMethod code for now seems okay, but we shouldn't leave redundant code fester for too long, so we should do something to track this – maybe open another bug indicating the rest of the GetMethod code should be removed at some point?

[Filip Pizlo]

(In reply to comment #2)
> (From update of attachment 116688 [details])
> r+, keeping the GetMethod code for now seems okay, but we shouldn't leave redundant code fester for too long, so we should do something to track this – maybe open another bug indicating the rest of the GetMethod code should be removed at some point?

Agree!  https://bugs.webkit.org/show_bug.cgi?id=73178

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/101217

[Filip Pizlo]

Comment on attachment 116688 [details]
the patch

Clearing flags.