Skip to content

Commit

Permalink
Disable fused multiply-subtract optimizations on activator calculatio…
Browse files Browse the repository at this point in the history
…ns (#8341)

Fixes TestAutoscalerPanicModeExponentialTrackAndStablize on arm64,
ppc64le and s390x.

While we are here make the order of operations match between the
expectedECB and excessBCF calculations. Floating point operations
aren't associative so performing them in the same order reduces
the risk of unexpected differences.

Background:

On some platforms (not amd64 - unless using a compiler other than
gc) the Go compiler 'fuses' some floating point operations. This
improves performance and sometimes accuracy. However this can
cause issues with test data that has been generated on a platform
without such fused operations (such as amd64) or has been constant
folded. A simplified example of the problem (values changed):

  Without fused multiply-subtract: 1.333... * 3 - 2 = 2.000...
  With fused multiply-subtract:    1.333... * 3 - 2 = 1.999...

This commit disables fused multiply-subtract operations in all
excess burst capacity calculations. This ensures that the
calculations match across all platforms and in situations where
constant folding takes place.
  • Loading branch information
mundaym committed Jun 16, 2020
1 parent fcfbb56 commit 2b325ca
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
7 changes: 4 additions & 3 deletions pkg/autoscaler/scaling/autoscaler.go
Expand Up @@ -254,9 +254,10 @@ func (a *Autoscaler) Scale(ctx context.Context, now time.Time) ScaleResult {
excessBCF = 0
// numAct stays 1, only needed to scale from 0.
case a.deciderSpec.TargetBurstCapacity > 0:
totCap := float64(originalReadyPodsCount) * a.deciderSpec.TotalValue
excessBCF = math.Floor(totCap - observedPanicValue -
a.deciderSpec.TargetBurstCapacity)
// Extra float64 cast disables fused multiply-subtract to force identical behavior on
// all platforms. See floating point section in https://golang.org/ref/spec#Operators.
totCap := float64(float64(originalReadyPodsCount) * a.deciderSpec.TotalValue)
excessBCF = math.Floor(totCap - a.deciderSpec.TargetBurstCapacity - observedPanicValue)
numAct = int32(math.Max(MinActivators,
math.Ceil((totCap+a.deciderSpec.TargetBurstCapacity)/a.deciderSpec.ActivatorCapacity)))
case a.deciderSpec.TargetBurstCapacity == -1:
Expand Down
4 changes: 3 additions & 1 deletion pkg/autoscaler/scaling/autoscaler_test.go
Expand Up @@ -70,7 +70,9 @@ func TestAutoscalerNoDataNoAutoscale(t *testing.T) {
}

func expectedEBC(totCap, targetBC, recordedConcurrency, numPods float64) int32 {
return int32(math.Floor(totCap/targetUtilization*numPods - targetBC - recordedConcurrency))
// Extra float64 cast disables fused multiply-subtract to force identical behavior on
// all platforms. See floating point section in https://golang.org/ref/spec#Operators.
return int32(math.Floor(float64(totCap/targetUtilization*numPods) - targetBC - recordedConcurrency))
}

func expectedNA(a *Autoscaler, numP float64) int32 {
Expand Down

0 comments on commit 2b325ca

Please sign in to comment.