Merge 94164248-error-on-zero to master

[Completes #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.."}