[PATCH 1/6] blocking_helpers: remove fstat wrapper

[PATCH 1/6] blocking_helpers: remove fstat wrapper

[Eric Wong]

The fstat syscall should never take long, even on sockets and
slow FSes.
---
 ext/clogger_ext/blocking_helpers.h | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/ext/clogger_ext/blocking_helpers.h b/ext/clogger_ext/blocking_helpers.h
index dd46cea..bb366ff 100644
--- a/ext/clogger_ext/blocking_helpers.h
+++ b/ext/clogger_ext/blocking_helpers.h
@@ -18,22 +18,6 @@ static int my_stat(const char *path, struct stat *buf)
            rb_thread_blocking_region((fn),(data), RUBY_UBF_IO, 0)
 #endif
 
-struct fstat_args { int fd; struct stat *buf; };
-static VALUE ng_fstat(void *ptr)
-{
-	struct fstat_args *a = ptr;
-	return (VALUE)fstat(a->fd, a->buf);
-}
-
-static int my_fstat(int fd, struct stat *buf)
-{
-	struct fstat_args a;
-
-	a.fd = fd;
-	a.buf = buf;
-	return (int)rb_thread_io_blocking_region(ng_fstat, &a, fd);
-}
-
 struct write_args { int fd; const void *buf; size_t count; };
 static VALUE ng_write(void *ptr)
 {
@@ -53,6 +37,5 @@ static ssize_t my_write(int fd, const void *buf, size_t count)
 	return r;
 }
 #  define stat(fd,buf) my_stat((fd),(buf))
-#  define fstat(fd,buf) my_fstat((fd),(buf))
 #  define write(fd,buf,count) my_write((fd),(buf),(count))
 #endif
-- 
1.9.0.rc3.13.gda73b5f

[PATCH 2/6] remove unused RARRAY_PTR macro

[Eric Wong]

We do not need it anymore
---
 ext/clogger_ext/ruby_1_9_compat.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ext/clogger_ext/ruby_1_9_compat.h b/ext/clogger_ext/ruby_1_9_compat.h
index 30a38d6..b5653dc 100644
--- a/ext/clogger_ext/ruby_1_9_compat.h
+++ b/ext/clogger_ext/ruby_1_9_compat.h
@@ -5,9 +5,6 @@
 #ifndef RSTRING_LEN
 #  define RSTRING_LEN(s) (RSTRING(s)->len)
 #endif
-#ifndef RARRAY_PTR
-#  define RARRAY_PTR(s) (RARRAY(s)->ptr)
-#endif
 #ifndef RARRAY_LEN
 #  define RARRAY_LEN(s) (RARRAY(s)->len)
 #endif
-- 
1.9.0.rc3.13.gda73b5f

[PATCH 3/6] use RB_GC_GUARD instead of volatile

[Eric Wong]

RB_GC_GUARD is more explicit in intent.
---
 ext/clogger_ext/clogger.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 8995932..ed01534 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -170,7 +170,7 @@ static VALUE byte_xs_str(VALUE from)
 {
 	static const char esc[] = "0123456789ABCDEF";
 	unsigned char *new_ptr;
-	unsigned char *ptr = (unsigned char *)RSTRING_PTR(from);
+	const unsigned char *ptr = (const unsigned char *)RSTRING_PTR(from);
 	long len = RSTRING_LEN(from);
 	long new_len = len;
 	VALUE rv;
@@ -188,7 +188,7 @@ static VALUE byte_xs_str(VALUE from)
 
 	rv = rb_str_new(NULL, new_len);
 	new_ptr = (unsigned char *)RSTRING_PTR(rv);
-	ptr = (unsigned char *)RSTRING_PTR(from);
+	ptr = (const unsigned char *)RSTRING_PTR(from);
 	for (; --len >= 0; ptr++) {
 		unsigned c = *ptr;
 
@@ -867,7 +867,7 @@ static VALUE ccall(struct clogger *c, VALUE env)
 			rb_ary_store(rv, 1, c->headers);
 		}
 	} else {
-		volatile VALUE tmp = rb_inspect(rv);
+		VALUE tmp = rb_inspect(rv);
 
 		c->status = INT2FIX(500);
 		c->headers = c->body = rb_ary_new();
@@ -875,6 +875,7 @@ static VALUE ccall(struct clogger *c, VALUE env)
 		rb_raise(rb_eTypeError,
 		         "app response not a 3 element Array: %s",
 			 RSTRING_PTR(tmp));
+		RB_GC_GUARD(tmp);
 	}
 
 	return rv;
-- 
1.9.0.rc3.13.gda73b5f

[PATCH 4/6] prevent potential premature GC in byte_xs

[Eric Wong]

If we convert an object to string, there is a potential the
compiler may optimize away the converted string if escaping
is needed.  Prevent that with RB_GC_GUARD.
---
 ext/clogger_ext/clogger.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index ed01534..8b05c34 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -166,10 +166,11 @@ static inline int need_escape(unsigned c)
 }
 
 /* we are encoding-agnostic, clients can send us all sorts of junk */
-static VALUE byte_xs_str(VALUE from)
+static VALUE byte_xs(VALUE obj)
 {
 	static const char esc[] = "0123456789ABCDEF";
 	unsigned char *new_ptr;
+	VALUE from = rb_obj_as_string(obj);
 	const unsigned char *ptr = (const unsigned char *)RSTRING_PTR(from);
 	long len = RSTRING_LEN(from);
 	long new_len = len;
@@ -203,14 +204,10 @@ static VALUE byte_xs_str(VALUE from)
 	}
 	assert(RSTRING_PTR(rv)[RSTRING_LEN(rv)] == '\0');
 
+	RB_GC_GUARD(from);
 	return rv;
 }
 
-static VALUE byte_xs(VALUE from)
-{
-	return byte_xs_str(rb_obj_as_string(from));
-}
-
 static void clogger_mark(void *ptr)
 {
 	struct clogger *c = ptr;
-- 
1.9.0.rc3.13.gda73b5f

[PATCH 5/6] remove each_id, it was never used

[Eric Wong]

---
 GNUmakefile               | 2 +-
 ext/clogger_ext/clogger.c | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/GNUmakefile b/GNUmakefile
index 574fffd..49a7c17 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -1,5 +1,5 @@
 all::
-RSYNC_DEST := rubyforge.org:/var/www/gforge-projects/clogger/
+RSYNC_DEST := clogger.bogomips.org:/srv/clogger/
 rfproject := clogger
 rfpackage := clogger
 include pkg.mk
diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 8b05c34..122e7fb 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -122,7 +122,6 @@ struct clogger {
 static ID write_id;
 static ID ltlt_id;
 static ID call_id;
-static ID each_id;
 static ID close_id;
 static ID to_i_id;
 static ID to_s_id;
@@ -1045,7 +1044,6 @@ void Init_clogger_ext(void)
 	write_id = rb_intern("write");
 	ltlt_id = rb_intern("<<");
 	call_id = rb_intern("call");
-	each_id = rb_intern("each");
 	close_id = rb_intern("close");
 	to_i_id = rb_intern("to_i");
 	to_s_id = rb_intern("to_s");
-- 
1.9.0.rc3.13.gda73b5f

[PATCH 6/6] use rb_thread_call_without_gvl for Ruby 2+

[Eric Wong]

rb_thread_blocking_region is deprecated and will be removed
---
 ext/clogger_ext/blocking_helpers.h | 34 ++++++++++++++++++++++++++--------
 ext/clogger_ext/extconf.rb         |  1 +
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/ext/clogger_ext/blocking_helpers.h b/ext/clogger_ext/blocking_helpers.h
index bb366ff..c03c2f5 100644
--- a/ext/clogger_ext/blocking_helpers.h
+++ b/ext/clogger_ext/blocking_helpers.h
@@ -1,27 +1,45 @@
-#ifdef HAVE_RB_THREAD_BLOCKING_REGION
-struct stat_args { const char *path; struct stat *buf; };
-static VALUE ng_stat(void *ptr)
+#if defined(HAVE_RB_THREAD_CALL_WITHOUT_GVL) && defined(HAVE_RUBY_THREAD_H)
+/* Ruby 2.0+ */
+#  include <ruby/thread.h>
+#  define WITHOUT_GVL(fn,a,ubf,b) \
+        rb_thread_call_without_gvl((fn),(a),(ubf),(b))
+#elif defined(HAVE_RB_THREAD_BLOCKING_REGION)
+typedef VALUE (*my_blocking_fn_t)(void*);
+#  define WITHOUT_GVL(fn,a,ubf,b) \
+	rb_thread_blocking_region((my_blocking_fn_t)(fn),(a),(ubf),(b))
+#endif
+
+#ifdef WITHOUT_GVL
+struct stat_args { int err; const char *path; struct stat *buf; };
+static void * ng_stat(void *ptr)
 {
 	struct stat_args *a = ptr;
-	return (VALUE)stat(a->path, a->buf);
+	a->err = stat(a->path, a->buf);
+	return NULL;
 }
+
 static int my_stat(const char *path, struct stat *buf)
 {
 	struct stat_args a;
 
 	a.path = path;
 	a.buf = buf;
-	return (int)rb_thread_blocking_region(ng_stat, &a, RUBY_UBF_IO, 0);
+	WITHOUT_GVL(ng_stat, &a, RUBY_UBF_IO, 0);
+	return a.err;
 }
+
 #ifndef HAVE_RB_THREAD_IO_BLOCKING_REGION
 #  define rb_thread_io_blocking_region(fn,data,fd) \
-           rb_thread_blocking_region((fn),(data), RUBY_UBF_IO, 0)
+           WITHOUT_GVL((fn),(data), RUBY_UBF_IO, 0)
+#else
+  VALUE rb_thread_io_blocking_region(VALUE(*)(void *), void *, int);
 #endif
 
 struct write_args { int fd; const void *buf; size_t count; };
 static VALUE ng_write(void *ptr)
 {
 	struct write_args *a = ptr;
+
 	return (VALUE)write(a->fd, a->buf, a->count);
 }
 static ssize_t my_write(int fd, const void *buf, size_t count)
@@ -36,6 +54,6 @@ static ssize_t my_write(int fd, const void *buf, size_t count)
 
 	return r;
 }
-#  define stat(fd,buf) my_stat((fd),(buf))
+#  define stat(path,buf) my_stat((path),(buf))
 #  define write(fd,buf,count) my_write((fd),(buf),(count))
-#endif
+#endif /* !WITHOUT_GVL */
diff --git a/ext/clogger_ext/extconf.rb b/ext/clogger_ext/extconf.rb
index 99f7deb..523b314 100644
--- a/ext/clogger_ext/extconf.rb
+++ b/ext/clogger_ext/extconf.rb
@@ -19,6 +19,7 @@ begin
   have_func('gmtime_r', 'time.h') or raise "gmtime_r needed"
   have_struct_member('struct tm', 'tm_gmtoff', 'time.h')
   have_func('rb_str_set_len', 'ruby.h')
+  have_func('rb_thread_call_without_gvl', 'ruby/thread.h')
   have_func('rb_thread_blocking_region', 'ruby.h')
   have_func('rb_thread_io_blocking_region', 'ruby.h')
   create_makefile('clogger_ext')
-- 
1.9.0.rc3.13.gda73b5f