Skip to content

Commit d1bbd8c

Browse files
remove error log pre-allocation and add benchmark
1 parent 0407c19 commit d1bbd8c

File tree

2 files changed

+57
-52
lines changed

2 files changed

+57
-52
lines changed

retry.go

Lines changed: 31 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -114,72 +114,61 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error {
114114
return nil
115115
}
116116

117-
var errorLog Error
118-
if !config.lastErrorOnly {
119-
errorLog = make(Error, config.attempts)
120-
} else {
121-
errorLog = make(Error, 1)
122-
}
117+
errorLog := Error{}
123118

124119
attemptsForError := make(map[error]uint, len(config.attemptsForError))
125120
for err, attempts := range config.attemptsForError {
126121
attemptsForError[err] = attempts
127122
}
128123

129-
lastErrIndex := n
130124
shouldRetry := true
131125
for shouldRetry {
132126
err := retryableFunc()
127+
if err == nil {
128+
return nil
129+
}
133130

134-
if err != nil {
135-
errorLog[lastErrIndex] = unpackUnrecoverable(err)
131+
errorLog = append(errorLog, unpackUnrecoverable(err))
136132

137-
if !config.retryIf(err) {
138-
break
139-
}
133+
if !config.retryIf(err) {
134+
break
135+
}
140136

141-
config.onRetry(n, err)
137+
config.onRetry(n, err)
142138

143-
for errToCheck, attempts := range attemptsForError {
144-
if errors.Is(err, errToCheck) {
145-
attempts--
146-
attemptsForError[errToCheck] = attempts
147-
shouldRetry = shouldRetry && attempts > 0
148-
}
139+
for errToCheck, attempts := range attemptsForError {
140+
if errors.Is(err, errToCheck) {
141+
attempts--
142+
attemptsForError[errToCheck] = attempts
143+
shouldRetry = shouldRetry && attempts > 0
149144
}
145+
}
150146

151-
// if this is last attempt - don't wait
152-
if n == config.attempts-1 {
153-
break
154-
}
147+
// if this is last attempt - don't wait
148+
if n == config.attempts-1 {
149+
break
150+
}
155151

156-
select {
157-
case <-config.timer.After(delay(config, n, err)):
158-
case <-config.context.Done():
159-
if config.lastErrorOnly {
160-
return config.context.Err()
161-
}
162-
n++
163-
errorLog[n] = config.context.Err()
164-
return errorLog[:lenWithoutNil(errorLog)]
152+
select {
153+
case <-config.timer.After(delay(config, n, err)):
154+
case <-config.context.Done():
155+
if config.lastErrorOnly {
156+
return config.context.Err()
165157
}
158+
n++
166159

167-
} else {
168-
return nil
160+
return append(errorLog, config.context.Err())
169161
}
170162

171163
n++
172164
shouldRetry = shouldRetry && n < config.attempts
173-
174-
if !config.lastErrorOnly {
175-
lastErrIndex = n
176-
}
177165
}
178166

179167
if config.lastErrorOnly {
180-
return errorLog[lastErrIndex]
168+
return errorLog.Unwrap()
181169
}
182-
return errorLog[:lenWithoutNil(errorLog)]
170+
171+
return errorLog
183172
}
184173

185174
func newDefaultRetryConfig() *Config {
@@ -203,7 +192,7 @@ type Error []error
203192
// Error method return string representation of Error
204193
// It is an implementation of error interface
205194
func (e Error) Error() string {
206-
logWithNumber := make([]string, lenWithoutNil(e))
195+
logWithNumber := make([]string, len(e))
207196
for i, l := range e {
208197
if l != nil {
209198
logWithNumber[i] = fmt.Sprintf("#%d: %s", i+1, l.Error())
@@ -247,17 +236,7 @@ When you need to unwrap all errors, you should use `WrappedErrors()` instead.
247236
Added in version 4.2.0.
248237
*/
249238
func (e Error) Unwrap() error {
250-
return e[lenWithoutNil(e)-1]
251-
}
252-
253-
func lenWithoutNil(e Error) (count int) {
254-
for _, v := range e {
255-
if v != nil {
256-
count++
257-
}
258-
}
259-
260-
return
239+
return e[len(e)-1]
261240
}
262241

263242
// WrappedErrors returns the list of errors that this Error is wrapping.

retry_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,3 +503,29 @@ func TestUnwrap(t *testing.T) {
503503
assert.Error(t, err)
504504
assert.Equal(t, testError, errors.Unwrap(err))
505505
}
506+
507+
func BenchmarkDo(b *testing.B) {
508+
testError := errors.New("test error")
509+
510+
for i := 0; i < b.N; i++ {
511+
Do(
512+
func() error {
513+
return testError
514+
},
515+
Attempts(10),
516+
Delay(0),
517+
)
518+
}
519+
}
520+
521+
func BenchmarkDoNoErrors(b *testing.B) {
522+
for i := 0; i < b.N; i++ {
523+
Do(
524+
func() error {
525+
return nil
526+
},
527+
Attempts(10),
528+
Delay(0),
529+
)
530+
}
531+
}

0 commit comments

Comments
 (0)