[squeak-dev] Stream nextPutAll: with an OrderedCollection go boom

Levente Uzonyi leves at caesar.elte.hu
Wed Mar 31 00:23:25 UTC 2021


On Tue, 30 Mar 2021, tim Rowledge wrote:

>
>
>> On 2021-03-30, at 1:05 PM, Levente Uzonyi <leves at caesar.elte.hu> wrote:
>> 
>> I still agree with my previous answers[1]: don't do that. Just use OrderedCollection's own API if you want to build a collection.
>
> Well, yeah, but.
>
> I like things to be decently uniform and at least try to meet the implicit contract. In this case the problem is purely because the test to do a 'fast hack' wrongly passes.

No, that's not the problem. The problem is that OrderedCollection >> #new: 
returns a collection that is always empty.

When WriteStream >> #nextPutAll: sees the empty collection, it tries to 
make enough room for the elements to be added by sending #growTo: to 
itself.
#growTo: sends #new: to the collection's class, which in case of 
OrderedCollection will return another empty collection.
So, collection will remain empty after #growTo:.
#nextPutAll: rightfully assumes that now there is enough space for the 
elements to be added, but there's still none, hence the error.

The real solution is to change WriteStream >> #growTo: to

WriteStream >> growTo: anInteger
 	" anInteger is the required minimal new size of the collection "

 	| oldSize newSize |
 	oldSize := collection size.
 	newSize := anInteger + (oldSize // 4 max: 20).
 	collection := collection grownBy: newSize - oldSize.
 	writeLimit := collection size.

It has 5 senders in the WriteStream hierarchy in my image. All of those 
unnecessarily add 10 to the requested new size, even through #growTo: will 
add at least 20 extra slots.

Collections-ul.932 fixes that.
There's also Collections-ul.933 in the Inbox which slightly improves the 
performance of OrderedCollection streamContents: by avoiding allocating 
an empty initial collection. If you find that useful, push it to the 
Trunk.


But even with that fix, there will be several subclasses of 
SequenceableCollection which won't work with WriteStreams and 
#streamContents:.


Levente

>
> tim
> --
> tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
> Security announcement - as of next week, passwords will be entered in Morse code.


More information about the Squeak-dev mailing list