Bug 117906 - fourthTier: DFG shouldn't exit just because a String GetByVal went out-of-bounds
Summary: fourthTier: DFG shouldn't exit just because a String GetByVal went out-of-bounds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-21 22:58 PDT by Filip Pizlo
Modified: 2013-06-22 21:57 PDT (History)
7 users (show)

See Also:


Attachments
work in progress (13.26 KB, patch)
2013-06-21 22:58 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the ptach (22.74 KB, patch)
2013-06-21 23:24 PDT, Filip Pizlo
mhahnenberg: 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 2013-06-21 22:58:06 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2013-06-21 22:58:58 PDT
Created attachment 205235 [details]
work in progress
Comment 2 Filip Pizlo 2013-06-21 23:24:28 PDT
Created attachment 205237 [details]
the ptach

Benchmark report for JSRegress on dethklok (MacBookPro9,1).

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/fourthTier/secondary/OpenSource/WebKitBuild/Release/jsc (r151875)
"FixString" at /Volumes/Data/pizlo/fourthTier/primary/OpenSource/WebKitBuild/Release/jsc (r151873)

Collected 30 samples per benchmark/VM, with 10 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                 FixString                                     

string-get-by-val-big-char                    8.6687+-0.3165            8.4542+-0.2872          might be 1.0254x faster
string-get-by-val-out-of-bounds-insane        7.2589+-0.3855     ^      5.9114+-0.2240        ^ definitely 1.2280x faster
string-get-by-val-out-of-bounds              54.9499+-0.7410     ^      2.5956+-0.0318        ^ definitely 21.1704x faster
string-get-by-val                             2.3552+-0.1035            2.3321+-0.0938        

<arithmetic>                                 18.3082+-0.2274     ^      4.8233+-0.0892        ^ definitely 3.7958x faster
<geometric> *                                 9.4639+-0.1531     ^      4.1602+-0.0602        ^ definitely 2.2749x faster
<harmonic>                                    5.7086+-0.1329     ^      3.6167+-0.0511        ^ definitely 1.5784x faster
Comment 3 Mark Hahnenberg 2013-06-22 10:16:33 PDT
Comment on attachment 205237 [details]
the ptach

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

r=me

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2060
> +            m_jit.addLazily(

Just for my education, what does addLazily do/what is its purpose?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2100
> +    

:-(
Comment 4 Filip Pizlo 2013-06-22 10:21:11 PDT
(In reply to comment #3)
> (From update of attachment 205237 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205237&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2060
> > +            m_jit.addLazily(
> 
> Just for my education, what does addLazily do/what is its purpose?

You can't add to a watchpoint set during DFG compilation anymore, since DFG compilation is concurrent.  So you just register your intent to be added.

Then when the main thread installs the code, it checks that the desired watchpoints are still valid; if they aren't then the compilation is discarded and if they are then the watchpoints are installed.

> 
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2100
> > +    
> 
> :-(
Comment 5 Mark Hahnenberg 2013-06-22 10:23:03 PDT
> 
> You can't add to a watchpoint set during DFG compilation anymore, since DFG compilation is concurrent.  So you just register your intent to be added.
> 
> Then when the main thread installs the code, it checks that the desired watchpoints are still valid; if they aren't then the compilation is discarded and if they are then the watchpoints are installed.

Cool, that's what I thought.
Comment 6 Filip Pizlo 2013-06-22 21:57:53 PDT
Landed in http://trac.webkit.org/changeset/151885