Inlining `scm_getc ()' and friends

Inlining `scm_getc ()' and friends

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

Hello,

Small I/O functions like `scm_getc ()' deserve to be inlined.  For
instance, Andy measured it some time ago:

  http://thread.gmane.org/gmane.lisp.guile.devel/6639

I'm planning to apply the attached patch to both `master' and 1.8.

To get an idea of its benefit, I run the attached "benchmark" (inspired
from that in Guile-Reader) that does a lot of `read'.  On my 1.2 GHz
Intel Centrino, the inlining yields a 15% run-time improvement for that
test.

OK to apply?

Thanks,
Ludovic.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: The patch --]
[-- Type: text/x-patch, Size: 5852 bytes --]

From 4eeedb81429859b683277eee3af38532ee02733f Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Mon, 14 Apr 2008 18:09:49 +0200
Subject: [PATCH] Inline `scm_getc', `scm_putc' and `scm_puts'.

---
 libguile/inline.h |   86 +++++++++++++++++++++++++++++++++++++++++++++++++---
 libguile/ports.c  |   60 +------------------------------------
 libguile/ports.h  |    5 +--
 3 files changed, 83 insertions(+), 68 deletions(-)

diff --git a/libguile/inline.h b/libguile/inline.h
index 8a6635e..8fa9a8c 100644
--- a/libguile/inline.h
+++ b/libguile/inline.h
@@ -25,17 +25,17 @@
    "inline.c".
 */
 
-#include "libguile/__scm.h"
-
-#if (SCM_DEBUG_CELL_ACCESSES == 1)
 #include <stdio.h>
-#endif
+#include <string.h>
+
+#include "libguile/__scm.h"
 
 #include "libguile/pairs.h"
 #include "libguile/gc.h"
 #include "libguile/threads.h"
 #include "libguile/unif.h"
-#include "libguile/pairs.h"
+#include "libguile/ports.h"
+#include "libguile/error.h"
 
 
 #ifndef SCM_INLINE_C_INCLUDING_INLINE_H
@@ -85,6 +85,10 @@ SCM_API void scm_array_handle_set (scm_t_array_handle *h, ssize_t pos, SCM val);
 
 SCM_API int scm_is_pair (SCM x);
 
+SCM_API int scm_getc (SCM port);
+SCM_API void scm_putc (char c, SCM port);
+SCM_API void scm_puts (const char *str_data, SCM port);
+
 #endif
 
 
@@ -285,5 +289,77 @@ scm_is_pair (SCM x)
   return SCM_I_CONSP (x);
 }
 
+
+/* Port I/O.  */
+
+#ifndef SCM_INLINE_C_INCLUDING_INLINE_H
+SCM_C_EXTERN_INLINE
+#endif
+int
+scm_getc (SCM port)
+{
+  int c;
+  scm_t_port *pt = SCM_PTAB_ENTRY (port);
+
+  if (pt->rw_active == SCM_PORT_WRITE)
+    /* may be marginally faster than calling scm_flush.  */
+    scm_ptobs[SCM_PTOBNUM (port)].flush (port);
+
+  if (pt->rw_random)
+    pt->rw_active = SCM_PORT_READ;
+
+  if (pt->read_pos >= pt->read_end)
+    {
+      if (scm_fill_input (port) == EOF)
+	return EOF;
+    }
+
+  c = *(pt->read_pos++);
+
+  switch (c)
+    {
+      case '\a':
+        break;
+      case '\b':
+        SCM_DECCOL (port);
+        break;
+      case '\n':
+        SCM_INCLINE (port);
+        break;
+      case '\r':
+        SCM_ZEROCOL (port);
+        break;
+      case '\t':
+        SCM_TABCOL (port);
+        break;
+      default:
+        SCM_INCCOL (port);
+        break;
+    }
+
+  return c;
+}
+
+#ifndef SCM_INLINE_C_INCLUDING_INLINE_H
+SCM_C_EXTERN_INLINE
+#endif
+void
+scm_putc (char c, SCM port)
+{
+  SCM_ASSERT_TYPE (SCM_OPOUTPORTP (port), port, 0, NULL, "output port");
+  scm_lfwrite (&c, 1, port);
+}
+
+#ifndef SCM_INLINE_C_INCLUDING_INLINE_H
+SCM_C_EXTERN_INLINE
+#endif
+void
+scm_puts (const char *s, SCM port)
+{
+  SCM_ASSERT_TYPE (SCM_OPOUTPORTP (port), port, 0, NULL, "output port");
+  scm_lfwrite (s, strlen (s), port);
+}
+
+
 #endif
 #endif
diff --git a/libguile/ports.c b/libguile/ports.c
index c4ccca3..b25a7d0 100644
--- a/libguile/ports.c
+++ b/libguile/ports.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2003, 2004, 2006, 2007 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2003, 2004, 2006, 2007, 2008 Free Software Foundation, Inc.
  * 
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -960,64 +960,6 @@ scm_fill_input (SCM port)
   return scm_ptobs[SCM_PTOBNUM (port)].fill_input (port);
 }
 
-int 
-scm_getc (SCM port)
-{
-  int c;
-  scm_t_port *pt = SCM_PTAB_ENTRY (port);
-
-  if (pt->rw_active == SCM_PORT_WRITE)
-    /* may be marginally faster than calling scm_flush.  */
-    scm_ptobs[SCM_PTOBNUM (port)].flush (port);
-  
-  if (pt->rw_random)
-    pt->rw_active = SCM_PORT_READ;
-
-  if (pt->read_pos >= pt->read_end)
-    {
-      if (scm_fill_input (port) == EOF)
-	return EOF;
-    }
-
-  c = *(pt->read_pos++);
-
-  switch (c)
-    {
-      case '\a':
-        break;
-      case '\b':
-        SCM_DECCOL (port);
-        break;
-      case '\n':
-        SCM_INCLINE (port);
-        break;
-      case '\r':
-        SCM_ZEROCOL (port);
-        break;
-      case '\t':
-        SCM_TABCOL (port);
-        break;
-      default:
-        SCM_INCCOL (port);
-        break;
-    }
- 
-  return c;
-}
-
-void 
-scm_putc (char c, SCM port)
-{
-  SCM_ASSERT_TYPE (SCM_OPOUTPORTP (port), port, 0, NULL, "output port");
-  scm_lfwrite (&c, 1, port);
-}
-
-void 
-scm_puts (const char *s, SCM port)
-{
-  SCM_ASSERT_TYPE (SCM_OPOUTPORTP (port), port, 0, NULL, "output port");
-  scm_lfwrite (s, strlen (s), port);
-}
 
 /* scm_lfwrite
  *
diff --git a/libguile/ports.h b/libguile/ports.h
index b93135e..fb0ef4e 100644
--- a/libguile/ports.h
+++ b/libguile/ports.h
@@ -3,7 +3,7 @@
 #ifndef SCM_PORTS_H
 #define SCM_PORTS_H
 
-/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2003, 2004, 2006 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2003, 2004, 2006, 2008 Free Software Foundation, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -264,15 +264,12 @@ SCM_API SCM scm_eof_object_p (SCM x);
 SCM_API SCM scm_force_output (SCM port);
 SCM_API SCM scm_flush_all_ports (void);
 SCM_API SCM scm_read_char (SCM port);
-SCM_API void scm_putc (char c, SCM port);
-SCM_API void scm_puts (const char *str_data, SCM port);
 SCM_API size_t scm_c_read (SCM port, void *buffer, size_t size);
 SCM_API void scm_c_write (SCM port, const void *buffer, size_t size);
 SCM_API void scm_lfwrite (const char *ptr, size_t size, SCM port);
 SCM_API void scm_flush (SCM port);
 SCM_API void scm_end_input (SCM port);
 SCM_API int scm_fill_input (SCM port);
-SCM_API int scm_getc (SCM port);
 SCM_API void scm_ungetc (int c, SCM port);
 SCM_API void scm_ungets (const char *s, int n, SCM port);
 SCM_API SCM scm_peek_char (SCM port);
-- 
1.5.5


[-- Attachment #3: The benchmark --]
[-- Type: application/octet-stream, Size: 1165 bytes --]

(define %files-to-load
  (map %search-load-path
       '("ice-9/boot-9.scm"  "ice-9/common-list.scm"
	 "ice-9/format.scm"  "ice-9/optargs.scm"
	 "ice-9/session.scm" "ice-9/getopt-long.scm"
         "ice-9/psyntax.pp")))

;; Number of iterations reading files.  Adjust this as a function of your
;; machine's CPU power.
(define %iterations 50)

(define (load-file-with-reader file-name reader)
  (with-input-from-file file-name
    (lambda ()
      (setvbuf (current-input-port) _IOFBF 4096)
      (let loop ((sexp (reader)))
        (if (eof-object? sexp)
            #t
	    (loop (reader)))))))

(define (how-long reader)

  ;; `get-internal-run-time' includes both system and user time.
  (define (time-elapsed) (tms:utime (times)))

  (let loop ((start (time-elapsed))
	     (iterations-left %iterations))
    (if (= 0 iterations-left)
	(- (time-elapsed) start)
	(begin
	  (for-each (lambda (file)
		      (load-file-with-reader file reader))
		    %files-to-load)
	  (loop start (- iterations-left 1))))))

(format #t "time elapsed: ~a~%"
        (how-long read))

;; Timing:
;;  master-with-inline-io: 161
;;  master:                191
;; => 15% improvement

Re: Inlining `scm_getc ()' and friends

[Neil Jerram]

ludo@gnu.org (Ludovic Courtès) writes:

> Hello,
>
> Small I/O functions like `scm_getc ()' deserve to be inlined.  For
> instance, Andy measured it some time ago:
>
>   http://thread.gmane.org/gmane.lisp.guile.devel/6639
>
> I'm planning to apply the attached patch to both `master' and 1.8.
>
> To get an idea of its benefit, I run the attached "benchmark" (inspired
> from that in Guile-Reader) that does a lot of `read'.  On my 1.2 GHz
> Intel Centrino, the inlining yields a 15% run-time improvement for that
> test.
>
> OK to apply?

Yes, all looks good to me.

Would it also make sense to commit the benchmark?  We already have a
"benchmark-suite" directory.

     Neil

Re: Inlining `scm_getc ()' and friends

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> Yes, all looks good to me.

Thanks, applied.

> Would it also make sense to commit the benchmark?  We already have a
> "benchmark-suite" directory.

I had a quick look and it seems this directory would need a little bit
of cleanup (e.g., "#!../libguile/guile" does not work here, should be
substituted as for `check-guile', etc.).  Thus I'm procrastinating that
for now...

Thanks,
Ludo'.

Re: Inlining `scm_getc ()' and friends

[Neil Jerram]

ludo@gnu.org (Ludovic Courtès) writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Would it also make sense to commit the benchmark?  We already have a
>> "benchmark-suite" directory.
>
> I had a quick look and it seems this directory would need a little bit
> of cleanup (e.g., "#!../libguile/guile" does not work here, should be
> substituted as for `check-guile', etc.).  Thus I'm procrastinating that
> for now...

But why not commit the benchmark code anyway?  If there are some
benchmarks there worth running, the more incentive for someone (me?)
to clean up the framework code!

(I think it would be good if we could do more to track Guile's
performance.  I'll post about that separately...)

      Neil

Re: Inlining `scm_getc ()' and friends

[Ludovic Courtès]

Hi Neil!

Neil Jerram <neil@ossau.uklinux.net> writes:

> But why not commit the benchmark code anyway?  If there are some
> benchmarks there worth running, the more incentive for someone (me?)
> to clean up the framework code!

You managed to make me change my mind and I just committed `read.bm'.
;-)

Actually the benchmark suite works pretty well, it's just that one has
to use the top-level `benchmark-guile' instead of
`benchmark-suite/guile-benchmark'.

I think you're right: it's a good idea to put small benchmarks there
rather than post code snippets to the list that eventually get lost.

Thanks,
Ludo'.