[squeak-dev] modifying copied context vars

Eliot Miranda eliot.miranda at gmail.com
Sat Mar 26 00:58:36 UTC 2016


Hi Nicolai,

On Fri, Mar 25, 2016 at 2:51 AM, Nicolai Hess <nicolaihess at gmail.com> wrote:

>
>
> 2016-03-25 6:26 GMT+01:00 Eliot Miranda <eliot.miranda at gmail.com>:
>
>> Hi Nicolai,
>>
>> > On Mar 24, 2016, at 1:53 AM, Nicolai Hess <nicolaihess at gmail.com>
>> wrote:
>> >
>> > Hello,
>> >
>> > we have a failing test (OCClosureCompilerTest>>#testDebuggerTempAccess)
>> > (fails since spur, but not related - I think).
>> >
>> > A simple example to reproduce the behavior:
>> > | local1 remote1|
>> > remote1:=4.
>> > local1 :=3.
>> > [:i | |c|
>> >     c := local1.
>> >     remote1 := i.
>> >     i halt.
>> >     "local1:=25.  <-- evaluate during debugging"
>> >      ] value:1.
>> > Transcript show:local1;cr.
>> > Transcript show:remote1;cr.
>> > (Executing this code and evaluating "local:=25" after the debugger
>> halts for
>> > "i halt" will modify the var "remote1" instead of "local1", as this is
>> a vector tempvar , proceeding
>> > the execution will crash at accessing the remote1 value).
>> >
>> >
>> > The purpose of the testDebuggerTempAccess test case is, evaluating code
>> > that changes the value of a copied temp var *does not* change the value
>> of
>> > this var in the outer context. (see comment in the test case:
>> > "this is not 25 as the var is a local, non escaping variable that was
>> copied into the block,
>> >     If the compiler would have known about the write, it would have
>> made the var escaping".
>> > )
>> >
>> > But the implementation (OCCopyingTempVariable>>#writeFromContext:
>> aContext scope: contextScope value: aValue)
>> > explicitly iterates through all outer context(s) and changes the vars
>> as well.
>> >
>> > 1. Question: Who is right?
>>
>> What do you mean?  Because the closure model is as it is (for very good
>> reason) the computer copies temporaries whose values cannot change during
>> the execution of a block into that block if the block references the
>> variable.  If the debugger is to support updating such copied variables
>> then it must create the illusion of "the variable being updated" because
>> there isn't just one variable.
>>
>
> I know that it only operates on a copy of this var (if the var is only
> read (in the original code)), but you *can* evaluate code
> during debugging, that can modify the var.
> I don't know why or if we want to support that. I just see a failing test
> case that tries to do exactly that.
> And the supposed behavior is, that the outer context variable value does
> not change.
>

But it would be much nicer if the debugger /did/ change all the copies of
the variable it could find, right?  What behaviour do we want to support?
What behaviour can we afford to support?



> But this test fails.
>
> I tried similar code in squeak, and there the outer context var does not
> change:
>
> |local remote outerContext|
> local :=1.
> remote:=2.
> outerContext := thisContext.
> (1 to:1) do:[:i |
>     remote := i.
>     Transcript show:local;cr.
>     i halt.
>     "remote:=20"
>     "local:=10"
>     "outerContext tempAt:1 put:30"
>     Transcript show:local;cr.
>     ].
> Transcript show:local;cr.
> Transcript show:remote;cr.
>
>
> If the debugger halts at "i halt" and you evaluate the code
>
>     "remote:=20"
> the remote value changes for t he block context and for its outer context
> - OK
>     "local:=10"
> the local value changes for the block context but not for its outer
> context - OK
>     "outerContext tempAt:1 put:30"
> the local value only changes for the outer context - OK
>
> Is this right?
>

I don't think it's either right or wrong; I think it's what one would
expect from a naive debugger and the current compilation semantics.  I
think you need to think about the problem at a tool level.

The right question to ask is "what behaviour would you like in the
debugger?"  In a debugger there are typically two interfaces that allow
modifying the variables in a context. One is via a doit in which the
variables are in scope, and one is an inspector on the variables embedded
in a debugger.  It would be nice if the two were consistent.  But it is
easier to make them consistent if only local values are changed.  The issue
is really how powerful you want to map DebuggerMap (or whatever the Pharo
equivalent).

Here are some issues to think about.  It's easy for a debugger map to work
out what the right indices are for a varable that is copied in various
scopes; it just asks the compiler for the relevant info.  It is /not/ easy
to find out all the potentially copied variables.  For example, the
following creates many block and many activations of those blocks, and a
copy of "copied" is in all of them.

| copied |
copied := 7.
^(1 to: copied) collect:
    [:i| [Semaphore new wait. copied] fork. [Semaphore new wait. copied]]

Just before the collect: terminates execution there are 9 contexts
accessing copied; the context declaring it, the context for the block [:i|
[Semaphore...copied]], and the 7 contexts waiting on a new semaphore.
There are 15 blocks whose copiedValues includes copied, the block [:i|
[Semaphore...copied]], the 7 copies of the first [Semaphore new wait.
copied] block, which all have contexts whose closureOrNil points to them,
and seven 7 copies of the second [Semaphore new wait. copied] block.  So
the only way to reliably change copy is to do allInstances on Context and
BlockClosure and collect all whose home context is the context that
declares copied and to change the 0th temp or the 0th copied value in each
context or block.  Is it worth it?  Is it perhaps too expensive in some
circumstances, and hence needs to be a preference?  Is the preference too
hard to understand or explain?

An alternative might be to warn the debugger user, saying something like
"warning, not all references to temporary variable foo can be updated." and
update only in activations on the stack.

Personally I would implement the all instances approach if I had the time.
But I haven't had time for 8 years ;-)


> But the reason why the test actually fails is different. It is because
>> evaluating
>> > the code that modifies the var "local1:=25" ends up to a call
>> > aContext tempAt: self indexFromIR put: aValue
>> > but the index (indexFromIR) can be different for every context (method
>> context / inner block
>> > context(s)).
>>
>> Right.
>>
>> >
>> > 2. Question: should the index be always the same?
>>
>> How can it be?  If, for example, a nullary block makes use of a copied
>> variable and has no local temps then that variable will end up with index
>> 0, no matter its indices in outer scopes.  Surely you're not proposing
>> padding the block with unused variables just so copied variables can have
>> the same index?
>>
>
> Yes, makes sense.
>
>
>>
>> >
>> > thanks inadvance
>> >
>> > Nicolai
>> >
>> >
>> > ps: since spur (resp. compiler changes that were done for new spur
>> images), the index of tempvars can be different.
>> > In pre-spur, this testcase modifies an outer context var, but one that
>> isn't checked (the argument "two"), therefore
>> > the test succeed. In spur, this testcase modifes a different outer
>> context var "remote1", and the test
>> > fails.
>>
>>
>> Since Sour hasn't changed the compiler there looks to be a bug.  Sour has
>> changed the identityHash size a lot from IIRC 11 bits to 22.  So hashed
>> collections can end up with different enumerations.  But that shouldn't
>> affect the ordering assigned to temps in the compiler.  I suggest tracking
>> down why the code is different in Spur is a very important thing to do.
>>
>
> Yes, we merged some code for spur support (opals compiler code) and lost
> some intermediate changes that were made for opal. Debugging this bugs was
> a mess.
>

Ouch.  Debugging the debugger.  Always frustrating.  Does Opal put the
indirection vectors in the same place in each declaring scope?  Also what
rule does Opal use for ordering copied values?  Here's the rule for
Squeak's compiler:

ParseNode class>>*tempSortBlock*
"Answer a block that can sort a set of temporaries into a stable
order so that different compilations produce the same results."
^[:t1 :t2| | be1 be2 bs1 bs2 |
  t1 index < t2 index "simple sort by index."
  or: [t1 index = t2 index "complex tie break"
 and: [t1 isRemote ~= t2 isRemote
ifTrue: [t2 isRemote] "put direct temps before indirect temps"
ifFalse:
[((be1 := t1 definingScope blockExtent) isNil
 or: [(be2 := t2 definingScope blockExtent) isNil])
ifTrue: [t1 name < t2 name] "only have the name left to go on"
ifFalse: "put temps from outer scopes before those from inner scopes"
[(bs1 := be1 first) < (bs2 := be2 first)
or: [bs1 = bs2 and: [t1 name < t2 name]]]]]]] "only have the name left to
go on"

Hence in the following:

| c1 r1 c2 r2 |
c1 := #(copied 1). c2 := #(copied 2).
r1 := #(remote 1). r2 := #(remote 2).
[ | c3 r3 |
  c3 := #(copied 3).
  r3 :=  #(remote 3).
  [r1 := r1 copy. r2 := r2 copy. r3 := r3 copy.
   { c1. c2. c3. thisContext shallowCopy}] value] value

Numbering the scopes 1, 2 & 3, then

Scope 1:
0: c1
1: c2
2: indirection1, r1 at 1, r2 at 2

Copied values in block for scope 2:
1: c1
2: c2
3: indirection1

Scope 2:
1: c1
2: c2
3: indirection1
4: c3
5: indirection2

Copied values in block for scope 3:
1: c1
2: c2
3: indirection1
4: c3
5: indirection2

Scope 3:
1: c1
2: c2
3: indirection1
4: c3
5: indirection2


Intuitively I expected

Scope 1:
0: c1
1: c2
2: indirection1, r1 at 1, r2 at 2

Copied values in block for scope 2:
1: c1
2: c2
3: indirection1

Scope 2:
1: c1
2: c2
3: indirection1
4: c3
5: indirection2

Copied values in block for scope 3:
1: c1
2: c2
3: c3
4: indirection1
5: indirection2

Scope 3:
1: c1
2: c2
3: c3
4: indirection1
5: indirection2


:-/


HTH
>>
>> Eliot
>>
>
> thanks
>
>
>> _,,,^..^,,,_ (phone)
>>
>> PS if you want to understand why copied variables are so important in the
>> closure model you need to understand context-to-stack mapping and the
>> overhead that not copying temps adds to returns in a context-to-stack
>> mapping VM.  It's not obvious but I hope my blog does an ok job of
>> explaining something alas complex but extremely important for performance.
>>
>
> I read your blog
>

Good.  So then ask yourself what the right thing to do is, and what the
affordable thing to do is.  You understand the issue, now you have to make
some decisions...

_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20160325/13b8784b/attachment.htm


More information about the Squeak-dev mailing list