Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spanner: decoder should re-use arg arrays #749

Closed
jba opened this issue Sep 1, 2017 · 1 comment
Closed

spanner: decoder should re-use arg arrays #749

jba opened this issue Sep 1, 2017 · 1 comment
Assignees
Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@jba
Copy link
Contributor

jba commented Sep 1, 2017

Currently, when the spanner client decodes an array column, it always creates a new array. This can result in more memory allocation than necessary. In this example, memory for a is reallocated for each row.

var a []string
... 
for {
    row, err := iter.Next()
    ...
    ... row.Column(0, &a) ...
}

This behavior is certainly safer than re-using memory, when the caller saves a for later use. But it makes it impossible to control memory. If we instead re-used the memory of a, then users could choose whether they wanted to save memory or make a copy.

If we do this, I recommend the following algorithm, which is consistent with the BigQuery client: if the given slice is too short (which includes the case where it is nil), allocate a new one. Otherwise, slice the given one to the right length and reuse it.

/cc @pongad @vkedia

@jba jba added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. api: spanner Issues related to the Spanner API. labels Sep 1, 2017
gopherbot pushed a commit that referenced this issue Sep 5, 2017
In an attempt to reduce code duplication, I experimented with
two general-purpose array decoders, one using a passed-in function
and one using reflection.

The function-based decoder is about 15% slower. The reflection-based
decoded is around 3x slower.

Using reflection is obviously unacceptable. The function-based decoder
reduces duplication slightly. I decided not to use it, partly since
the code around array decoding may change if #749 is adopted.

I also moved all the benchmarks to the same file, and used sub-benchmarks.

Change-Id: If14cff25bdcd3541447000af84eb998bedc008d6
Reviewed-on: https://code-review.googlesource.com/16350
Reviewed-by: kokoro <[email protected]>
Reviewed-by: Vikas Kedia <[email protected]>
@jba
Copy link
Contributor Author

jba commented Dec 11, 2017

Moved to wiki.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

1 participant