[squeak-dev] StandardFileStream rant continued [was Problem handling nextLine]

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Tue Nov 17 22:00:23 UTC 2009


Beside,
look at what happens when we execute:

strm := StandardFileStream oldFileNamed: 'foo'.
[[stream atEnd] whileFalse: [strm upTo: Character cr]] ensure: [strm close].

We
- first read 2000 characters in the buffer,
- keep only a few ones (the first line),
- abandon the rest of the buffer to the garbageCollector,
- position the file back
- and restart reading 2000 chars at next call...
Prodigiously inefficient.
I did not check in VMMaker what real work take place under the carpet,
but the primitives looks enough like corresponding standard OS
functions.

Why not just
- reuse at image level the same buffer,
- avoid spurious garbage collection,
- keep a pointer on available data (already read in buffer),
- avoid moving the file pointer back and forth,
- and avoid reading the same file zone again and again ?
We have to thank our OS and hardware to buffer disk data for us and be
glad Squeak does not handle disk heads with such a policy !

Wanna gain a bit of speed ? Don't wait Cog, there is plenty of room here !

Nicolas

2009/11/17 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
> I get it.
> I wanted to use #upToAnyOf: crlf.
> Unfortunately, this one would not tell me whether a cr or a lf was hit
> (or the end of stream).
> So I inlined the sole implementor in the hierarchy naively thinking it
> would just work...
> MY ERROR!
> not only is it sub optimal (slow), #upToAnyOf: is broken in
> StandardFileStream...
>
> Recipe:
> - create an inst var in abstract superclass,
> - use it in many methods,
> - define a subclass that do not use this inst var,
> - define only a subset of superclass messages in the subclass,
> congratulations you created at least as many bugs as the number of
> methods you failed to override.
>
> When I read all this code, I get the feeling that the whole stream
> hierarchy is broken...
> If you trust library consistency, you're in trouble.
> That is the next big core library deserving clean-up in trunk.
> I guess Pharo will have an easier task integrating Nile complete rewrite ;)
>
> OK, end of rant :)
>
> Nicolas
>
> 2009/11/17 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
>> Hi Casimiro,
>> it would help if you would provide receiver class information.
>> Otherwise, I see I probably messed up the horrific Stream hierarchy !
>> I will revert this change until a better fix - mean more tests .
>>
>> Cheers
>>
>> 2009/11/17 Casimiro de Almeida Barreto <casimiro.barreto at gmail.com>:
>>> After today's update, the following code:
>>>
>>>    [ fHandle atEnd ] whileFalse: [
>>>        stringFromFile := fHandle nextLine.
>>>        draw := Draw newFromString: stringFromFile.
>>>        draw isNil
>>>            ifTrue: [
>>>                Transcript show: 'Failed to load draw.'; cr.
>>>                fHandle close.
>>>                ^nil ].
>>>        model draws addLast: draw ].
>>>
>>> Results in the following error:
>>>
>>> nextLine
>>>    "Answer next line (may be empty), or nil if at end.
>>>    Handle a zoo of line delimiters CR, LF, or CR-LF pair"
>>>
>>>    | newStream element crlf |
>>>    self atEnd ifTrue: [^nil].
>>>    crlf := CharacterSet crlf.
>>>    newStream := WriteStream on: (collection species new: 100).
>>>    [self atEnd ifTrue: [^newStream contents].
>>>    crlf includes: (element := self next)]
>>>        whileFalse: [newStream nextPut: element].
>>>    element = Character cr ifTrue: [self peekFor: Character lf]. "handle
>>> an eventual CR LF pair"
>>>    ^newStream contents
>>>
>>> =============================================================
>>>
>>> new: sizeRequested
>>>    "Answer an initialized instance of this class with the number of
>>> indexable
>>>    variables specified by the argument, sizeRequested."
>>>
>>>    ^ (self basicNew: sizeRequested) initialize
>>>
>>> =============================================================
>>>
>>> basicNew: sizeRequested
>>>    "Primitive. Answer an instance of this class with the number
>>>    of indexable variables specified by the argument, sizeRequested.
>>>    Fail if this class is not indexable or if the argument is not a
>>>    positive Integer, or if there is not enough memory available.
>>>    Essential. See Object documentation whatIsAPrimitive."
>>>
>>>    <primitive: 71>
>>>    self isVariable ifFalse:
>>>        [self error: self printString, ' cannot have variable sized
>>> instances'].
>>>    (sizeRequested isInteger and: [sizeRequested >= 0]) ifTrue:
>>>        ["arg okay; space must be low."
>>>        self environment signalLowSpace.
>>>        ^ self basicNew: sizeRequested  "retry if user proceeds"].
>>>    self primitiveFailed
>>>
>>> =============================================================
>>>
>>> error: aString
>>>    "Throw a generic Error exception."
>>>
>>>    ^Error new signal: aString
>>>
>>> Error: UndefinedObject cannot have variable sized instances
>>>
>>>
>>>
>>>
>>>
>>>
>>
>



More information about the Squeak-dev mailing list