[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
|