Re: [Pharo-project] Bug in FileSystem
On 07.01.2012 14:05, Henrik Sperre Johansen wrote:
On 07.01.2012 13:20, Stéphane Ducasse wrote:
sounds like :)
Stef
On Jan 7, 2012, at 1:10 PM, Mariano Martinez Peck wrote:
FSReadStream>> next: count | result | result := ByteArray new: count. handle at: position read: result startingAt: 1 count: count. position := position + 1. ^ result
shouldn't be
FSReadStream>> next: count | result | result := ByteArray new: count. handle at: position read: result startingAt: 1 count: count. position := position + count. ^ result
At least with that (among some extensions) Fuel tests pass with FS :)
Cheers
-- Mariano http://marianopeck.wordpress.com
Still wrong though, I think... It doesn't handle the case where you try to read past end of stream.
next: count ^self nextInto: (ByteArray new: count)
would probably be better.
Cheers, Henry
As for the API: 1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations. 2) FSHandle >>at: offset read: buffer startingAt: start count: count is a weirdly named selector for what it does imho... Doesn't #at: offset read: count into: buffer startingAt: start make more sense? Cheers, Henry
2012/1/7 Henrik Sperre Johansen <henrik.s.johansen@veloxit.no>:
On 07.01.2012 14:05, Henrik Sperre Johansen wrote:
On 07.01.2012 13:20, Stéphane Ducasse wrote:
sounds like :)
Stef
On Jan 7, 2012, at 1:10 PM, Mariano Martinez Peck wrote:
FSReadStream>>  next: count   | result |   result := ByteArray new: count.   handle at: position read: result startingAt: 1 count: count.   position := position + 1.    ^ result
shouldn't be
FSReadStream>>  next: count   | result |   result := ByteArray new: count.   handle at: position read: result startingAt: 1 count: count.   position := position + count.    ^ result
At least with that (among some extensions) Fuel tests pass with FS :)
Cheers
-- Mariano http://marianopeck.wordpress.com
Still wrong though, I think... It doesn't handle the case where you try to read past end of stream.
next: count ^self nextInto: (ByteArray new: count)
would probably be better.
Cheers, Henry
As for the API:
1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations. 2) FSHandle >>at: offset read: buffer startingAt: start count: count is a weirdly named selector for what it does imho... Doesn't #at: offset read: count into: buffer startingAt: start  make more sense?
Sure. But without reading source code I also wonder why mixing two operations into a single ? I mean seek() and read() Since the underlying OS handle is most probably a stream, it should preserve its own position. Is it because the handle might be shared and FS thus has to provide some kind of atomic operation ? Nicolas
Cheers, Henry
On 07 Jan 2012, at 14:19, Henrik Sperre Johansen wrote:
1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations.
The 2 key methods for efficient IO are: readInto: collection startingAt: offset count: requestedCount "Read requestedCount elements into collection starting at offset, returning the number of elements read, there could be less elements available." next: count putAll: collection startingAt: offset "Write count bytes from collection starting at offset." All other bulk operations can be written in terms of these (see for example Zodiac streams). Sven
2012/1/7 Sven Van Caekenberghe <sven@beta9.be>:
On 07 Jan 2012, at 14:19, Henrik Sperre Johansen wrote:
1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations.
The 2 key methods for efficient IO are:
readInto: collection startingAt: offset count: requestedCount     "Read requestedCount elements into collection starting at offset,     returning the number of elements read, there could be less elements available."
next: count putAll: collection startingAt: offset     "Write count bytes from collection starting at offset."
All other bulk operations can be written in terms of these (see for example Zodiac streams).
Sven
+1. All streams should provide these API, and if possible, provide an optimized implementation. It was not the case in Squeak, i started to refactor a bit, but incrementally modifying this dinosaurus without creating circularities is a sport! I still have 3 years old refactorings unpublished because I'm not sure how to reproduce the changes in a safe order ;) Nicolas
How can we build and improve FS? I'm too busy with other topics but I would love to get some momentum on that topic. Stef
On 07 Jan 2012, at 14:19, Henrik Sperre Johansen wrote:
1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations.
The 2 key methods for efficient IO are:
readInto: collection startingAt: offset count: requestedCount "Read requestedCount elements into collection starting at offset, returning the number of elements read, there could be less elements available."
next: count putAll: collection startingAt: offset "Write count bytes from collection starting at offset."
All other bulk operations can be written in terms of these (see for example Zodiac streams).
Sven
+1. All streams should provide these API, and if possible, provide an optimized implementation. It was not the case in Squeak, i started to refactor a bit, but incrementally modifying this dinosaurus without creating circularities is a sport! I still have 3 years old refactorings unpublished because I'm not sure how to reproduce the changes in a safe order ;)
Nicolas
As for the API:
1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations.
Hi Henry. One of the extensions I needed to add to FSReadStream in order to make it work with Fuel was exatly #next:into: The method is: next: number into: aCollection | count | count := handle at: position read: aCollection startingAt: 1 count: number. position := position + count. ^ count < aCollection size ifTrue: [aCollection first: count] ifFalse: [aCollection] I didn't understand what you meant by "It's really the only way to do buffers without garbage allocations" So...should I integrate this method #next:into: as it is together to the fix to #read:
2) FSHandle >>at: offset read: buffer startingAt: start count: count is a weirdly named selector for what it does imho... Doesn't #at: offset read: count into: buffer startingAt: start make more sense?
Cheers, Henry
-- Mariano http://marianopeck.wordpress.com
So, here it is: http://code.google.com/p/pharo/issues/detail?id=5161 On Sun, Jan 8, 2012 at 2:39 PM, Mariano Martinez Peck <marianopeck@gmail.com
wrote:
As for the API:
1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations.
Hi Henry. One of the extensions I needed to add to FSReadStream in order to make it work with Fuel was exatly #next:into: The method is:
next: number into: aCollection | count | count := handle at: position read: aCollection startingAt: 1 count: number.
position := position + count. ^ count < aCollection size ifTrue: [aCollection first: count] ifFalse: [aCollection]
I didn't understand what you meant by "It's really the only way to do buffers without garbage allocations" So...should I integrate this method #next:into: as it is together to the fix to #read:
2) FSHandle >>at: offset read: buffer startingAt: start count: count is a weirdly named selector for what it does imho... Doesn't #at: offset read: count into: buffer startingAt: start make more sense?
Cheers, Henry
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
participants (5)
-
Henrik Sperre Johansen -
Mariano Martinez Peck -
Nicolas Cellier -
Stéphane Ducasse -
Sven Van Caekenberghe