coldata: do not allocate wasteful memory for UUIDs #63231
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)
There was a problem hiding this comment.
Reviewable status:
pkg/col/coldata/bytes.go, line 51 at r1 (raw file):
// that will be added to this Bytes. It can be 0 when the length is unknown in // which case BytesInitialAllocationFactor will be used as a guess. func NewBytes(n int, avgElementLength int) *Bytes {
[nit] It's cleaner to have separate constructors instead of special values (e.g. NewBytes, NewBytesWithAvgLength). The one without the length would just call out to the general one with BytesInitialAllocationFactor. For one, you won't have to add a /* avgElementLength */ to all the call sites :)
Previously, whenever we're allocating a new `Bytes` vector, we would use 64 bytes as an average element size to decide how large of a flat byte slice to allocate initially. This is wasteful in case of Uuids because then we know that each element will be exactly of 16 bytes. This commit uses that knowledge to remove wasteful allocations. Release note: None
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/col/coldata/bytes.go, line 51 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] It's cleaner to have separate constructors instead of special values (e.g. NewBytes, NewBytesWithAvgLength). The one without the length would just call out to the general one with BytesInitialAllocationFactor. For one, you won't have to add a
/* avgElementLength */to all the call sites :)
Good point, done.
|
Build failed (retrying...): |
|
Build failed (retrying...): |
There was a problem hiding this comment.
Reviewable status:
|
Build failed (retrying...): |
|
Build succeeded: |
|
@yuzefovich is this a candidate for a backport? |
|
I think so, thanks Raphael. |
Previously, whenever we're allocating a new
Bytesvector, we would use64 bytes as an average element size to decide how large of a flat byte
slice to allocate initially. This is wasteful in case of Uuids because
then we know that each element will be exactly of 16 bytes. This commit
uses that knowledge to remove wasteful allocations.
Release note: None