Use floating point for MemorySize division

Glyn Normington · Glyn Normington · commit 3d237a49ab25 · 2013-08-13T09:30:14.000+01:00
Using whole number division resulted in close to default memory size
warnings in cases where the memory size was far from the default.
Using floating point division instead fixes the problem.

Add diagnostics and extra tests.

[#54997812]
diff --git a/lib/java_buildpack/jre/memory/memory_bucket.rb b/lib/java_buildpack/jre/memory/memory_bucket.rb
@@ -14,6 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+require 'java_buildpack/diagnostics/logger_factory'
 require 'java_buildpack/jre'
 require 'java_buildpack/jre/memory/memory_size'
 
@@ -47,6 +48,8 @@ def initialize(name, weighting, size, adjustable, total_memory)
       @adjustable = (validate_adjustable adjustable) && !@size_specified && total_memory
       @total_memory = total_memory ? validate_memory_size(total_memory, 'total_memory') : nil
       @size = @size_specified || default_size
+      logger = JavaBuildpack::Diagnostics::LoggerFactory.get_logger
+      logger.debug { inspect }
     end
 
     # Returns the excess memory in this memory bucket.
diff --git a/lib/java_buildpack/jre/memory/memory_size.rb b/lib/java_buildpack/jre/memory/memory_size.rb
@@ -31,7 +31,7 @@ def initialize(size)
       v = size[0..-2]
       raise "Invalid memory size '#{size}'" unless MemorySize.is_integer v
       v = size.to_i
-      # Store the number of kilobytes.
+      # Store the number of bytes.
       case unit
       when 'b', 'B'
         @bytes = v
@@ -107,8 +107,8 @@ def -(other)
     # @param [MemorySize, Numeric] other the memory size or numeric value to divide by
     # @return [MemorySize, Numeric] the result
     def /(other)
-      return @bytes / other.bytes if other.is_a? MemorySize
-      return MemorySize.from_numeric((@bytes / other).round) if other.is_a? Numeric
+      return @bytes / other.bytes.to_f if other.is_a? MemorySize
+      return MemorySize.from_numeric((@bytes / other.to_f).round) if other.is_a? Numeric
       raise "Cannot divide a MemorySize by an instance of #{other.class}"
     end
 
diff --git a/lib/java_buildpack/jre/memory/weight_balancing_memory_heuristic.rb b/lib/java_buildpack/jre/memory/weight_balancing_memory_heuristic.rb
@@ -137,6 +137,7 @@ def issue_close_to_default_warnings(buckets, heuristics, memory_limit)
               actual_size = bucket.size
               if default_size != MemorySize::ZERO
                 factor = ((actual_size - default_size) / default_size).abs
+                @logger.debug { "factor for memory size #{type} is #{factor}" }
               end
               if (default_size == MemorySize::ZERO && actual_size == MemorySize::ZERO) || factor < CLOSE_TO_DEFAULT_FACTOR
                 @logger.warn "The configured value #{actual_size} of memory size #{type} is close to the default value #{default_size}. Consider deleting the configured value and taking the default."
diff --git a/spec/java_buildpack/jre/memory/memory_size_spec.rb b/spec/java_buildpack/jre/memory/memory_size_spec.rb
@@ -98,10 +98,18 @@ module JavaBuildpack::Jre
       expect(ONE_MEG / 2).to eq(HALF_A_MEG)
     end
 
+    it 'should divide a memory size by a numeric using floating point' do
+      expect(MemorySize.new('3B') / 2).to eq(MemorySize.new('2B'))
+    end
+
     it 'should divide a memory size by another memory size correctly' do
       expect(ONE_MEG / HALF_A_MEG).to eq(2)
     end
 
+    it 'should divide a memory size by another memory size using floating point' do
+      expect(HALF_A_MEG / ONE_MEG).to eq(0.5)
+    end
+
     it 'should fail when a memory size is divided by an incorrect type' do
       expect { MemorySize.new('1B') / '' }.to raise_error(/Cannot\ divide/)
     end
diff --git a/spec/java_buildpack/jre/memory/weight_balancing_memory_heuristic_spec.rb b/spec/java_buildpack/jre/memory/weight_balancing_memory_heuristic_spec.rb
@@ -211,6 +211,13 @@ module JavaBuildpack::Jre
       end
     end
 
+    it 'should not issue a warning when the specified maximum permgen size is not close to the default' do
+      with_memory_limit('1G') do
+        WeightBalancingMemoryHeuristic.new({ 'permgen' => '128M' }, TEST_WEIGHTINGS, VALID_SIZES, VALID_HEURISTICS, JAVA_OPTS).resolve
+        expect(buildpack_log_contents).not_to match(/WARN.*is close to the default/)
+      end
+    end
+
     it 'should fail when the specified maximum memory is larger than the total memory size' do
       with_memory_limit('4096m') do
         expect { WeightBalancingMemoryHeuristic.new({ 'heap' => '5g' }, TEST_WEIGHTINGS, VALID_SIZES, VALID_HEURISTICS, JAVA_OPTS).resolve }.to raise_error(/exceeded/)
