perf: Avoid making an extra heap copy in DecodeBytes #865
Conversation
WalkthroughThe recent changes involve optimizations in data handling, specifically concerning byte slices. Both Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
ValarDragon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
internal/encoding/encoding.go
Outdated
| // bz2 := make([]byte, size) | ||
| // copy(bz2, bz[n:end]) | ||
| return bz[n:end], end, nil |
There was a problem hiding this comment.
The removal of the byte slice copy operation is a significant change. It assumes that the input bytes are immutable post-read, which could lead to memory safety issues if this assumption is violated. It is crucial to verify that all current and future usages of this function adhere to this assumption.
|
@ValarDragon could we close it since it is included in #877? also #864 |
|
Sure! |
Deserializing bytes right now is making a new heap copy of the slice. In IAVL v1, with legacy nodes, over a 100 block period this appeared to be
0.3%of the CPU time. (Plus heap / memory pressure)This is only needed if the input bytes are mutated, but this is not the case in any usage of this, as each read is coming from a new
GetNodeor cache result. GetNode returns new heap-data for the value, and the cache entries do not mutate. Hence we can save on CPU time + heap pressure with this.