Rev talk - Nov 2009 - [PATCH iobuffer] Allow global tuning of the default node size

Hi, I''ve also pushed out to git://yhbt.net/iobuffer

Background:

  Using the stupid dd(1)-like benchmark in Unicorn[1] with the Rainbows!
  Rev concurrency model, I noticed Rev being significantly[2] slower
  than the EventMachine one with certain response sizes, while other
  (both smaller and larger, the two were roughly on par[3].  It turned
  out the default benchmark defaults of writing 4096-byte response
  bodies managed to cause many extra allocations because of HTTP headers
  being written separately.

  Since Rev::IO::INPUT_SIZE is 16384, I figured it would make sense to
  use 16384 in Rainbows!, too.  This should also have the pleasant side
  effect of allowing the malloc() implementation to share unused blocks
  between Rev input and output more easily.  I noticed this performance
  improvement both using tcmalloc and glibc ptmalloc2.

  I would not hesitate changing the DEFAULT_NODE_SIZE to 16384 so
  applications are less likely to want to change it, too.

  [1] git://git.bogomips.org/unicorn.git test/benchmark/dd.ru
  [2] roughly half as fast as EM
  [3] EM is still a little faster, further investigation needed...
>From 715cb9915de00e857f3e727e8c49117bbf06ffb7 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson at yhbt.net>
Date: Fri, 27 Nov 2009 11:08:09 -0800
Subject: [PATCH] Allow global tuning of the default node size

This is useful for applications that use Rev and do not
allocate IO::Buffer objects directly.
---
 ext/iobuffer.c      |   56 +++++++++++++++++++++++++++++++++++++++++++-------
 spec/buffer_spec.rb |   25 ++++++++++++++++++++++
 2 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/ext/iobuffer.c b/ext/iobuffer.c
index 30f798d..aa8a6ab 100644
--- a/ext/iobuffer.c
+++ b/ext/iobuffer.c
@@ -23,6 +23,7 @@
 
 /* Default number of bytes in each node''s buffer.  Should be >= MTU
*/
 #define DEFAULT_NODE_SIZE 4096
+static unsigned default_node_size = DEFAULT_NODE_SIZE;
 
 struct buffer {
   unsigned size, node_size;
@@ -43,6 +44,8 @@ static VALUE IO_Buffer_allocate(VALUE klass);
 static void IO_Buffer_mark(struct buffer *);
 static void IO_Buffer_free(struct buffer *);
 
+static VALUE IO_Buffer_default_node_size(VALUE klass);
+static VALUE IO_Buffer_set_default_node_size(VALUE klass, VALUE size);
 static VALUE IO_Buffer_initialize(int argc, VALUE *argv, VALUE self);
 static VALUE IO_Buffer_clear(VALUE self);
 static VALUE IO_Buffer_size(VALUE self);
@@ -77,6 +80,11 @@ void Init_iobuffer()
   cIO_Buffer = rb_define_class_under(rb_cIO, "Buffer", rb_cObject);
   rb_define_alloc_func(cIO_Buffer, IO_Buffer_allocate);
 
+  rb_define_singleton_method(cIO_Buffer, "default_node_size",
+                             IO_Buffer_default_node_size, 0);
+  rb_define_singleton_method(cIO_Buffer, "default_node_size=",
+                             IO_Buffer_set_default_node_size, 1);
+
   rb_define_method(cIO_Buffer, "initialize", IO_Buffer_initialize,
-1);
   rb_define_method(cIO_Buffer, "clear", IO_Buffer_clear, 0);
   rb_define_method(cIO_Buffer, "size", IO_Buffer_size, 0);
@@ -108,29 +116,61 @@ static void IO_Buffer_free(struct buffer *buf)
 }
 
 /**
+ * call-seq:
+ *   IO_Buffer.default_node_size -> 4096
+ *
+ * Retrieves the current value of the default node size.
+ */
+static VALUE IO_Buffer_default_node_size(VALUE klass)
+{
+  return UINT2NUM(default_node_size);
+}
+
+/*
+ * safely converts node sizes from Ruby numerics to C and raising
+ * ArgumentError or RangeError on invalid sizes
+ */
+static unsigned convert_node_size(VALUE size)
+{
+  int node_size = NUM2INT(size);
+
+  if(node_size < 1) rb_raise(rb_eArgError, "invalid buffer size");
+
+  return (unsigned)node_size;
+}
+
+/**
+ * call-seq:
+ *   IO_Buffer.default_node_size = 16384
+ *
+ * Sets the default node size for calling IO::Buffer.new with no arguments.
+ */
+static VALUE IO_Buffer_set_default_node_size(VALUE klass, VALUE size)
+{
+  default_node_size = convert_node_size(size);
+
+  return size;
+}
+
+/**
  *  call-seq:
- *    IO_Buffer.new(size = DEFAULT_NODE_SIZE) -> IO_Buffer
+ *    IO_Buffer.new(size = IO::Buffer.default_node_size) -> IO_Buffer
  * 
  * Create a new IO_Buffer with linked segments of the given size
  */
 static VALUE IO_Buffer_initialize(int argc, VALUE *argv, VALUE self)
 {
   VALUE node_size_obj;
-  int node_size;
   struct buffer *buf;
 
   if(rb_scan_args(argc, argv, "01", &node_size_obj) == 1) {
-    node_size = NUM2INT(node_size_obj);
-
-    if(node_size < 1) rb_raise(rb_eArgError, "invalid buffer
size");
-
     Data_Get_Struct(self, struct buffer, buf);
 
     /* Make sure we''re not changing the buffer size after data has
been allocated */
     assert(!buf->head);
     assert(!buf->pool_head);
 
-    buf->node_size = node_size;
+    buf->node_size = convert_node_size(node_size_obj);
   }
 
   return Qnil;
@@ -329,7 +369,7 @@ static struct buffer *buffer_new(void)
   buf = (struct buffer *)xmalloc(sizeof(struct buffer));
   buf->head = buf->tail = buf->pool_head = buf->pool_tail = 0;
   buf->size = 0;
-  buf->node_size = DEFAULT_NODE_SIZE;
+  buf->node_size = default_node_size;
 	
   return buf;
 }
diff --git a/spec/buffer_spec.rb b/spec/buffer_spec.rb
index 4329bf6..fa38740 100644
--- a/spec/buffer_spec.rb
+++ b/spec/buffer_spec.rb
@@ -98,6 +98,31 @@ describe IO::Buffer do
     @buffer.should_not be_empty
   end
 
+  it "can set default node size" do
+    IO::Buffer.default_node_size = 1
+    IO::Buffer.default_node_size.should == 1
+    (IO::Buffer.default_node_size = 4096).should == 4096
+  end
+
+  it "can be created with a different node size" do
+    IO::Buffer.new(16384)
+  end
+
+  it "cannot set invalid node sizes" do
+    proc {
+      IO::Buffer.default_node_size = 0xffffffffffffffff
+    }.should raise_error(RangeError)
+    proc {
+      IO::Buffer.default_node_size = 0
+    }.should raise_error(ArgumentError)
+    proc {
+      IO::Buffer.new(0xffffffffffffffff)
+    }.should raise_error(RangeError)
+    proc {
+      IO::Buffer.new(0)
+    }.should raise_error(ArgumentError)
+  end
+
   #######
   private
   #######
-- 
Eric Wong

Merged and pushed.  I bumped DEFAULT_NODE_SIZE to 16384 myself.

I''ll try doing a release here soon unless you anticipate making any
more
changes.  My first release on Gemcutter, I guess.  Should be interesting.

On Fri, Nov 27, 2009 at 12:28 PM, Eric Wong <normalperson at yhbt.net>
wrote:
> Hi, I''ve also pushed out to git://yhbt.net/iobuffer
>
> Background:
>
>  Using the stupid dd(1)-like benchmark in Unicorn[1] with the Rainbows!
>  Rev concurrency model, I noticed Rev being significantly[2] slower
>  than the EventMachine one with certain response sizes, while other
>  (both smaller and larger, the two were roughly on par[3].  It turned
>  out the default benchmark defaults of writing 4096-byte response
>  bodies managed to cause many extra allocations because of HTTP headers
>  being written separately.
>
>  Since Rev::IO::INPUT_SIZE is 16384, I figured it would make sense to
>  use 16384 in Rainbows!, too.  This should also have the pleasant side
>  effect of allowing the malloc() implementation to share unused blocks
>  between Rev input and output more easily.  I noticed this performance
>  improvement both using tcmalloc and glibc ptmalloc2.
>
>  I would not hesitate changing the DEFAULT_NODE_SIZE to 16384 so
>  applications are less likely to want to change it, too.
>
>  [1] git://git.bogomips.org/unicorn.git test/benchmark/dd.ru
>  [2] roughly half as fast as EM
>  [3] EM is still a little faster, further investigation needed...
>
> >From 715cb9915de00e857f3e727e8c49117bbf06ffb7 Mon Sep 17 00:00:00 2001
> From: Eric Wong <normalperson at yhbt.net>
> Date: Fri, 27 Nov 2009 11:08:09 -0800
> Subject: [PATCH] Allow global tuning of the default node size
>
> This is useful for applications that use Rev and do not
> allocate IO::Buffer objects directly.
> ---
>  ext/iobuffer.c      |   56
> +++++++++++++++++++++++++++++++++++++++++++-------
>  spec/buffer_spec.rb |   25 ++++++++++++++++++++++
>  2 files changed, 73 insertions(+), 8 deletions(-)
>
> diff --git a/ext/iobuffer.c b/ext/iobuffer.c
> index 30f798d..aa8a6ab 100644
> --- a/ext/iobuffer.c
> +++ b/ext/iobuffer.c
> @@ -23,6 +23,7 @@
>
>  /* Default number of bytes in each node''s buffer.  Should be
>= MTU */
>  #define DEFAULT_NODE_SIZE 4096
> +static unsigned default_node_size = DEFAULT_NODE_SIZE;
>
>  struct buffer {
>   unsigned size, node_size;
> @@ -43,6 +44,8 @@ static VALUE IO_Buffer_allocate(VALUE klass);
>  static void IO_Buffer_mark(struct buffer *);
>  static void IO_Buffer_free(struct buffer *);
>
> +static VALUE IO_Buffer_default_node_size(VALUE klass);
> +static VALUE IO_Buffer_set_default_node_size(VALUE klass, VALUE size);
>  static VALUE IO_Buffer_initialize(int argc, VALUE *argv, VALUE self);
>  static VALUE IO_Buffer_clear(VALUE self);
>  static VALUE IO_Buffer_size(VALUE self);
> @@ -77,6 +80,11 @@ void Init_iobuffer()
>   cIO_Buffer = rb_define_class_under(rb_cIO, "Buffer",
rb_cObject);
>   rb_define_alloc_func(cIO_Buffer, IO_Buffer_allocate);
>
> +  rb_define_singleton_method(cIO_Buffer, "default_node_size",
> +                             IO_Buffer_default_node_size, 0);
> +  rb_define_singleton_method(cIO_Buffer, "default_node_size=",
> +                             IO_Buffer_set_default_node_size, 1);
> +
>   rb_define_method(cIO_Buffer, "initialize",
IO_Buffer_initialize, -1);
>   rb_define_method(cIO_Buffer, "clear", IO_Buffer_clear, 0);
>   rb_define_method(cIO_Buffer, "size", IO_Buffer_size, 0);
> @@ -108,29 +116,61 @@ static void IO_Buffer_free(struct buffer *buf)
>  }
>
>  /**
> + * call-seq:
> + *   IO_Buffer.default_node_size -> 4096
> + *
> + * Retrieves the current value of the default node size.
> + */
> +static VALUE IO_Buffer_default_node_size(VALUE klass)
> +{
> +  return UINT2NUM(default_node_size);
> +}
> +
> +/*
> + * safely converts node sizes from Ruby numerics to C and raising
> + * ArgumentError or RangeError on invalid sizes
> + */
> +static unsigned convert_node_size(VALUE size)
> +{
> +  int node_size = NUM2INT(size);
> +
> +  if(node_size < 1) rb_raise(rb_eArgError, "invalid buffer
size");
> +
> +  return (unsigned)node_size;
> +}
> +
> +/**
> + * call-seq:
> + *   IO_Buffer.default_node_size = 16384
> + *
> + * Sets the default node size for calling IO::Buffer.new with no
> arguments.
> + */
> +static VALUE IO_Buffer_set_default_node_size(VALUE klass, VALUE size)
> +{
> +  default_node_size = convert_node_size(size);
> +
> +  return size;
> +}
> +
> +/**
>  *  call-seq:
> - *    IO_Buffer.new(size = DEFAULT_NODE_SIZE) -> IO_Buffer
> + *    IO_Buffer.new(size = IO::Buffer.default_node_size) -> IO_Buffer
>  *
>  * Create a new IO_Buffer with linked segments of the given size
>  */
>  static VALUE IO_Buffer_initialize(int argc, VALUE *argv, VALUE self)
>  {
>   VALUE node_size_obj;
> -  int node_size;
>   struct buffer *buf;
>
>   if(rb_scan_args(argc, argv, "01", &node_size_obj) == 1) {
> -    node_size = NUM2INT(node_size_obj);
> -
> -    if(node_size < 1) rb_raise(rb_eArgError, "invalid buffer
size");
> -
>     Data_Get_Struct(self, struct buffer, buf);
>
>     /* Make sure we''re not changing the buffer size after data has
been
> allocated */
>     assert(!buf->head);
>     assert(!buf->pool_head);
>
> -    buf->node_size = node_size;
> +    buf->node_size = convert_node_size(node_size_obj);
>   }
>
>   return Qnil;
> @@ -329,7 +369,7 @@ static struct buffer *buffer_new(void)
>   buf = (struct buffer *)xmalloc(sizeof(struct buffer));
>   buf->head = buf->tail = buf->pool_head = buf->pool_tail = 0;
>   buf->size = 0;
> -  buf->node_size = DEFAULT_NODE_SIZE;
> +  buf->node_size = default_node_size;
>
>   return buf;
>  }
> diff --git a/spec/buffer_spec.rb b/spec/buffer_spec.rb
> index 4329bf6..fa38740 100644
> --- a/spec/buffer_spec.rb
> +++ b/spec/buffer_spec.rb
> @@ -98,6 +98,31 @@ describe IO::Buffer do
>     @buffer.should_not be_empty
>   end
>
> +  it "can set default node size" do
> +    IO::Buffer.default_node_size = 1
> +    IO::Buffer.default_node_size.should == 1
> +    (IO::Buffer.default_node_size = 4096).should == 4096
> +  end
> +
> +  it "can be created with a different node size" do
> +    IO::Buffer.new(16384)
> +  end
> +
> +  it "cannot set invalid node sizes" do
> +    proc {
> +      IO::Buffer.default_node_size = 0xffffffffffffffff
> +    }.should raise_error(RangeError)
> +    proc {
> +      IO::Buffer.default_node_size = 0
> +    }.should raise_error(ArgumentError)
> +    proc {
> +      IO::Buffer.new(0xffffffffffffffff)
> +    }.should raise_error(RangeError)
> +    proc {
> +      IO::Buffer.new(0)
> +    }.should raise_error(ArgumentError)
> +  end
> +
>   #######
>   private
>   #######
> --
> Eric Wong
> _______________________________________________
> Rev-talk mailing list
> Rev-talk at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rev-talk
>


-- 
Tony Arcieri
Medioh/Nagravision
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091128/6fb7bfae/attachment.html>

Tony Arcieri <tony at medioh.com> wrote:
> Merged and pushed.  I bumped DEFAULT_NODE_SIZE to 16384 myself.
> 
> I''ll try doing a release here soon unless you anticipate making
any more
> changes.  My first release on Gemcutter, I guess.  Should be interesting.
Awesome, thanks.  I don''t think I have time or need to look into more
performance tuning here at the moment.

-- 
Eric Wong

Okay, while as it stands I was investigating how to push stuff to Gemcutter
and already went ahead and pushed it. :)

It''s now released as iobuffer 0.1.2

On Sat, Nov 28, 2009 at 11:14 AM, Eric Wong <normalperson at yhbt.net>
wrote:
> Tony Arcieri <tony at medioh.com> wrote:
> > Merged and pushed.  I bumped DEFAULT_NODE_SIZE to 16384 myself.
> >
> > I''ll try doing a release here soon unless you anticipate
making any more
> > changes.  My first release on Gemcutter, I guess.  Should be
interesting.
>
> Awesome, thanks.  I don''t think I have time or need to look into
more
> performance tuning here at the moment.
>
> --
> Eric Wong
>


-- 
Tony Arcieri
Medioh/Nagravision
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091128/17a03c4a/attachment.html>

Tony Arcieri <tony at medioh.com> wrote:
> Okay, while as it stands I was investigating how to push stuff to Gemcutter
> and already went ahead and pushed it. :)
> 
> It''s now released as iobuffer 0.1.2
That explains why iobuffer.o already exists when I unpack the gem :>

"gem install" does not work out-of-the-box for me:

make
cc -shared -o iobuffer.so iobuffer.o -L. -L/home/ew/lib -L. -Wl,-O1 -rdynamic
-Wl,-export-dynamic    -lc  -ldl -lcrypt -lm   -lc
iobuffer.o: file not recognized: File format not recognized
collect2: ld returned 1 exit status
make: *** [iobuffer.so] Error 1

-- 
Eric Wong

On Sat, Nov 28, 2009 at 1:31 PM, Eric Wong <normalperson at yhbt.net>
wrote:
> Tony Arcieri <tony at medioh.com> wrote:
> > Okay, while as it stands I was investigating how to push stuff to
> Gemcutter
> > and already went ahead and pushed it. :)
> >
> > It''s now released as iobuffer 0.1.2
>
> That explains why iobuffer.o already exists when I unpack the gem :>
>
> "gem install" does not work out-of-the-box for me:
>
> make
> cc -shared -o iobuffer.so iobuffer.o -L. -L/home/ew/lib -L. -Wl,-O1
> -rdynamic -Wl,-export-dynamic    -lc  -ldl -lcrypt -lm   -lc
> iobuffer.o: file not recognized: File format not recognized
> collect2: ld returned 1 exit status
> make: *** [iobuffer.so] Error 1
>
Oof...

Yeah, the build/release system is definitely all ad hoc and nasty.

Perhaps I should look at switching to hoe or echoe or whatever the new
hotness is for packaging/releasing gems.

-- 
Tony Arcieri
Medioh/Nagravision
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091128/13afcd31/attachment-0001.html>

Just redid the release... I updated the gemspec to only glob .c and .rb
files.  This should be sufficient for iobuffer and prevent future botched
releases like this in the future.

For rev it will be a bit more involved, and I''ve similarly botched such
releases in the past.

Anyway, just pushed iobuffer 0.1.3.  Let me know if that works.

On Sat, Nov 28, 2009 at 2:01 PM, Tony Arcieri <tony at medioh.com> wrote:
> On Sat, Nov 28, 2009 at 1:31 PM, Eric Wong <normalperson at yhbt.net>
wrote:
>
>> Tony Arcieri <tony at medioh.com> wrote:
>> > Okay, while as it stands I was investigating how to push stuff to
>> Gemcutter
>> > and already went ahead and pushed it. :)
>> >
>> > It''s now released as iobuffer 0.1.2
>>
>> That explains why iobuffer.o already exists when I unpack the gem :>
>>
>> "gem install" does not work out-of-the-box for me:
>>
>> make
>> cc -shared -o iobuffer.so iobuffer.o -L. -L/home/ew/lib -L. -Wl,-O1
>> -rdynamic -Wl,-export-dynamic    -lc  -ldl -lcrypt -lm   -lc
>> iobuffer.o: file not recognized: File format not recognized
>> collect2: ld returned 1 exit status
>> make: *** [iobuffer.so] Error 1
>>
>
> Oof...
>
> Yeah, the build/release system is definitely all ad hoc and nasty.
>
> Perhaps I should look at switching to hoe or echoe or whatever the new
> hotness is for packaging/releasing gems.
>
> --
> Tony Arcieri
> Medioh/Nagravision
>


-- 
Tony Arcieri
Medioh/Nagravision
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091128/cf7adc0c/attachment.html>

Tony Arcieri <tony at medioh.com> wrote:
> On Sat, Nov 28, 2009 at 1:31 PM, Eric Wong <normalperson at yhbt.net>
wrote:
> > Tony Arcieri <tony at medioh.com> wrote:
> > > Okay, while as it stands I was investigating how to push stuff to
> > Gemcutter
> > > and already went ahead and pushed it. :)
> > >
> > > It''s now released as iobuffer 0.1.2
> >
> > That explains why iobuffer.o already exists when I unpack the gem
:>
> >
> > "gem install" does not work out-of-the-box for me:
> >
> > make
> > cc -shared -o iobuffer.so iobuffer.o -L. -L/home/ew/lib -L. -Wl,-O1
> > -rdynamic -Wl,-export-dynamic    -lc  -ldl -lcrypt -lm   -lc
> > iobuffer.o: file not recognized: File format not recognized
> > collect2: ld returned 1 exit status
> > make: *** [iobuffer.so] Error 1
> >
> 
> Oof...
> 
> Yeah, the build/release system is definitely all ad hoc and nasty.
> 
> Perhaps I should look at switching to hoe or echoe or whatever the new
> hotness is for packaging/releasing gems.
I got confused/annoyed by little things in both Hoe and Echoe and just
decided to go with GNU makefiles in most of my projects :>

I''m not afraid to admit that I actually *like* programming in gmake,
and it lets me run tests in parallel and I prefer writing Bourne shell
for a lot of things, too.

-- 
Eric Wong

Tony Arcieri <tony at medioh.com> wrote:
> Anyway, just pushed iobuffer 0.1.3.  Let me know if that works.
Yup, works on both Linux x86 and x86_64  Thanks!

-- 
Eric Wong