You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by kc...@apache.org on 2008/07/09 01:20:15 UTC

svn commit: r675049 - in /incubator/thrift/trunk/lib/rb: lib/thrift/transport.rb spec/transport_spec.rb

Author: kclark
Date: Tue Jul  8 16:20:15 2008
New Revision: 675049

URL: http://svn.apache.org/viewvc?rev=675049&view=rev
Log:
rb: Buffer the slice!s in MemoryBuffer for a significant performance increase [THRIFT-63]

Author: Kevin Ballard <ke...@rapleaf.com>

Currently it buffers up to 4kB before throwing away the data.
Tests with 1MB shows the exact same performance characteristics.

Modified:
    incubator/thrift/trunk/lib/rb/lib/thrift/transport.rb
    incubator/thrift/trunk/lib/rb/spec/transport_spec.rb

Modified: incubator/thrift/trunk/lib/rb/lib/thrift/transport.rb
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/lib/thrift/transport.rb?rev=675049&r1=675048&r2=675049&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/lib/thrift/transport.rb (original)
+++ incubator/thrift/trunk/lib/rb/lib/thrift/transport.rb Tue Jul  8 16:20:15 2008
@@ -217,12 +217,16 @@
   deprecate_class! :TFramedTransportFactory => FramedTransportFactory
 
   class MemoryBuffer < Transport
+    GARBAGE_BUFFER_SIZE = 4*(2**10) # 4kB
+
     # If you pass a string to this, you should #dup that string
     # unless you want it to be modified by #read and #write
     #--
-    # yes this behavior is intentional
+    # this behavior is no longer required. If you wish to change it
+    # go ahead, just make sure the specs pass
     def initialize(buffer = nil)
       @buf = buffer || ''
+      @index = 0
     end
 
     def open?
@@ -236,20 +240,28 @@
     end
 
     def peek
-      not @buf.empty?
+      @index < @buf.size
     end
 
     # this method does not use the passed object directly but copies it
     def reset_buffer(new_buf = '')
       @buf.replace new_buf
+      @index = 0
     end
 
     def available
-      @buf.length
+      @buf.length - @index
     end
 
     def read(len)
-      @buf.slice!(0, len)
+      data = @buf.slice(@index, len)
+      @index += len
+      @index = @buf.size if @index > @buf.size
+      if @index >= GARBAGE_BUFFER_SIZE
+        @buf = @buf.slice(@index..-1)
+        @index = 0
+      end
+      data
     end
 
     def write(wbuf)
@@ -262,19 +274,17 @@
     # For fast binary protocol access
     def borrow(size = nil)
       if size.nil?
-        @buf[0..-1]
+        @buf[@index..-1]
       else
-        if size > @buf.length
+        if size > available
           raise EOFError # Memory buffers only get one shot.
         else
-          @buf[0..size]
+          @buf[@index, size]
         end
       end
     end
 
-    def consume!(size)
-      @buf.slice!(0, size)
-    end
+    alias_method :consume!, :read
   end
   deprecate_class! :TMemoryBuffer => MemoryBuffer
 

Modified: incubator/thrift/trunk/lib/rb/spec/transport_spec.rb
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/spec/transport_spec.rb?rev=675049&r1=675048&r2=675049&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/spec/transport_spec.rb (original)
+++ incubator/thrift/trunk/lib/rb/spec/transport_spec.rb Tue Jul  8 16:20:15 2008
@@ -240,7 +240,8 @@
       s = "this is a test"
       @buffer = MemoryBuffer.new(s)
       @buffer.read(4).should == "this"
-      s.should == " is a test"
+      s.slice!(-4..-1)
+      @buffer.read(@buffer.available).should == " is a "
     end
 
     it "should always remain open" do