Add proper implementation of next_clear_bit for Bitset and update int…

…_allocator to use it, and fix issues with Bitset creating large fixnums.

- Fix lots of left-shift related bugs in Bitset (as we were creating a large number of BigNum's with potentially thousands of binary digits to hold our "words" due to left-shift converting FixNums to BigNums. It's still going to happen from 61-64 bits, but way better than 2048-bit fixnums being used).
- Added benchmark for IntAllocator. It used to take up to a minute to allocate 10,000 integers on a 2.7 GHz Intel Core i5, now takes 4 seconds for 65,536 (which we killed after waiting for 6 minutes using the old allocator).

benchmarks/int_allocator.rb

@@ -0,0 +1,34 @@
+$LOAD_PATH << File.expand_path(File.join(File.dirname(__FILE__), "..", "lib"))
+
+require 'amq/int_allocator'
+require "benchmark"
+
+allocator = AMQ::IntAllocator.new(1,65535)
+mutex = Mutex.new
+
+Benchmark.bm do |x|
+
+
+  x.report("allocate") do
+    allocator = AMQ::IntAllocator.new(1,65535)
+    1.upto(65534) do |i|
+      mutex.synchronize do
+        n = allocator.allocate
+        raise 'it be broke' unless n == i
+      end
+    end
+  end
+
+  x.report("allocate_with_release") do
+    allocator = AMQ::IntAllocator.new(1,65535)
+    1.upto(65534) do |i|
+      mutex.synchronize do
+        n = allocator.allocate
+        if i % 5 == 0
+          allocator.release(n)
+        end
+      end
+    end
+  end
+
+end

lib/amq/bit_set.rb

@@ -7,6 +7,7 @@ module AMQ
   # Originally part of amqp gem. Extracted to make it possible for Bunny to use it.
   class BitSet
 
+    attr_reader :words_in_use
     #
     # API
     #
@@ -28,8 +29,10 @@ def initialize(nbits)
     # @param [Integer] A bit to set
     # @api public
     def set(i)
+      check_range(i)
       w = self.word_index(i)
-      @words[w] |= (1 << i)
+      result = @words[w] |= (1 << (i % BITS_PER_WORD))
+      result
     end # set(i)
 
     # Fetches flag value for given bit.
@@ -38,9 +41,10 @@ def set(i)
     # @return [Boolean] true if given bit is set, false otherwise
     # @api public
     def get(i)
+      check_range(i)
       w = self.word_index(i)
 
-      (@words[w] & (1 << i)) != 0
+      (@words[w] & (1 << i % BITS_PER_WORD)) != 0
     end # get(i)
     alias [] get
 
@@ -49,10 +53,12 @@ def get(i)
     # @param [Integer] A bit to unset
     # @api public
     def unset(i)
+      check_range(i)
       w = self.word_index(i)
       return if w.nil?
 
-      @words[w] &= ~(1 << i)
+      result = @words[w] &= ~(1 << i % BITS_PER_WORD)
+      result
     end # unset(i)
 
     # Clears all bits in the set
@@ -61,22 +67,53 @@ def clear
       self.init_words(@nbits)
     end # clear
 
+    def next_clear_bit()
+      @words.each_with_index do |word, i|
+        if word == WORD_MASK
+          next
+        end
+        return i * BITS_PER_WORD + BitSet.number_of_trailing_ones(word)
+      end
+      -1
+    end # next_clear_bit
+
+    def to_s
+      result = ""
+      @words.each do |w|
+        result += w.to_s(2).rjust(BITS_PER_WORD,'0') + ":"
+      end
+      result
+    end # to_s
 
     #
     # Implementation
     #
 
+    # @private
+    def self.number_of_trailing_ones(num)
+      0.upto(BITS_PER_WORD) do |bit|
+        return bit if num[bit] == 0
+      end
+      BITS_PER_WORD
+    end # number_of_trailing_ones
+
+    # @private
+    def word_index(i)
+      i >> ADDRESS_BITS_PER_WORD
+    end # word_index
+
     protected
 
     # @private
     def init_words(nbits)
       n      = word_index(nbits-1) + 1
-      @words = Array.new(n) { 1 }
+      @words = Array.new(n) { 0 }
     end # init_words
 
-    # @private
-    def word_index(i)
-      i >> ADDRESS_BITS_PER_WORD
-    end # word_index(i)
+    def check_range(i)
+      if i < 0 || i >= @nbits
+        raise IndexError.new("Cannot access bit #{i} from a BitSet with #{@nbits} bits")
+      end
+    end # check_range
   end # BitSet
 end # AMQ

lib/amq/int_allocator.rb

@@ -43,10 +43,16 @@ def initialize(lo, hi)
     #
     # @return [Integer] Allocated integer if allocation succeeded. nil otherwise.
     def allocate
-      if n = find_unallocated_position
-        @free_set.set(n)
 
-        n
+      if n = @free_set.next_clear_bit
+
+        if n < @hi - 1 then
+          @free_set.set(n)
+          n + 1
+        else
+          -1
+        end
+
       else
         -1
       end
@@ -57,13 +63,13 @@ def allocate
     #
     # @return [NilClass] nil
     def free(reservation)
-      @free_set.unset(reservation)
+      @free_set.unset(reservation-1)
     end # free(reservation)
     alias release free
 
     # @return [Boolean] true if provided argument was previously allocated, false otherwise
     def allocated?(reservation)
-      @free_set.get(reservation)
+      @free_set.get(reservation-1)
     end # allocated?(reservation)
 
     # Releases the whole allocation range
@@ -75,22 +81,5 @@ def reset
 
     protected
 
-    # This implementation is significantly less efficient
-    # that what the RabbitMQ Java client has (based on java.lang.Long#nextSetBit and
-    # java.lang.Long.numberOfTrailingZeros, and thus binary search over bits).
-    # But for channel id generation, this is a good enough implementation.
-    #
-    # @private
-    def find_unallocated_position
-      r = nil
-      @range.each do |i|
-        if !@free_set.get(i)
-          r = i
-          break;
-        end
-      end
-
-      r
-    end # find_unallocated_position
   end # IntAllocator
 end # AMQ

spec/amq/bit_set_spec.rb

@@ -19,6 +19,32 @@
   # Examples
   #
 
+  describe "#new" do
+    it "has no bits set at the start" do
+      bs = AMQ::BitSet.new(128)
+      0.upto(127) do |i|
+        bs[i].should == false
+      end
+    end # it
+  end # describe
+
+  describe "#word_index" do
+    subject do
+      described_class.new(nbits)
+    end
+    it "returns 0 when the word is between 0 and 63" do
+      subject.word_index(0).should == 0
+      subject.word_index(63).should == 0
+    end # it
+    it "returns 1 when the word is between 64 and 127" do
+      subject.word_index(64).should == 1
+      subject.word_index(127).should == 1
+    end # it
+    it "returns 2 when the word is between 128 and another number" do
+      subject.word_index(128).should == 2
+    end # it
+  end # describe
+
   describe "#get, #[]" do
     describe "when bit at given position is set" do
       subject do
@@ -41,6 +67,19 @@
         subject.get(5).should be_false
       end # it
     end # describe
+
+    describe "when index out of range" do
+      subject do
+        described_class.new(nbits)
+      end
+
+      it "should raise IndexError for negative index" do
+        lambda { subject.get(-1) }.should raise_error(IndexError)
+      end # it
+      it "should raise IndexError for index >= number of bits" do
+        lambda { subject.get(nbits) }.should raise_error(IndexError)
+      end # it
+    end # describe
   end # describe
 
 
@@ -56,7 +95,7 @@
         subject.set(3)
         subject[3].should be_true
       end # it
-    end
+    end # describe
 
     describe "when bit at given position is off" do
       subject do
@@ -72,7 +111,20 @@
 
         subject.set(3387)
         subject.get(3387).should be_true
+      end # it
+    end # describe
+
+    describe "when index out of range" do
+      subject do
+        described_class.new(nbits)
       end
+
+      it "should raise IndexError for negative index" do
+        lambda { subject.set(-1) }.should raise_error(IndexError)
+      end # it
+      it "should raise IndexError for index >= number of bits" do
+        lambda { subject.set(nbits) }.should raise_error(IndexError)
+      end # it
     end # describe
   end # describe
 
@@ -103,6 +155,19 @@
         subject.get(3).should be_false
       end # it
     end # describe
+
+    describe "when index out of range" do
+      subject do
+        described_class.new(nbits)
+      end
+
+      it "should raise IndexError for negative index" do
+        lambda { subject.unset(-1) }.should raise_error(IndexError)
+      end # it
+      it "should raise IndexError for index >= number of bits" do
+        lambda { subject.unset(nbits) }.should raise_error(IndexError)
+      end # it
+    end # describe
   end # describe
 
 
@@ -123,6 +188,45 @@
 
       subject.get(3).should be_false
       subject.get(7668).should be_false
+    end # it
+  end # describe
+
+  describe "#number_of_trailing_ones" do
+    it "calculates them" do
+      described_class.number_of_trailing_ones(0).should == 0
+      described_class.number_of_trailing_ones(1).should == 1
+      described_class.number_of_trailing_ones(2).should == 0
+      described_class.number_of_trailing_ones(3).should == 2
+      described_class.number_of_trailing_ones(4).should == 0
+    end # it
+  end # describe
+
+  describe '#next_clear_bit' do
+    subject do
+      described_class.new(255)
     end
-  end
+    it "returns sequential values when none have been returned" do
+      subject.next_clear_bit.should == 0
+      subject.set(0)
+      subject.next_clear_bit.should == 1
+      subject.set(1)
+      subject.next_clear_bit.should == 2
+      subject.unset(1)
+      subject.next_clear_bit.should == 1
+    end # it
+
+    it "returns the same number as long as nothing is set" do
+      subject.next_clear_bit.should == 0
+      subject.next_clear_bit.should == 0
+    end # it
+
+    it "handles more than 128 bits" do
+      0.upto(254) do |i|
+        subject.set(i)
+        subject.next_clear_bit.should == i + 1
+      end
+      subject.unset(254)
+      subject.get(254).should be_false
+    end # it
+  end # describe
 end