2014/1/19 Levente Uzonyi <leves at elte.hu>

> 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 ]
>
>
>
Or to profit by luck: we pre-allocated 100, the originalContents grew to
200 but precisely fits stream position:

originalContents size = stream position ifTrue: [^originalContents]


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
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20140119/ce4a4576/attachment.htm