Question about inlining | How to access named temps in FullBlockClosure?

On Mar 26, 2020, at 5:15 AM, Marcel Taeumel <[hidden email]> wrote:

Hi Eliot,

thanks for the elaborate explanation! Wow, even more things are getting blocked while we are longing for Scorch. :D Could you tell us when you expect to start making your blessing available for Squeak, only very rough? Are we speaking of months? Years? Decades? ;-)

Here're my two cents on the story:

> The check is to find anywhere we see a (non-optimized) block created that closes over an index variable and is passed as an argument.

Hm, what will happen with this:

1 to: 5 do: [:each |

    [:object | object + each] in: [:block | result add: block]].

The left block is not an argument, but still it would be relevant to disable the inlining of the outer block. So your rule has to respect both receiver and arguments, am I right? This will increase the "collateral damage" of unnecessary slow down even further, we would also have to forgo inlining for things like {[:object | object + each] cull: 42} etc.

Or this one:

1 to: 5 do: [:each |

    [result add: thisContext sender top] value; yourself].

And finally:

1 to: 5 do: [:each |

    result add: [thisContext tempVarNamed: 'each']].

You can (almost) always crack the system, so - if we should decide to turn off inlining sometimes - however we do this, in each case, there will be a logical break at some position. In my humble opinion, this would cause more confusion than actually solve problems. There has never been given a guarantee that blocks will be not inlined. If you wish to prevent this, the simplest and clearest solution IMO is still

Best,

Christoph

Hi Eliot, hi all!

I am having issues with the inlining of #to:do: in combination with decompiling a block and accessing the temps.

Here is a piece of code that works as expected:

| result context |

result := OrderedCollection new.

(1 to: 5) do: [:each |

result add: [:object | object + each]].

context := (result at: 3) outerContext.

context namedTempAt: (context tempNames indexOf: 'each'). " == 3"

HOWEVER, using #to:do: the result changes because code gets inlined:

| result context |

result := OrderedCollection new.

1 to: 5 do: [:each |

result add: [:object | object + each]].

context := (result at: 3) outerContext.

context namedTempAt: (context tempNames indexOf: 'each'). " == 6"

What is my goal here? I want to unpack the block statement and fill in the temps. For all those [:object | object + each] in "result" I want to collect the strings:

-> 'object + 1'

-> 'object + 2'

-> 'object + 3'

-> 'object + 4'

-> 'object + 5'

Is there a way to access the values of temps when the block closures are created this way?

Best,

Marcel

On Mar 26, 2020, at 10:42 AM, Thiede, Christoph <[hidden email]> wrote:

Here're my two cents on the story:

> The check is to find anywhere we see a (non-optimized) block created that closes over an index variable and is passed as an argument.

Hm, what will happen with this:

1 to: 5 do: [:each |

    [:object | object + each] in: [:block | result add: block]].

The left block is not an argument, but still it would be relevant to disable the inlining of the outer block. So your rule has to respect both receiver and arguments, am I right? This will increase the "collateral damage" of unnecessary slow down even further, we would also have to forgo inlining for things like {[:object | object + each] cull: 42} etc.

Or this one:

1 to: 5 do: [:each |

    [result add: thisContext sender top] value; yourself].

And finally:

1 to: 5 do: [:each |

    result add: [thisContext tempVarNamed: 'each']].

You can (almost) always crack the system, so - if we should decide to turn off inlining sometimes - however we do this, in each case, there will be a logical break at some position. In my humble opinion, this would cause more confusion than actually solve problems. There has never been given a guarantee that blocks will be not inlined.

If you wish to prevent this, the simplest and clearest solution IMO is still

1 to: 5 do: [:each |

    result add: [:object | object + each]

] yourself. 

Best,

Christoph

---

Von: Squeak-dev <[hidden email]> im Auftrag von Eliot Miranda <[hidden email]>
Gesendet: Donnerstag, 26. März 2020 17:20:08
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Question about inlining | How to access named temps in FullBlockClosure?

 

Hi Marcel,

On Mar 26, 2020, at 5:15 AM, Marcel Taeumel <[hidden email]> wrote:

Hi Eliot, hi all!

I am having issues with the inlining of #to:do: in combination with decompiling a block and accessing the temps.

Here is a piece of code that works as expected:

| result context |

result := OrderedCollection new.

(1 to: 5) do: [:each |

result add: [:object | object + each]].

context := (result at: 3) outerContext.

context namedTempAt: (context tempNames indexOf: 'each'). " == 3"

HOWEVER, using #to:do: the result changes because code gets inlined:

| result context |

result := OrderedCollection new.

1 to: 5 do: [:each |

result add: [:object | object + each]].

context := (result at: 3) outerContext.

context namedTempAt: (context tempNames indexOf: 'each'). " == 6"

What is my goal here? I want to unpack the block statement and fill in the temps. For all those [:object | object + each] in "result" I want to collect the strings:

-> 'object + 1'

-> 'object + 2'

-> 'object + 3'

-> 'object + 4'

-> 'object + 5'

Is there a way to access the values of temps when the block closures are created this way?

Nice one.  So the thing to realize is that, because of inlining, the second example is in fact equivalent to

| result context each |

result := OrderedCollection new.

each := 1.

[ each <= 5 ] whileTrue:

    [ result add: [:object | object + each]].

context := (result at: 3) outerContext.

context namedTempAt: (context tempNames indexOf: 'each') "= 6"

and when written like this it is clear that the search for the value will end up being as found, 6.

So then the thing to realize is that the inlining decision is wrong.  That in fact the closing over of each (the index variable of the inlined to:do:) within an uninlined inner block means that the outer block cannot be inlined without breaking the inner block.

So the next thing to determine is whether the compiler can easily be modified to identify this case and hence prevent inlining in case like these.

After that we have to assess and find out how many cases in the system (eg in trunk) would no longer be inlined and decide whether the cure is worse than the disease.  The issue here is two fold, the loss of performance due to not inlining, and the slow down to compilation due to the check(s) needed to prevent inlining in this case.

Why would adding the check potentially cause many to:[by:]do: invocations to no longer be inlined? The check is to find anywhere we see a (non-optimized) block created that closes over an index variable and is passed as an argument.  Because this is a dynamic language we cannot assume anything about the message the block is an argument to; the programmer could redefine an implementation of a method with that selector to capture the block argument just as your example does.  So we have to assume the block could be captured, and hence that the to:[by:]do: in which it occurs cannot be inlined.

Other Smalltalks have decided to live with the disease.  I think that the right decision is *not* to live with the disease, but to fix the inliner, and wait for Scorch to “do the right thing” and inline dynamically.  Scorch can inline precisely because it does indeed find out about the caller, and inline if the caller does not capture the block, and adds the dependency information to enable the optimized version to be discarded if redefinition occurs.

The first thing to do is write a test asserting the correct behaviour.

The second thing to do is learn how and where the bytecode compiler decides whether or not to inline to:[by:]do:.

The third thing to do is to modify that inlining decision to generate correct code for your case.

The fourth thing to do is then to find out how many methods are affected.

Finally we can then assess whether we want to live with the disease or not.

Best,

Marcel

Best, Eliot

_,,,^..^,,,_ (phone)