[PATCH 0/2] Some FileStream optimizations

A small return. ;)

Patch 2 was suggested by Holger, and makes FileStream>>#contents as
efficient as FileDescriptor>>#contents.  The first one instead I found
while looking for more similar opportunities regarding #fill.

Please review!

Paolo Bonzini (2):
  optimize FileStream>>#upTo: and FileStream>>#nextLine
  optimize FileStream>>#nextAvailable:... to skip buffer.

 kernel/FileStream.st | 55 ++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 44 insertions(+), 17 deletions(-)

--
1.8.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

2013-05-04  Paolo Bonzini  <[hidden email]>

        * kernel/FileStream.st: Avoid a useless copy from the
        buffer when #upTo: and #nextLine are using a stream.
---
 kernel/FileStream.st | 45 ++++++++++++++++++++++++++++++---------------
 1 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/kernel/FileStream.st b/kernel/FileStream.st
index 1aa5635..0c66169 100644
--- a/kernel/FileStream.st
+++ b/kernel/FileStream.st
@@ -394,7 +394,7 @@ file object, such as /dev/rmt0 on UNIX or MTA0: on VMS).'>
  the stream's contents if no such character is found."
 
  <category: 'overriding inherited methods'>
- | n resultStream result ch |
+ | start resultStream result ch |
  writePtr notNil ifTrue: [self flush].
  ptr > endPtr
     ifTrue:
@@ -408,16 +408,22 @@ file object, such as /dev/rmt0 on UNIX or MTA0: on VMS).'>
  [:i |
  (ch := collection at: i) == aCharacter
     ifTrue:
- [result := collection copyFrom: ptr to: i - 1.
+ [start := ptr.
  ptr := i + 1.
 
- "If we went through the loop only once, we're done."
- resultStream isNil ifTrue: [^result].
+ "If we went through the loop only once, copy from
+ the buffer."
+ resultStream isNil ifTrue: [
+    result := collection copyFrom: start to: i - 1.
+    ^result].
 
  "Else finish the stream and return its contents."
- ^resultStream
-    nextPutAll: result;
-    contents]].
+ resultStream
+    next: i - start
+    putAll: collection
+    startingAt: start.
+
+ ^resultStream contents]].
  resultStream isNil
     ifTrue:
  [resultStream := WriteStream on: (self species new: endPtr - ptr + 20)].
@@ -438,7 +444,7 @@ file object, such as /dev/rmt0 on UNIX or MTA0: on VMS).'>
  stream's contents if no new-line character is found."
 
  <category: 'overriding inherited methods'>
- | n resultStream result ch |
+ | start resultStream result ch |
  writePtr notNil ifTrue: [self flush].
  ptr > endPtr
     ifTrue:
@@ -452,17 +458,26 @@ file object, such as /dev/rmt0 on UNIX or MTA0: on VMS).'>
  [:i |
  ((ch := collection at: i) == ##(Character cr) or: [ch == ##(Character nl)])
     ifTrue:
- [result := collection copyFrom: ptr to: i - 1.
+ [start := ptr.
  ptr := i + 1.
- ch == ##(Character cr) ifTrue: [self peekFor: ##(Character nl)].
 
- "If we went through the loop only once, we're done."
- resultStream isNil ifTrue: [^result].
+ "If we went through the loop only once, copy from
+ the buffer.  We cannot check for CR/LF yet, #peekFor:
+ could refill collection."
+ resultStream isNil ifTrue: [
+    result := collection copyFrom: start to: i - 1.
+    ch == ##(Character cr) ifTrue: [self peekFor: ##(Character nl)].
+    ^result].
 
  "Else finish the stream and return its contents."
- ^resultStream
-    nextPutAll: result;
-    contents]].
+ resultStream
+    next: i - start
+    putAll: collection
+    startingAt: start.
+
+ ch == ##(Character cr) ifTrue: [self peekFor: ##(Character nl)].
+ ^resultStream contents]].
+
  resultStream isNil
     ifTrue:
  [resultStream := WriteStream on: (self species new: endPtr - ptr + 20)].
--
1.8.2



_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

2013-05-04  Paolo Bonzini  <[hidden email]>

        * kernel/FileStream.st: Skip the buffer in #nextAvailable:... if
        it is empty.
---
 kernel/FileStream.st | 10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/FileStream.st b/kernel/FileStream.st
index 0c66169..255f76d 100644
--- a/kernel/FileStream.st
+++ b/kernel/FileStream.st
@@ -499,7 +499,10 @@ file object, such as /dev/rmt0 on UNIX or MTA0: on VMS).'>
  <category: 'buffering'>
  | last n |
  writePtr notNil ifTrue: [self flush].
- ptr > endPtr ifTrue: [self fill].
+ ptr > endPtr ifTrue: [
+            anInteger > collection size
+                ifTrue: [^super nextAvailable: anInteger putAllOn: aStream].
+            self fill].
 
  "Fetch data from the buffer, without doing more than one I/O operation."
  last := endPtr min: ptr + anInteger - 1.
@@ -516,7 +519,10 @@ file object, such as /dev/rmt0 on UNIX or MTA0: on VMS).'>
  <category: 'buffering'>
  | last n |
  writePtr notNil ifTrue: [self flush].
- ptr > endPtr ifTrue: [self fill].
+ ptr > endPtr ifTrue: [
+            anInteger > collection size
+                ifTrue: [^super nextAvailable: anInteger into: aCollection startingAt: pos].
+            self fill].
 
  "Fetch data from the buffer, without doing more than one I/O operation."
  last := endPtr min: ptr + anInteger - 1.
--
1.8.2


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Sat, May 04, 2013 at 06:26:44PM +0200, Paolo Bonzini wrote:

> - | n resultStream result ch |
> + | start resultStream result ch |

n was unused so far?



> + [start := ptr.
>   ptr := i + 1.
> - ch == ##(Character cr) ifTrue: [self peekFor: ##(Character nl)].

ah, I didn't know the ##(Character cr) trick. Nice that this is optimized.


The patch is looking fine.

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Sat, May 04, 2013 at 06:26:43PM +0200, Paolo Bonzini wrote:
> A small return. ;)
>
> Patch 2 was suggested by Holger, and makes FileStream>>#contents as
> efficient as FileDescriptor>>#contents.  The first one instead I found
> while looking for more similar opportunities regarding #fill.
>
> Please review!

Hi,

I can't find my testcase/example for this issue. I remember small reads
and plenty of fcntl. Do you still have access to the past?  The code
appears to work, the change looks sensible but I can't say if it is a
speed-up. :)

holger

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

Il 28/05/2013 20:31, Holger Hans Peter Freyther ha scritto:
> Hi,
>
> I can't find my testcase/example for this issue. I remember small reads
> and plenty of fcntl. Do you still have access to the past?  The code
> appears to work, the change looks sensible but I can't say if it is a
> speed-up. :)

The testcase is simply "'foo' asFile readStream contents".

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Wed, May 29, 2013 at 08:46:27AM +0200, Paolo Bonzini wrote:
>
> The testcase is simply "'foo' asFile readStream contents".

Yes, I did that but compared to 3.2.5/master vs. patch I don't
see much change in regard to the amount of fcntl. Which means
there was something else (which I forgot now) that we considered
inefficient.

holger

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

Il 29/05/2013 10:09, Holger Hans Peter Freyther ha scritto:
>> >
>> > The testcase is simply "'foo' asFile readStream contents".
> Yes, I did that but compared to 3.2.5/master vs. patch I don't
> see much change in regard to the amount of fcntl. Which means
> there was something else (which I forgot now) that we considered
> inefficient.

It's the amount of lseek and read, not fcntl.

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Wed, May 29, 2013 at 04:22:03PM +0200, Paolo Bonzini wrote:
> Il 29/05/2013 10:09, Holger Hans Peter Freyther ha scritto:
> >> >
> >> > The testcase is simply "'foo' asFile readStream contents".
> > Yes, I did that but compared to 3.2.5/master vs. patch I don't
> > see much change in regard to the amount of fcntl. Which means
> > there was something else (which I forgot now) that we considered
> > inefficient.
>
> It's the amount of lseek and read, not fcntl.

I remember that we saw excess.. and I didn't see a difference
as I was trying to read from /proc/cpuinfo and there is no
difference. Reading from a real file gives me a difference.


read(4, "!\n! OsmoBSC (0.9.14.182-edce) co"..., 1024) = 1024
read(4, "ty 30000\n  location_area_code 82"..., 1024) = 1024
read(4, " phys_chan_config TCH/F\n     hop"..., 1024) = 940

vs.

read(3, "!\n! OsmoBSC (0.9.14.182-edce) co"..., 2988) = 2988


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk