Produce non-zero exit code and message if any allocation is zero.

After a successful balancing of the memory into buckets, check that none of
the explicit memory allocations is zero (after rounding down to whole
kilobytes); if one is zero, fail with an error message. Code improved to make
this easier to read in the Balance() method and removed the dependency upon
internal state for tracking the error in subsequent method calls.

The tests are improved to increase coverage and make failure testing clearer.

[#94164248]

Author: Steve Powell

integration/main_test.go

@@ -159,7 +159,7 @@ var _ = Describe("java-buildpack-memory-calculator executable", func() {
 
 			It("fails with an error", func() {
 				Ω(cmdErr).Should(HaveOccurred(), "exit status")
-				Ω(string(sErr)).Should(ContainSubstring("Cannot balance memory: Total memory exceeded by configuration: ["), "stderr")
+				Ω(string(sErr)).Should(ContainSubstring("Cannot balance memory: Memory allocation failed for configuration: ["), "stderr")
 				Ω(string(sOut)).Should(Equal(""), "stdout")
 			})
 		})

memory/allocator.go

@@ -34,7 +34,6 @@ type allocator struct {
 	originalSizes map[string]Range  // unmodified after creation
 	buckets       map[string]Bucket // named buckets for allocation
 	warnings      []string          // warnings if allocation found issues
-	failure       error             // error if allocation failed
 }
 
 func NewAllocator(sizes map[string]Range, heuristics map[string]float64) (*allocator, error) {
@@ -57,21 +56,20 @@ const (
 // Balance memory between buckets, adjusting stack units, observing
 // constraints, and detecting memory wastage and default proximity.
 func (a *allocator) Balance(memLimit MemSize) error {
-	// Adjust stack bucket, if it exists
+	// adjust stack bucket, if it exists
 	stackBucket, estNumThreads := a.normaliseStack(memLimit)
 
-	// balance buckets
-	a.balance(memLimit)
+	// distribute memory among the buckets
+	if berr := a.balance(memLimit); berr != nil {
+		return fmt.Errorf("Memory allocation failed for configuration: %v, : %s", getSizes(a.originalSizes), berr)
+	}
 
-	// Validate result and gather warnings
+	// validate result and gather warnings
 	a.validateAllocation(memLimit)
 
-	// Re-adjust stack bucket, if it exists
+	// reset stack bucket, if it exists
 	a.unnormaliseStack(stackBucket, estNumThreads)
 
-	if a.failure != nil {
-		return fmt.Errorf("Total memory exceeded by configuration: %v", getSizes(a.originalSizes))
-	}
 	return nil
 }
 
@@ -82,9 +80,6 @@ func (a *allocator) SetLowerBounds() {
 }
 
 func (a *allocator) Switches(sfs switches.Funs) []string {
-	if a.failure != nil {
-		return nil
-	}
 	var strs = make([]string, 0, 10)
 	for s, b := range a.buckets {
 		strs = append(strs, sfs.Apply(s, b.GetSize().String())...)
@@ -93,12 +88,10 @@ func (a *allocator) Switches(sfs switches.Funs) []string {
 }
 
 func (a *allocator) GetWarnings() []string {
-	if a.failure != nil {
-		return nil
-	}
 	return a.warnings
 }
 
+// getSizes returns a slice of memory type range strings
 func getSizes(ss map[string]Range) []string {
 	result := []string{}
 	for n, s := range ss {
@@ -130,6 +123,7 @@ func totalWeight(bs map[string]Bucket) float64 {
 	return w
 }
 
+// Replace stack bucket to make it represent total memory for stacks temporarily
 func (a *allocator) normaliseStack(memLimit MemSize) (originalStackBucket Bucket, estNumThreads float64) {
 	if sb, ok := a.buckets["stack"]; ok {
 		stackMem := weightedSize(totalWeight(a.buckets), memLimit, sb)
@@ -146,6 +140,7 @@ func weightedSize(totWeight float64, memLimit MemSize, b Bucket) float64 {
 	return (float64(memLimit) * b.Weight()) / totWeight
 }
 
+// Replace stack bucket, and set size per thread
 func (a *allocator) unnormaliseStack(sb Bucket, estNum float64) {
 	if sb == nil {
 		return
@@ -156,18 +151,26 @@ func (a *allocator) unnormaliseStack(sb Bucket, estNum float64) {
 }
 
 // Balance memory between buckets, observing constraints.
-func (a *allocator) balance(memLimit MemSize) {
+func (a *allocator) balance(memLimit MemSize) error {
 	remaining := copyBucketMap(a.buckets)
 	removed := true
 
 	for removed && len(remaining) != 0 {
 		var err error
 		memLimit, removed, err = balanceOrRemove(remaining, memLimit)
 		if err != nil {
-			a.failure = err
-			return
+			return err
 		}
 	}
+
+	// check for zero allocations
+	for n, b := range a.buckets {
+		if b.GetSize().String() == "0" {
+			return fmt.Errorf("Cannot allocate memory to '%n' type", n)
+		}
+	}
+
+	return nil
 }
 
 func (a *allocator) validateAllocation(memLimit MemSize) {
@@ -179,16 +182,8 @@ func (a *allocator) validateAllocation(memLimit MemSize) {
 
 func memoryWastageWarnings(bs map[string]Bucket, memLimit MemSize) []string {
 	warnings := []string{}
-	if nativeBucket, ok := bs["native"]; ok && nativeBucket.Range().Floor() == MEMSIZE_ZERO {
-		totWeight := totalWeight(bs)
-		floatSize := NATIVE_MEMORY_WARNING_FACTOR * weightedSize(totWeight, memLimit, nativeBucket)
-		if MemSize(math.Floor(0.5 + floatSize)).LessThan(*nativeBucket.GetSize()) {
-			warnings = append(warnings,
-				fmt.Sprintf(
-					"There is more than %g times more spare native memory than the default "+
-						"so configured Java memory may be too small or available memory may be too large",
-					NATIVE_MEMORY_WARNING_FACTOR))
-		}
+	if nb, ok := bs["native"]; ok {
+		warnings = append(warnings, nativeBucketWarning(nb, totalWeight(bs), memLimit)...)
 	}
 
 	totalSize := MEMSIZE_ZERO
@@ -205,6 +200,19 @@ func memoryWastageWarnings(bs map[string]Bucket, memLimit MemSize) []string {
 	return warnings
 }
 
+func nativeBucketWarning(nativeBucket Bucket, totWeight float64, memLimit MemSize) []string {
+	if nativeBucket.Range().Floor() == MEMSIZE_ZERO {
+		floatSize := NATIVE_MEMORY_WARNING_FACTOR * weightedSize(totWeight, memLimit, nativeBucket)
+		if MemSize(math.Floor(0.5 + floatSize)).LessThan(*nativeBucket.GetSize()) {
+			return []string{fmt.Sprintf(
+				"There is more than %g times more spare native memory than the default "+
+					"so configured Java memory may be too small or available memory may be too large",
+				NATIVE_MEMORY_WARNING_FACTOR)}
+		}
+	}
+	return []string{}
+}
+
 func closeToDefaultWarnings(bs map[string]Bucket, sizes map[string]Range, memLimit MemSize) []string {
 	totWeight := totalWeight(bs)
 	warnings := []string{}

memory/allocator_test.go

@@ -56,10 +56,6 @@ var _ = Describe("Allocator", func() {
 		}
 	})
 
-	JustBeforeEach(func() {
-		a = shouldWork(memory.NewAllocator(convertToRanges(sizes), weights))
-	})
-
 	Context("constructor", func() {
 
 		Context("with good parameters", func() {
@@ -79,6 +75,10 @@ var _ = Describe("Allocator", func() {
 				}
 			})
 
+			JustBeforeEach(func() {
+				a = shouldWork(memory.NewAllocator(convertToRanges(sizes), weights))
+			})
+
 			It("succeeds", func() {
 				Ω(memory.GetBuckets(a)).Should(ConsistOf(
 					"Bucket{name: stack, size: <nil>, range: 2M..2M, weight: 1}",
@@ -108,14 +108,89 @@ var _ = Describe("Allocator", func() {
 				)) // heap, permgen, stack
 			})
 		})
+	})
+
+	Context("balancing", func() {
+		var (
+			memLimit = memory.MEMSIZE_ZERO
+			aerr     error
+		)
+
+		JustBeforeEach(func() {
+			a = shouldWork(memory.NewAllocator(convertToRanges(sizes), weights))
+			aerr = a.Balance(memLimit)
+		})
+
+		Context("badly", func() {
+
+			JustBeforeEach(func() {
+				Ω(aerr).Should(HaveOccurred())
+			})
+
+			Context("with no memory and one bucket", func() {
+				BeforeEach(func() {
+					sizes = strmap{"heap": "0.."}
+					weights = floatmap{"heap": 5.0}
+					memLimit = memory.MEMSIZE_ZERO
+				})
+				It("fails", func() {})
+			})
+
+			Context("with not enough memory and one bucket", func() {
+				BeforeEach(func() {
+					sizes = strmap{"heap": "64m.."}
+					weights = floatmap{"heap": 5.0}
+					memLimit = memory.NewMemSize(32 * mEGA)
+				})
+				It("fails", func() {})
+			})
+
+			Context("with not enough memory and two buckets", func() {
+				BeforeEach(func() {
+					sizes = strmap{"heap": "33m..", "hope": "32m.."}
+					weights = floatmap{"heap": 1.0, "hope": 1.0}
+					memLimit = memory.NewMemSize(64 * mEGA)
+				})
+				It("fails", func() {})
+			})
 
-		Context("good balancing", func() {
-			var (
-				memLimit = memory.MemSize(0)
-			)
+			Context("with just enough memory for one out of two buckets", func() {
+				BeforeEach(func() {
+					sizes = strmap{"heap": "38m..", "hope": ".."}
+					weights = floatmap{"heap": 1.0, "hope": 1.0}
+					memLimit = memory.NewMemSize(38 * mEGA)
+				})
+				It("fails", func() {})
+			})
+		})
 
+		Context("well", func() {
 			JustBeforeEach(func() {
-				Ω(a.Balance(memLimit)).ShouldNot(HaveOccurred())
+				Ω(aerr).ShouldNot(HaveOccurred())
+			})
+
+			Context("with exactly enough memory and one bucket", func() {
+				BeforeEach(func() {
+					sizes = strmap{"heap": "64m.."}
+					weights = floatmap{"heap": 5.0}
+					memLimit = memory.NewMemSize(64 * mEGA)
+				})
+				It("fills the bucket up", func() {
+					Ω(memory.GetBuckets(a)).Should(ConsistOf(
+						"Bucket{name: heap, size: 64M, range: 64M.., weight: 5}",
+					))
+				})
+			})
+
+			Context("with no memory and no buckets", func() {
+				BeforeEach(func() {
+					sizes = strmap{"heap": "0.."}
+					weights = floatmap{}
+					memLimit = memory.MEMSIZE_ZERO
+				})
+				It("results in no buckets", func() {
+					Ω(memory.GetBuckets(a)).Should(BeEmpty())
+				})
 			})
 
 			Context("with single bucket to 'balance'", func() {
@@ -133,7 +208,7 @@ var _ = Describe("Allocator", func() {
 				})
 			})
 
-			Context("with no buckets to 'balance'", func() {
+			Context("with some memory and no buckets to 'balance'", func() {
 
 				BeforeEach(func() {
 					sizes = strmap{}
@@ -146,7 +221,7 @@ var _ = Describe("Allocator", func() {
 				})
 			})
 
-			Context("with two buckets to balance", func() {
+			Context("with some memory and two buckets to balance", func() {
 
 				BeforeEach(func() {
 					sizes = strmap{"heap": "0..", "hope": "0.."}