Bug 179765 – Provide a runtime option for disabling the optimization of recursive tail calls

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

[Robin Morisset]

This should make benchmarking and debugging this optimization a bit less painful.
Name of the option: optimizeRecursiveTailCalls

[Robin Morisset]

Created attachment 327060 [details]
Patch

[Mark Lam]

Comment on attachment 327060 [details]
Patch

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

r=me

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1318
> +    if (Options::optimizeRecursiveTailCalls()) {

I suggest making this:
    if (LIKELY(Options::optimizeRecursiveTailCalls())) {

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425
> +    if (!Options::optimizeRecursiveTailCalls())

I suggest making this:
    if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))

[Robin Morisset]

Created attachment 327062 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 327062 [details]
Patch for landing

Clearing flags on attachment: 327062

Committed r224918: <https://trac.webkit.org/changeset/224918>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35588622>

[Saam Barati]

Comment on attachment 327062 [details]
Patch for landing

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

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425
> +    if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))

Why reference this option anywhere else? Isn’t it only  here that we care?

[Robin Morisset]

(In reply to Saam Barati from comment #7)
> Comment on attachment 327062 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=327062&action=review
> 
> > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425
> > +    if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))
> 
> Why reference this option anywhere else? Isn’t it only  here that we care?

The other two places where we reference this option take care of splitting the entry block for functions that have at least one tail call, so the optimisation has somewhere to jump to.
I decided to also make them conditional on the option, to make it easier to benchmark cleanly the cost incurred by this.

[Saam Barati]

(In reply to Robin Morisset from comment #8)
> (In reply to Saam Barati from comment #7)
> > Comment on attachment 327062 [details]
> > Patch for landing
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=327062&action=review
> > 
> > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425
> > > +    if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))
> > 
> > Why reference this option anywhere else? Isn’t it only  here that we care?
> 
> The other two places where we reference this option take care of splitting
> the entry block for functions that have at least one tail call, so the
> optimisation has somewhere to jump to.
> I decided to also make them conditional on the option, to make it easier to
> benchmark cleanly the cost incurred by this.

👍🏼