[squeak-dev] The Inbox: CollectionsTests-dtl.212.mcz

Levente Uzonyi leves at elte.hu
Sun Jan 19 21:03:11 UTC 2014


On Sun, 19 Jan 2014, Levente Uzonyi wrote:

> On Sun, 19 Jan 2014, Levente Uzonyi wrote:
>
>> 
>> 
>> On Sun, 19 Jan 2014, David T. Lewis wrote:
>> 
>>> On Sun, Jan 19, 2014 at 06:06:37PM +0100, Levente Uzonyi wrote:
>>>> On Sun, 19 Jan 2014, Nicolas Cellier wrote:
>>>> 
>>>>> It's a bug. Let's remove the optimizatoin. If we really want it, this
>>>>> could be a implemented like
>>>>> 
>>>>> WriteStream>>sharedContents? | sz | ^(sz := position max: readLimit) =
>>>>> collection size ifTrue: [collection] ifFalse: [collection copyFrom: 1 
>>>>> to:
>>>>> sz]
>>>> 
>>>> #new:streamContents: is about optimization, nothing else. I think it's a
>>>> better solution to set the stream to the end before comparing the 
>>>> position
>>>> with the size, because I don't see any other possible use for
>>>> #sharedContents.
>>> 
>>> Do you mean something like this?
>>>
>>>  new: newSize streamContents: blockWithArg
>>>      | stream p endPos |
>>>      stream := WriteStream on: (self new: newSize).
>>>      blockWithArg value: stream.
>>>      p := stream position.
>> 
>> It doesn't matter what the current position of the stream is. If there's 
>> data after the current position, then #contents will return it. So I think 
>> this is
>
> Um, it seems like #contents only cares about position, but not readLimit 
> (even though it updates readLimit), which may or may not be a bug...

Well, it's not a bug, but me being a bit sleepy. All we need to do is to 
check if the stream is still using the original collection.

new: newSize streamContents: blockWithArg

 	| stream collection |
 	collection := self new: newSize.
 	stream := WriteStream on: collection.
 	blockWithArg value: stream.
 	(stream originalContents == collection and: [ stream position = newSize ])
 		ifTrue: [ ^collection ]
 		ifFalse: [ ^stream contents ]

Or another variant, which checks the size of #originalContents.

new: newSize streamContents: blockWithArg

 	| stream originalContents |
 	stream := WriteStream on: (self new: newSize).
 	blockWithArg value: stream.
 	originalContents := stream originalContents.
 	(originalContents size = newSize and: [ stream position = newSize ])
 		ifTrue: [ ^originalContents ]
 		ifFalse: [ ^stream contents ]


Levente

>
>
> Levente
>
>> how the method should be:
>> 
>> new: newSize streamContents: blockWithArg
>>
>> 	| stream |
>> 	stream := WriteStream on: (self new: newSize).
>> 	blockWithArg value: stream.
>> 	stream setToEnd position = newSize
>> 		ifTrue: [ ^stream originalContents ]
>> 		ifFalse: [ ^stream contents ]
>> 
>> 
>> Levente
>>
>>>      endPos := stream setToEnd position.
>>>      p = endPos
>>>          ifTrue: [p = newSize
>>>                  ifTrue: [^ stream originalContents]]
>>>          ifFalse: [stream position: p].
>>>      ^ stream contents
>>> 
>>> 
>>> I'm not sure what that would do to the performance gains.
>>> 
>>> Dave
>>> 
>>> 
>>> 
>> 
>> 
>
>


More information about the Squeak-dev mailing list