Update the package to use generics #16
Conversation
Notes:
- The Entry interface now has a type parameter, so that it can talk
about its own underlying type.
- Methods which relied on being able to return nil for the entry now
return a pointer to the type.
- We now use the any alias instead of interface{} anywhere it was left
after adding type parameters.
- In tests, instances of `assert.Equal(t, nil, ...)` have been changed
to `assert.Nil(t, ...)`. This is because assert.Equal does not
consider the untyped nil constant to be equal to a nil pointer of a
particular type; probably it worked before because the other argument
was an interface.
Benchmarks are broadly similar. Before:
goos: linux
goarch: amd64
pkg: github.com/raviqqe/hamt
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
BenchmarkHamtInsert-8 195 6086113 ns/op
BenchmarkHamtDelete-8 710 1626577 ns/op
BenchmarkHamtFirstRestIteration-8 1 4049853564 ns/op
BenchmarkHamtForEachIteration-8 9024 112623 ns/op
BenchmarkBuiltinMapForEach-8 13258 87643 ns/op
BenchmarkBuiltinSliceForEach-8 496905 2403 ns/op
BenchmarkSetInsert-8 121 10263234 ns/op
BenchmarkSetSize-8 32 35424252 ns/op
--- BENCH: BenchmarkSetSize-8
set_test.go:160: 1
set_test.go:160: 2
set_test.go:160: 3
set_test.go:160: 4
set_test.go:160: 5
set_test.go:160: 6
set_test.go:160: 7
set_test.go:160: 8
set_test.go:160: 9
set_test.go:160: 10
... [output truncated]
PASS
ok github.com/raviqqe/hamt 27.370s
...And after:
goos: linux
goarch: amd64
pkg: github.com/raviqqe/hamt
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
BenchmarkHamtInsert-8 193 6351027 ns/op
BenchmarkHamtDelete-8 733 1539728 ns/op
BenchmarkHamtFirstRestIteration-8 1 4908351503 ns/op
BenchmarkHamtForEachIteration-8 15807 71250 ns/op
BenchmarkBuiltinMapForEach-8 13308 89030 ns/op
BenchmarkBuiltinSliceForEach-8 485073 2444 ns/op
BenchmarkSetInsert-8 138 9306362 ns/op
BenchmarkSetSize-8 37 30683603 ns/op
--- BENCH: BenchmarkSetSize-8
set_test.go:160: 1
set_test.go:160: 2
set_test.go:160: 3
set_test.go:160: 4
set_test.go:160: 5
set_test.go:160: 6
set_test.go:160: 7
set_test.go:160: 8
set_test.go:160: 9
set_test.go:160: 10
... [output truncated]
PASS
ok github.com/raviqqe/hamt 30.325s
|
Hi, thanks for this change! This is what I always wanted to do since Go introduced generics but I didn't really have time and it's a bit hard for me as I don't write Go daily anymore. So I'll try my best to review it this weekend and bump a major version! |
| return m.Find(k) != nil | ||
| } | ||
|
|
||
| // FirstRest returns a key-value pair in a map and a rest of the map. | ||
| // This method is useful for iteration. | ||
| // The key and value would be nil if the map is empty. | ||
| func (m Map) FirstRest() (Entry, interface{}, Map) { | ||
| func (m Map[K, V]) FirstRest() (*K, *V, Map[K, V]) { |
There was a problem hiding this comment.
I was thinking of this and Find operations for map types. Should we actually delegate the boxing of key and value types to users? This actually allows in-place modification of keys although it's invalid and I'm not sure if it's an intuitive interface. WDYT?
There was a problem hiding this comment.
Ah Go doesn't have Option. My brain is so Rusty now... orz
There was a problem hiding this comment.
We could add an extra ok return I suppose, by analogy to how built-in maps do this?
There was a problem hiding this comment.
Yeah. That sounds like a good alternative although I probably made it return values directly so that users can write codes in a more functional way (e.g. f(map.Find(k))) without checking flags every time.
Codecov Report
@@ Coverage Diff @@
## main #16 +/- ##
==========================================
- Coverage 95.06% 95.00% -0.07%
==========================================
Files 5 5
Lines 243 240 -3
==========================================
- Hits 231 228 -3
Misses 10 10
Partials 2 2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This patch updates the API to take advantage of the generics support in Go 1.18+. It is a breaking change, so if merged the next release should be 2.0. If you're not interested in this change let me know; I'll fork for my own purposes in that case.
Notes:
assert.Equal(t, nil, ...)have been changed toassert.Nil(t, ...). This is because assert.Equal does not consider the untyped nil constant to be equal to a nil pointer of a particular type; probably it worked before because the other argument was an interface.Benchmarks are broadly similar. Before:
goos: linux
goarch: amd64
pkg: github.com/raviqqe/hamt
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
BenchmarkHamtInsert-8 195 6086113 ns/op
BenchmarkHamtDelete-8 710 1626577 ns/op
BenchmarkHamtFirstRestIteration-8 1 4049853564 ns/op
BenchmarkHamtForEachIteration-8 9024 112623 ns/op
BenchmarkBuiltinMapForEach-8 13258 87643 ns/op
BenchmarkBuiltinSliceForEach-8 496905 2403 ns/op
BenchmarkSetInsert-8 121 10263234 ns/op
BenchmarkSetSize-8 32 35424252 ns/op
--- BENCH: BenchmarkSetSize-8
set_test.go:160: 1
set_test.go:160: 2
set_test.go:160: 3
set_test.go:160: 4
set_test.go:160: 5
set_test.go:160: 6
set_test.go:160: 7
set_test.go:160: 8
set_test.go:160: 9
set_test.go:160: 10
... [output truncated]
PASS
ok github.com/raviqqe/hamt 27.370s
...And after:
goos: linux
goarch: amd64
pkg: github.com/raviqqe/hamt
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
BenchmarkHamtInsert-8 193 6351027 ns/op
BenchmarkHamtDelete-8 733 1539728 ns/op
BenchmarkHamtFirstRestIteration-8 1 4908351503 ns/op
BenchmarkHamtForEachIteration-8 15807 71250 ns/op
BenchmarkBuiltinMapForEach-8 13308 89030 ns/op
BenchmarkBuiltinSliceForEach-8 485073 2444 ns/op
BenchmarkSetInsert-8 138 9306362 ns/op
BenchmarkSetSize-8 37 30683603 ns/op
--- BENCH: BenchmarkSetSize-8
set_test.go:160: 1
set_test.go:160: 2
set_test.go:160: 3
set_test.go:160: 4
set_test.go:160: 5
set_test.go:160: 6
set_test.go:160: 7
set_test.go:160: 8
set_test.go:160: 9
set_test.go:160: 10
... [output truncated]
PASS
ok github.com/raviqqe/hamt 30.325s