feat: Add size in bytes to CQ types#510
Conversation
⏱️ Benchmark results
|
erezrokah
left a comment
There was a problem hiding this comment.
Could we use https://github.com/DmitriyVTitov/size for this?
It seems calculating the size in bytes of values has a lot of gotchas, so maybe better to use a library that already has a few tests for it?
If not using a library, maybe use reflect.Size() https://go.dev/play/p/Sogem09ks3o
We don't want to use https://github.com/DmitriyVTitov/size/blob/master/size.go as this uses reflection. There is no much gotchas for size calculation but also we mostly use it for a ballpark number it doesn't has to be the exact number of bytes. To have the exact number of bytes we need to |
dc62fcc to
2d6bc48
Compare
| } | ||
|
|
||
| func (dst *Int8Array) Size() int { | ||
| return 8 * len(dst.Elements) |
There was a problem hiding this comment.
Can we call Int8.Size() here?
There was a problem hiding this comment.
I think this one is probably okay, the name even has 8 in it, and we're unlikely to redefine the size of 64-bit ints :) Otherwise we have to write:
if len(dst.Elements) == 0 {
return 0
}
return dst.Elements[0] * len(dst.Elements)
And then there's still the assumption that all the elements have the same size.
We could also do:
totalSize := 0
for _, element := range dst.Elements {
totalSize += element.Size()
}
return totalSize
but that would be unnecessarily inefficient, being O(N) instead of O(1)
I think the question is what we want, size in memory, or serialized size. |
🤖 I have created a release *beep* *boop* --- ## [1.22.0](v1.21.0...v1.22.0) (2023-01-06) ### Features * Add size in bytes to CQ types ([#510](#510)) ([7c15d9a](7c15d9a)) * Add WithIgnoreInTestsTransformer ([#579](#579)) ([f836abd](f836abd)) * Add WithResolverTransformer ([#578](#578)) ([5aeba0e](5aeba0e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This makes sure every CQType implements size so we can report to the users also how many bytes/MB were synced in addition to number of resources/rows.
This will also be useful in our analytics.