[squeak-dev] Re: [Pharo-project] Prune stack serialization

Eliot Miranda eliot.miranda at gmail.com
Fri Dec 9 23:31:18 UTC 2011


On Fri, Dec 9, 2011 at 3:01 PM, Mariano Martinez Peck <marianopeck at gmail.com
> wrote:

> Hi guys. When we implemented this possibility of pruning the closures when
> they were clean, one of our crazy tests started to fail. It is not that the
> test is very important, but I am interested in trying to understand why it
> fails. The test (simplied) is:
>
>     | aSortedCollection materialized |
>     instanceVariableForTesting := false.
>
>     aSortedCollection := SortedCollection sortBlock: [:a :b |
>         instanceVariableForTesting
>             ifTrue: [ a <= b ]
>             ifFalse: [ a >= b ] ].
>
>     materialized := self resultOfSerializeAndMaterialize:
> aSortedCollection.
>
>     materialized addAll: #(2 3 1).
>     aSortedCollection addAll: #(2 3 1).
>
>
> Well, the thing is that the materialized SortedCollection, it is
> sortBLock, the closure is quite crazy and the 'instanceVariableForTesting'
> doesn't have the boolean but instead a #Processor    WTF??   So then the
> #addAll:  fails with a MustBeBoolean.
>
> Do you think there is something I should check for this closure?  I think
> it may have to be related with the instanceVariable...
>

Clearly the closure's copied value (instanceVariableForTesting), its 1st
indexable variable, should be a boolean.  Just explore
    | instanceVariableForTesting |
    instanceVariableForTesting := false.
    [:a :b | instanceVariableForTesting ifTrue: [ a <= b ] ifFalse: [ a >=
b ] ]
and you'll see the closure's 1st indexable variable is false.  So why is it
being (de)serialized as a Processor?  There's your bug.



> Just for the record, what we are doing is something like:
>
> serializeReferencesOf: aBlockClosure with: aSerialization
>
>     1 to: aBlockClosure basicSize do: [ :index |
>         aSerialization encodeReferenceTo: (aBlockClosure basicAt: index) ].
>
>     aBlockClosure isClean
>         ifTrue: [
>             aSerialization stream nextPut: 1.
>             aSerialization encodeReferenceTo:  aBlockClosure method.
>             aSerialization encodeReferenceTo:  aBlockClosure receiver.
>             ]
>         ifFalse: [
>             aSerialization stream nextPut: 0.
>             aSerialization encodeReferenceTo:  aBlockClosure outerContext.
>         ].
>
>     aSerialization encodeReferenceTo:  aBlockClosure startpc.
>     aSerialization encodeReferenceTo:  aBlockClosure numArgs.
>
>
>
> -----
>
> materializeReferencesOf: aBlockClosure with: aMaterialization
>
>     | methodOrContext method context |
>     1 to: aBlockClosure basicSize do: [ :index |
>         aBlockClosure basicAt: index put: (aMaterialization
> nextEncodedReference).
>     ].
>
>     (aMaterialization stream next = 1)
>         ifTrue: [ " it was a clean BlockClosure, otherwise it should have
> been 0"
>             method := aMaterialization nextEncodedReference.
>             aBlockClosure
>                     outerContext:     (MethodContext
>                                         sender: nil
>                                         receiver: aMaterialization
> nextEncodedReference
>                                         method: method
>                                         arguments: #() )
>                     startpc: aMaterialization nextEncodedReference
>                     numArgs: aMaterialization nextEncodedReference.
>
>             ]
>         ifFalse: [
>                 context := aMaterialization nextEncodedReference.
>                 aBlockClosure
>                     outerContext: context
>                     startpc: aMaterialization nextEncodedReference
>                     numArgs: aMaterialization nextEncodedReference].
>
>
> -----
>
>
> Thanks in advance!
>
>
>
>
>
> On Sun, Dec 4, 2011 at 8:33 PM, Mariano Martinez Peck <
> marianopeck at gmail.com> wrote:
>
>> Thanks Juan. With the latest version of Eliot, the following are failing:
>>
>>
>>   self deny: [ | outerBlockTemp | [ outerBlockTemp printString ] isClean
>> ] value.
>>   self deny: [ | outerBlockTemp | [ outerBlockTemp :=7 ] isClean ] value.
>>   self deny: [ tempVar + 1 ] isClean.
>>   self deny: [ tempVar := 1 ] isClean.
>>   self deny: [ ClassVar + 1 ] isClean.
>>   self deny: [ ClassVar := 1 ] isClean.
>>
>> Thanks
>>
>>
>> On Sat, Dec 3, 2011 at 3:01 AM, Juan Vuletich <juan at jvuletich.org> wrote:
>>
>>> Eliot Miranda wrote:
>>>
>>>
>>>>
>>>> On Fri, Dec 2, 20Tha11 at 4:01 PM, Juan Vuletich <juan at jvuletich.org<mailto:
>>>> juan at jvuletich.org>> wrote:
>>>>
>>>>    Eliot Miranda wrote:
>>>>
>>>>        Hi Juan,
>>>>
>>>>        ...
>>>>
>>>>
>>>>
>>>>           I can say that it does indeed work. But I'd be really
>>>>        grateful if
>>>>           Eliot took a good look at that code!
>>>>
>>>>
>>>>        There is a much better way, using
>>>>        InstructionStream>>**interpreetNextInstructionFor: which
>>>>        abstracts away from the bytecode encoding and allows one to
>>>>        simply use messages.  e.g. look at
>>>>
>>>>        and (SortedCollection sortBlock: [:a :b| a compare: b
>>>>        caseSensitive: false]) sortBlock abstractBytecodes evaluates to
>>>>
>>>>        an OrderedCollection(#**pushTemporaryVariable:
>>>>        #pushTemporaryVariable: #pushConstant: #send:super:numArgs:)
>>>>
>>>>        So a check for clean blocks would check for a block's
>>>>        abstractBytecodes not including some particular set of banned
>>>>        messages (which I *think* is pushReceiver
>>>>        pushReceiverVariable: popIntoReceiverVariable:
>>>>        storeIntoReceiverVariable: methodReturnConstant:
>>>>        methodReturnReceiver methodReturnTop).
>>>>
>>>>        e.g.
>>>>
>>>>        isClean
>>>>             ^(self abstractBytecodes intersection: #(pushReceiver
>>>>        pushReceiverVariable: popIntoReceiverVariable:
>>>>        storeIntoReceiverVariable: methodReturnConstant:
>>>>        methodReturnReceiver methodReturnTop)) isEmpty
>>>>
>>>>
>>>>           Cheers,
>>>>           Juan Vuletich
>>>>
>>>>
>>>>
>>>>
>>>>        --         best,
>>>>        Eliot
>>>>
>>>>
>>>>    Thanks Eliot. However, it looks to me that this needs more tweaking.
>>>>
>>>> <blush>Damn right :)  I've done zero testing.  It was just an
>>>> example</blush>
>>>>
>>>>    For instance, your method answers true for
>>>>      [ xx size ] isClean
>>>>    and
>>>>      [ ^ nil ] isClean
>>>>    while mine answers false for both. Maybe we could serialize non
>>>>    local returns using the Compiler (although we won't be able to
>>>>    return anywhere), but external variable acess is a problem.
>>>>
>>>>    Will keep playing with this for a while...
>>>>
>>>>
>>>> Can you (can I??) write some tests?  What's xx in the above?  Ah!
>>>> [^nil] isn't clean in my code since it should be pc <= end, not pc < end!!
>>>>  Try the attached...
>>>>
>>>
>>> This is a start:
>>>
>>> testIsClean
>>>   "
>>>   ClosureTests new testIsClean
>>>   "
>>>   | tempVar |
>>>   tempVar := 1.
>>>   self assert: [ 3 + 4 ] isClean.
>>>   self assert: [ :a | a * 2 ] isClean.
>>>   self assert: [ Smalltalk size ] isClean.
>>>   self assert: [ :blockArg | blockArg printString ] isClean.
>>>   self assert: [ | blockTemp | blockTemp printString ] isClean.
>>>   self assert: [ | blockTemp | blockTemp :=7 ] isClean.
>>>   self deny: [ | outerBlockTemp | [ outerBlockTemp printString ] isClean
>>> ] value.
>>>   self deny: [ | outerBlockTemp | [ outerBlockTemp :=7 ] isClean ] value.
>>>   self deny: [ tempVar + 1 ] isClean.
>>>   self deny: [ tempVar := 1 ] isClean.
>>>   self deny: [ ivar + 1 ] isClean.
>>>   self deny: [ ivar := 1 ] isClean.
>>>   self deny: [ ^ true ] isClean.
>>>   self deny: [ self printString ] isClean.
>>>   self deny: [ ^ self ] isClean.
>>>   self deny: [ ClassVar + 1 ] isClean.
>>>   self deny: [ ClassVar := 1 ] isClean.
>>>
>>> This test passes with my implementation.
>>>
>>>
>>>
>>>>
>>>>    Cheers,
>>>>    Juan Vuletich
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> best,
>>>> Eliot
>>>>
>>>>
>>> Cheers,
>>> Juan Vuletich
>>>
>>>
>>
>>
>> --
>> Mariano
>> http://marianopeck.wordpress.com
>>
>>
>
>
> --
> Mariano
> http://marianopeck.wordpress.com
>
>
>
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20111209/fb840a51/attachment.htm


More information about the Squeak-dev mailing list