Skip to content

Commit 6c7fc3d

Browse files
committed
fix(faststats): correct Percentile() to use 0.0-1.0 range as documented
The Percentile() function documented that it accepts p in [0.0 - 1.0], but the implementation incorrectly checked `p >= 100` and divided by 100. This caused Var() method to return near-minimum values for all percentiles since it called Percentile(.25), Percentile(.5), etc. Fixed by changing bounds check to `p >= 1` and removing the /100 divisor. Updated tests to use correct 0.0-1.0 range values.
1 parent e808a41 commit 6c7fc3d

File tree

2 files changed

+22
-22
lines changed

2 files changed

+22
-22
lines changed

faststats/rolling_percentile.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,10 @@ func (s SortedDurations) Percentile(p float64) time.Duration {
8888
if p <= 0 {
8989
return s[0]
9090
}
91-
if p >= 100 {
91+
if p >= 1 {
9292
return s[len(s)-1]
9393
}
94-
absoluteIndex := p / 100 * float64(len(s)-1)
94+
absoluteIndex := p * float64(len(s)-1)
9595

9696
// The real value is now an approximation between here
9797
// For example, if absoluteIndex is 5.5, then we want to return a value

faststats/rolling_percentile_test.go

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -131,27 +131,27 @@ func TestRollingPercentile_AddDuration(t *testing.T) {
131131
x.AddDuration(time.Second*2, now)
132132
snap := x.SnapshotAt(now)
133133
expectSnap(t, "at one item", snap, 1, time.Second*2, map[float64]time.Duration{
134-
0: time.Second * 2,
135-
99: time.Second * 2,
136-
100: time.Second * 2,
134+
0: time.Second * 2,
135+
0.99: time.Second * 2,
136+
1.0: time.Second * 2,
137137
})
138138

139139
x.AddDuration(time.Second, now)
140140
snap = x.SnapshotAt(now)
141141
expectSnap(t, "at second item", snap, 2, time.Second*3/2, map[float64]time.Duration{
142142
0: time.Second,
143-
50: time.Second + time.Second/2,
144-
100: time.Second * 2,
143+
0.5: time.Second + time.Second/2,
144+
1.0: time.Second * 2,
145145
})
146146

147147
x.AddDuration(time.Second*3, now)
148148
snap = x.SnapshotAt(now)
149149
expectSnap(t, "at third item", snap, 3, time.Second*2, map[float64]time.Duration{
150-
0: time.Second,
151-
25: time.Second + time.Second/2,
152-
50: time.Second * 2,
153-
75: time.Second*2 + time.Second/2,
154-
100: time.Second * 3,
150+
0: time.Second,
151+
0.25: time.Second + time.Second/2,
152+
0.5: time.Second * 2,
153+
0.75: time.Second*2 + time.Second/2,
154+
1.0: time.Second * 3,
155155
})
156156
}
157157

@@ -178,15 +178,15 @@ func TestRollingPercentile_Movement(t *testing.T) {
178178
x.AddDuration(time.Millisecond*3, now)
179179
x.AddDuration(time.Millisecond*2, now.Add(time.Millisecond*500))
180180
x.AddDuration(time.Millisecond*4, now.Add(time.Millisecond*900))
181-
// should have vlaues 1, 2, 3, 4
181+
// should have values 1, 2, 3, 4
182182

183183
snap := x.SnapshotAt(now.Add(time.Millisecond * 900))
184184
expectSnap(t, "at start", snap, 4, time.Millisecond*10/4, map[float64]time.Duration{
185-
0: time.Millisecond,
186-
1.0 / 3.0 * 100: time.Millisecond * 2,
187-
50: time.Millisecond*2 + time.Millisecond/2,
188-
2.0 / 3.0 * 100: time.Millisecond * 3,
189-
100: time.Millisecond * 4,
185+
0: time.Millisecond,
186+
1.0 / 3.0: time.Millisecond * 2,
187+
0.5: time.Millisecond*2 + time.Millisecond/2,
188+
2.0 / 3.0: time.Millisecond * 3,
189+
1.0: time.Millisecond * 4,
190190
})
191191

192192
x.AddDuration(time.Millisecond*5, now.Add(time.Millisecond*1001))
@@ -195,15 +195,15 @@ func TestRollingPercentile_Movement(t *testing.T) {
195195
// expect [2, 4, 5]
196196
expectSnap(t, "after falling off", snap, 3, time.Millisecond*11/3, map[float64]time.Duration{
197197
0: time.Millisecond * 2,
198-
50: time.Millisecond * 4,
199-
100: time.Millisecond * 5,
198+
0.5: time.Millisecond * 4,
199+
1.0: time.Millisecond * 5,
200200
})
201201

202202
snap = x.SnapshotAt(now.Add(time.Hour))
203203
// All values should fall off
204204
expectSnap(t, "after all falling off", snap, 0, -1, map[float64]time.Duration{
205205
0: -1,
206-
50: -1,
207-
100: -1,
206+
0.5: -1,
207+
1.0: -1,
208208
})
209209
}

0 commit comments

Comments
 (0)