[tstate] change map[key] from string to fixed size byte array#170
[tstate] change map[key] from string to fixed size byte array#170
Conversation
Signed-off-by: Sam Batschelet <sam.batschelet@avalabs.org>
Signed-off-by: Sam Batschelet <sam.batschelet@avalabs.org>
|
Could you change the The string conversion I was doing definitely was not optimal...this is probably the right approach. |
|
You should make a similar change here: Lines 44 to 79 in a61157d |
Signed-off-by: Sam Batschelet <sam.batschelet@avalabs.org>
3b773d1 to
d8abb92
Compare
| storage := map[tstate.Key][]byte{} | ||
| for _, k := range tx.StateKeys(sm) { | ||
| sk := string(k) | ||
| sk := tstate.ToStateKeyArray(k) |
There was a problem hiding this comment.
I wonder if the best middle-ground is just to use the approach in BenchmarkOptimizedStringKeyed?
https://pthevenet.com/posts/programming/go/bytesliceindexedmaps/
We then get most of the benefit while retaining flexibility 🤷 .
There was a problem hiding this comment.
yeah the flexibility loss isn't worth the gain, working on benchmark a few alternatives including yours, hope to have that soon.
|
This PR has become stale because it has been open for 30 days with no activity. Adding the |
This is an experimentation with replacing the current internal maps from
map[string][]bytetomap[[65]byte][]byte. In benchmarks I found that using a byte array was much faster than using string as a key. I chose the length based on the largest keys I saw used by transfervm for warp. As you can see by the profiles this cut out a fairly expensive overhead ofmappassign_faststr.ref. https://gitlab.com/pthevenet/go-small-answers/-/blob/29c03407006a269f8f6ba27fa3123fb9742ddf7c/byte-slice-indexed-maps/benchmark_test.go
This reduces the allocations for
FetchAndSetScopeand provides much cleaner profile (will upload files soon).before

after
