refactor: reuse helpers where possible#737
refactor: reuse helpers where possible#737NathanBaulch wants to merge 2 commits intosamber:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #737 +/- ##
==========================================
- Coverage 94.04% 93.79% -0.26%
==========================================
Files 18 18
Lines 2838 2577 -261
==========================================
- Hits 2669 2417 -252
+ Misses 152 146 -6
+ Partials 17 14 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| return mIn, index | ||
| return MinIndexBy(collection, func(a, b T) bool { return a < b }) |
There was a problem hiding this comment.
I just made a quick benchmark to check if inlining works well.
func MinIndex2[T constraints.Ordered](collection []T) (T, int) {
return MinIndexBy(collection, func(a, b T) bool { return a < b })
}
func BenchmarkMinIndex(b *testing.B) {
arr := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}
b.Run("lo.MinIndex", func(b *testing.B) {
for n := 0; n < b.N; n++ {
_, _ = MinIndex(arr)
}
})
b.Run("lo.MinIndexNew", func(b *testing.B) {
for n := 0; n < b.N; n++ {
_, _ = MinIndex2(arr)
}
})
}
I did not investigate more, but the result is not very good:
goos: darwin
goarch: arm64
pkg: github.com/samber/lo
cpu: Apple M3
=== RUN BenchmarkMinIndex
BenchmarkMinIndex
=== RUN BenchmarkMinIndex/lo.MinIndex
BenchmarkMinIndex/lo.MinIndex
BenchmarkMinIndex/lo.MinIndex-8 329917464 3.370 ns/op 0 B/op 0 allocs/op
=== RUN BenchmarkMinIndex/lo.MinIndexNew
BenchmarkMinIndex/lo.MinIndexNew
BenchmarkMinIndex/lo.MinIndexNew-8 261727790 4.435 ns/op 0 B/op 0 allocs/op
PASS
There was a problem hiding this comment.
Ah, that's disappointing. Hopefully I can look deeper into this sometime.
Side note, it would be nice if we had a comprehensive suite of benchmarks with realistic examples that could be automatically checked for perf regressions. Someday...
3e86ff5 to
b0e1b5b
Compare
b0e1b5b to
0e42573
Compare
0e42573 to
3afe206
Compare
|
It's much worse on my machine! 😓 |
|
Ok, so I'm closing this PR for now. Improving readability is cool, but for such simple helpers, I consider the performance gain as more important. FYI, some helpers have been combined for performance reasons: Filter+Map -> FilterMap. |
This is a continuation of #691, which in turn was done in response to the huge number of code reuse opportunities I found whilst building the
itsubpackage.This PR manages to reduce the total net SLOC by 409 lines (+103 -512) by carefully using existing helpers wherever it can be done efficiently without additional memory allocations or unnecessary compute.
I've utilized first-class functions quite a lot, especially one of my favorite patterns involving
Partial,HasKeyandKeyifyto build a compact and efficient predicate, eg:I've also added internal
_map,_filterand_rejectfunctions that omit the index argument on callbacks to further improve first-class function syntax opportunities. Let me know if these should be renamed.