[PATCH] Improved string writing

[PATCH] Improved string writing

[Ludovic Courtès]

Hi,

While running Guile 1.7.2 under `strace', I noticed the following
unpleasant thing:

  write(1, "\"", 1)                       = 1
  write(1, "h", 1)                        = 1
  write(1, "e", 1)                        = 1
  write(1, "l", 1)                        = 1
  write(1, "l", 1)                        = 1
  write(1, "o", 1)                        = 1
  write(1, "\"", 1)                       = 1

All these system calls for just writing the string "hello".  :-)

The patch below adds buffering in `iprin1' (print.c) so that regular
strings (i.e. strings that don't contain any special characters) can be
written with only one `write' system call, while other strings[*] may be
printed with only a few `write' system calls.

Note that `format' in (ice-9 format) uses `write-char', which
unfortunately leads to the same problem as above.  So this will have to
be fixed eventually too.

Please, let me know if the patch looks ok.

Thanks,
Ludovic.

[*] Test case: (list->string (map integer->char (iota 40))).


\f
diff -ubB --show-c-function /home/ludo/tmp/guile-1.7.2/libguile/print.c\~ /home/ludo/tmp/guile-1.7.2/libguile/print.c
--- /home/ludo/tmp/guile-1.7.2/libguile/print.c~	2005-03-03 23:12:33.000000000 +0100
+++ /home/ludo/tmp/guile-1.7.2/libguile/print.c	2005-04-20 14:09:14.000000000 +0200
@@ -508,29 +508,57 @@ iprin1 (SCM exp, SCM port, scm_print_sta
 	    {
 	      size_t i, len;
 	      const char *data;
+	      char *buffer;
+	      size_t buffer_len, buffer_pos = 0;;
 
-	      scm_putc ('"', port);
 	      len = scm_i_string_length (exp);
 	      data = scm_i_string_chars (exp);
+
+	      /* Allocate a string buffer large enough for the common case of
+		 a printable string.  */
+	      buffer = alloca (len + 20);
+	      buffer_len = len + 20;
+
+#define PUSH_TO_BUFFER(_chr)					\
+	      do						\
+		{						\
+		  if (buffer_pos + 1 > buffer_len)		\
+		    {						\
+		      /* Flush BUFFER to PORT.  */		\
+		      scm_lfwrite (buffer, buffer_len, port);	\
+		      buffer_pos = 0;				\
+		    }						\
+								\
+		  buffer[buffer_pos++] = (_chr);		\
+		}						\
+	      while (0)
+
+	      PUSH_TO_BUFFER ('"');
 	      for (i = 0; i < len; ++i)
 		{
 		  unsigned char ch = data[i];
 		  if ((ch < 32 && ch != '\n') || (127 <= ch && ch < 148))
 		    {
 		      static char const hex[]="0123456789abcdef";
-		      scm_putc ('\\', port);
-		      scm_putc ('x', port);
-		      scm_putc (hex [ch / 16], port);
-		      scm_putc (hex [ch % 16], port);
+		      PUSH_TO_BUFFER ('\\');
+		      PUSH_TO_BUFFER ('x');
+		      PUSH_TO_BUFFER (hex [ch / 16]);
+		      PUSH_TO_BUFFER (hex [ch % 16]);
 		    }
 		  else
 		    {
 		      if (ch == '"' || ch == '\\')
-			scm_putc ('\\', port);
-		      scm_putc (ch, port);
+			PUSH_TO_BUFFER ('\\');
+		      PUSH_TO_BUFFER (ch);
 		    }
 		}
-	      scm_putc ('"', port);
+	      PUSH_TO_BUFFER ('"');
+
+	      if (buffer_pos > 0)
+		/* Flush BUFFER to PORT up to BUFFER_POS.  */
+		scm_lfwrite (buffer, buffer_pos, port);
+
+#undef PUSH_TO_BUFFER
 	      scm_remember_upto_here_1 (exp);
 	    }
 	  else


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> While running Guile 1.7.2 under `strace', I noticed the following
> unpleasant thing:

What happened to the port buffering?

> The patch below adds buffering in `iprin1' (print.c)

That doesn't sound like a good place.

> Note that `format' in (ice-9 format) uses `write-char', which
> unfortunately leads to the same problem as above.

For the fixed parts of the format string it probably does that to keep
track of the output column, something it should let the port do.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> What happened to the port buffering?

Indeed, it occurred to me that this is easily fixed by making the port
line-buffered with `setvbuf'...

Actually, I hadn't considered using it because `stdout' is line-buffered
by default, at least on GNU/Linux (SUSv2 states that it shall be
"fully-buffered" by default [1, 2]).  But it looks like Guile's file
ports are unbuffered by default.  Maybe _this_ should be changed?

> That doesn't sound like a good place.

Well, this is not user-visible buffering, just an internal optimization.
It looks better to call `scm_lfwrite ()' once than `scm_putc ()' for
each character, especially since `scm_putc ()' calls `scm_lfwrite ()'
which in turn calls the write method of this port's type, etc.  So, IMO,
this change may still be valuable.

> For the fixed parts of the format string it probably does that to keep
> track of the output column, something it should let the port do.

`scm_lfwrite ()' does this so Guile code doesn't have to care about it.

Thanks,
Ludovic.

[1] http://www.opengroup.org/onlinepubs/007908799/xsh/stderr.html
[2] http://www.opengroup.org/onlinepubs/007908799/xsh/stdio.html


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> Actually, I hadn't considered using it because `stdout' is line-buffered
> by default, at least on GNU/Linux

Guile doesn't actually use stdio.

> (SUSv2 states that it shall be "fully-buffered" by default [1, 2]).

Fully buffered on a non-tty.  On a tty you're right the tradition is
line buffered output.

> But it looks like Guile's file ports are unbuffered by default.

Ah yes, on a tty.  (scm_init_standard_ports in init.c.)

> Maybe _this_ should be changed?

Alas I think that'd be an incompatible change, programs printing a
prompt or progress info probably depend on immediate output as the
default.

(In C stdio a prompt is handled by stdout automatically flushing when
stdin is read, but there's no such interlock in guile, I think.)

It'd be good to add a bit to the manual somewhere about this.  I just
realized I struck something similar to what you're talking about only
the other day, running a guile subprocess from emacs.  I fiddled about
with strings to ensure decent size chunks got written for performance.
(Emacs uses a pseudo-tty to talk to subprocesses, rather than a pipe.)

> It looks better to call `scm_lfwrite ()' once than `scm_putc ()' for
> each character, especially since `scm_putc ()' calls `scm_lfwrite ()'

Yes, but I would limit that to what can be done easily locally, like
two adjacent putcs turned into one write.  I'd leave general sort of
buffering to the port or an application.

> `scm_lfwrite ()' does this so Guile code doesn't have to care about it.

Yep, format.scm started as portable to systems without builtin output
column tracking, I think.  I thought of changing to use the port as a
simplification, I guess it's also an optimization.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Paul Jarc]

Kevin Ryde <user42@zip.com.au> wrote:
> Alas I think that'd be an incompatible change, programs printing a
> prompt or progress info probably depend on immediate output as the
> default.

But the output can be immediate without splitting it up into multiple
system calls.  (display "foo") can result in write(1, "foo", 3), and
as long as that write is done before (display) returns, it's still
just as immediate.


paul


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Kevin Ryde]

prj@po.cwru.edu (Paul Jarc) writes:
>
> But the output can be immediate without splitting it up into multiple
> system calls.  (display "foo") can result in write(1, "foo", 3), and
> as long as that write is done before (display) returns, it's still
> just as immediate.

Yep, that's the sort of chunking that should definitely happen, if it
doesn't already.  What was the original output that sparked this?


(I notice "write" style string output in iprin1() always goes
char-by-char, it'd be nice if it scanned ahead for a char needing an
escape, to send the non-escape block as a single lfwrite.  Decent size
runs of plain chars are probably quite likely.)


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> (I notice "write" style string output in iprin1() always goes
> char-by-char, it'd be nice if it scanned ahead for a char needing an
> escape, to send the non-escape block as a single lfwrite.  Decent size
> runs of plain chars are probably quite likely.)

When modifying `iprin1 ()', I considered traversing the string to see if
it contained non-printable characters, and then deciding whether to
write it with a single `scm_lfwrite ()' call (if it did not contain
non-printable char) or not.

I preferred my "traverse-and-rewrite" approach (again, this really isn't
buffering) as it sounded less complex and probably more optimal in the
presence of non-printable characters.  But you're welcome to prove me
wrong.  ;-)

As for `display' (which Paul pointed out), it turns out that it is
already special-cased in `iprin1 ()' for strings (see
`if SCM_WRITINGP (pstate) ... else scm_lfwrite (...)'), meaning that
_displaying_ a string always leads to a single `scm_lfwrite ()' call.
IOW, even if the output port is unbuffered, displaying a string
currently leads to a single `write' syscall.

To summarize: the problem I was trying to address is only relevant to
`write' for strings.  It is a significant optimization in the case of an
unbuffered output port which is the default.  It's a less important
optimization if the output port is buffered.

Thanks,
Ludovic.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Kevin Ryde]

I expanded the words about the standard ports a bit, for a start:


 -- Scheme Procedure: current-input-port
 -- C Function: scm_current_input_port ()
     Return the current input port.  This is the default port used by
     many input procedures.

     Initially this is the "standard input" in Unix and C terminology.
     When the standard input is a tty the port is unbuffered, otherwise
     it's fully buffered.

     Unbuffered input is good if an application runs an interactive
     subprocess, since any type-ahead input won't go into Guile's buffer
     and hence be unavailable to the subprocess.

     Note that Guile buffering is completely separate from the tty "line
     discipline".  In the usual cooked mode on a tty, Guile only sees a
     line of input once the user presses <return>.

 -- Scheme Procedure: current-output-port
 -- C Function: scm_current_output_port ()
     Return the current output port.  This is the default port used by
     many output procedures.

     Initially this is the "standard output" in Unix and C terminology.
     When the standard output is a tty this port is unbuffered,
     otherwise it's fully buffered.

     Unbuffered output to a tty is good for ensuring progress output or
     a prompt is seen.  But an application which always prints whole
     lines could change to line buffered, or an application with a lot
     of output could go fully buffered and perhaps make explicit
     `force-output' calls (*note Writing::) at selected points.

 -- Scheme Procedure: current-error-port
 -- C Function: scm_current_error_port ()
     Return the port to which errors and warnings should be sent.

     Initially this is the "standard error" in Unix and C terminology.
     When the standard error is a tty this port is unbuffered, otherwise
     it's fully buffered.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> I expanded the words about the standard ports a bit, for a start:

Maybe there should be pointers to `setvbuf' as well?

Still, I don't think this makes the changes I proposed earlier in this
thread irrelevant.  :-)

Thanks,
Ludovic.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Marius Vollmer]

Kevin Ryde <user42@zip.com.au> writes:

> (I notice "write" style string output in iprin1() always goes
> char-by-char, it'd be nice if it scanned ahead for a char needing an
> escape, to send the non-escape block as a single lfwrite.  Decent size
> runs of plain chars are probably quite likely.)

I like this variant best.  I don't think that writing strings with
lots of escape sequences to an unbuffered port happens often enough to
justify implementing a second buffering mechanism.

When outputting an escaped char, we can additionally assemble the
escape sequence in a fixed size buffer and then output that in one go.

I will make that change myself in the next days, but if anyone beats
me to it...

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Ludovic Courtès]

Hi,

Marius Vollmer <mvo@zagadka.de> writes:

> Kevin Ryde <user42@zip.com.au> writes:
>
>> (I notice "write" style string output in iprin1() always goes
>> char-by-char, it'd be nice if it scanned ahead for a char needing an
>> escape, to send the non-escape block as a single lfwrite.  Decent size
>> runs of plain chars are probably quite likely.)
>
> I like this variant best.  I don't think that writing strings with
> lots of escape sequences to an unbuffered port happens often enough to
> justify implementing a second buffering mechanism.
>
> When outputting an escaped char, we can additionally assemble the
> escape sequence in a fixed size buffer and then output that in one go.

I think you overlooked my patch.  :-)

It also assumes that outputting escaped characters is pretty rare.
Therefore, it only relies on a fixed-size buffer (whose size is that of
the string plus a few bytes).  This buffer gets filled in in _one_
string traversal if there are no (or not a lot of) escaped characters.

So the string doesn't need to be traversed earlier, and escaped chars
need not be sent alone to `scm_lfwrite ()'.

Well, anyway, that's not such a big deal.  ;-)

Thanks,
Ludovic.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Marius Vollmer]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

>> I like this variant best.  I don't think that writing strings with
>> lots of escape sequences to an unbuffered port happens often enough to
>> justify implementing a second buffering mechanism.
>>
>> When outputting an escaped char, we can additionally assemble the
>> escape sequence in a fixed size buffer and then output that in one go.
>
> I think you overlooked my patch.  :-)

No, I saw it. :) 

> It also assumes that outputting escaped characters is pretty rare.
> Therefore, it only relies on a fixed-size buffer (whose size is that of
> the string plus a few bytes).  This buffer gets filled in in _one_
> string traversal if there are no (or not a lot of) escaped characters.

Yes, but you still need to check for overflow etc.  Your
PUSH_TO_BUFFER macro is not really trivial and it used in six places
or so.  In my view, that's a full buffering mechanism...

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Marius Vollmer]

Marius Vollmer <mvo@zagadka.de> writes:

> I will make that change myself in the next days, but if anyone beats
> me to it...

Here is a patch for what I have in mind.  I have applied it already.

Index: libguile/print.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/print.c,v
retrieving revision 1.179
diff -u -r1.179 print.c
--- libguile/print.c	23 May 2005 19:57:21 -0000	1.179
+++ libguile/print.c	6 Jun 2005 19:45:04 -0000
@@ -506,30 +506,38 @@
 	case scm_tc7_string:
 	  if (SCM_WRITINGP (pstate))
 	    {
-	      size_t i, len;
+	      size_t i, j, len;
 	      const char *data;
 
 	      scm_putc ('"', port);
 	      len = scm_i_string_length (exp);
 	      data = scm_i_string_chars (exp);
-	      for (i = 0; i < len; ++i)
+	      for (i = 0, j = 0; i < len; ++i)
 		{
 		  unsigned char ch = data[i];
 		  if ((ch < 32 && ch != '\n') || (127 <= ch && ch < 148))
 		    {
 		      static char const hex[]="0123456789abcdef";
-		      scm_putc ('\\', port);
-		      scm_putc ('x', port);
-		      scm_putc (hex [ch / 16], port);
-		      scm_putc (hex [ch % 16], port);
+		      char buf[4];
+
+		      scm_lfwrite (data+j, i-j, port);
+		      buf[0] = '\\';
+		      buf[1] = 'x';
+		      buf[2] =  hex [ch / 16];
+		      buf[3] = hex [ch % 16];
+		      scm_lfwrite (buf, 4, port);
+		      data = scm_i_string_chars (exp);
+		      j = i+1;
 		    }
-		  else
+		  else if (ch == '"' || ch == '\\')
 		    {
-		      if (ch == '"' || ch == '\\')
-			scm_putc ('\\', port);
-		      scm_putc (ch, port);
+		      scm_lfwrite (data+j, i-j, port);
+		      scm_putc ('\\', port);
+		      data = scm_i_string_chars (exp);
+		      j = i;
 		    }
 		}
+	      scm_lfwrite (data+j, i-j, port);
 	      scm_putc ('"', port);
 	      scm_remember_upto_here_1 (exp);
 	    }

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Ludovic Courtès]

Marius Vollmer <mvo@zagadka.de> writes:

> Here is a patch for what I have in mind.  I have applied it already.

Just a note: it seems that the line

> +		      data = scm_i_string_chars (exp);

appears twice within the loop but is not needed.

Thanks,
Ludovic.


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user

Re: [PATCH] Improved string writing

[Marius Vollmer]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Marius Vollmer <mvo@zagadka.de> writes:
>
>> Here is a patch for what I have in mind.  I have applied it already.
>
> Just a note: it seems that the line
>
>> +		      data = scm_i_string_chars (exp);
>
> appears twice within the loop but is not needed.

This is actually needed with the new string representation.  See
strings.h:

   Internal, low level interface to the character arrays

   - Use scm_i_string_chars to get a pointer to the byte array of a
     string for reading.  Use scm_i_string_length to get the number of
     bytes in that array.  The array is not null-terminated.

   - The array is valid as long as the corresponding SCM object is
     protected but only until the next SCM_TICK.  During such a 'safe
     point', strings might change their representation.

The call to scm_lfwrite might do anything since the port might be a
softport, for example.  Thus, we need to call scm_i_string_chars again
after each such call.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-user mailing list
Guile-user@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-user