Skip to content

perf(bigtable): build the entry proto once in createResource - #14017

Open
laughingman7743 wants to merge 1 commit into
googleapis:mainfrom
laughingman7743:perf-bigtable-createresource-single-toproto
Open

perf(bigtable): build the entry proto once in createResource#14017
laughingman7743 wants to merge 1 commit into
googleapis:mainfrom
laughingman7743:perf-bigtable-createresource-single-toproto

Conversation

@laughingman7743

Copy link
Copy Markdown

Fixes #14016.

The defect

createResource() builds the same MutateRowsRequest.Entry twice for every element added to a
bulk-mutation batcher — once through countBytes() to read the serialized size, once directly to
read the mutation count:

@Override
public BatchResource createResource(RowMutationEntry element) {
  long byteCount = countBytes(element);
  return MutateRowsBatchResource.create(1, byteCount, element.toProto().getMutationsCount());
}

RowMutationEntry.toProto() memoizes nothing — it builds a fresh proto per call — so the second
construction is pure overhead. BatcherImpl.add() calls createResource for every accepted
element (BatcherImpl.java:232, gax 2.82.0), so this is per mutation, not per batch.

The change

Build the proto once and read both values from it. Behaviour-preserving: the two values come from
an identical proto.

countBytes is deliberately left alone — it remains part of the BatchingDescriptor contract and
is still called from TracedBatchingCallable and from the interface's own default
createResource.

What it saves

One toProto() construction per element. Measured on OpenJDK 21.0.5 (aarch64, -Xmx4g, default
collector) against google-cloud-bigtable 2.80.0 and gax 2.82.0, entries pre-built into a pool,
allocation sampled over the same loop as the timing:

Entry shape serialized size allocated today allocated after change
1 cell, 64 B value 88 B 352 B 176 B -50.0%
8 cells, 128 B values 1191 B 672 B 336 B -50.0%
64 cells, 128 B values 9533 B 2464 B 1232 B -50.0%
1000 cells, 128 B values 149 897 B 32 416 B 16 208 B -50.0%

I am not claiming a wall-clock figure. The end-to-end A/B on my machine ranged from -73% to
+23% for the same shape across JVM forks — noise, not signal — and I would rather say so than quote
the flattering end of it. The allocation halving above was exact in every fork and every shape, and
one toProto() construction measured in isolation (three forks, its own arm) costs 27.1 ns at one
cell rising to 2433.1 ns at a thousand. If you have a JMH harness you trust, its number should be
preferred to mine.

The saving is the construction only, not a serialized-size walk: Mutation.addMutation() already
calls getSerializedSize() per cell, and protobuf memoizes size per message instance, so the
shared child protos are warm before either call. The allocation grows at roughly 16 B per mutation
— two eight-byte references — which is what identifies it as reference copying rather than payload
copying.

Verification

MutateRowsBatchingDescriptorTest had no coverage of createResource or createEmptyResource at
all; both are added, and the whole class passes (9 tests, was 7).

These are characterization tests, and they do not discriminate the change — the fix is
behaviour-preserving, so they pass against the unpatched code too, which I checked rather than
assumed. What they do is pin behaviour that was previously unpinned. That they can fail at all was
verified by mutation:

Mutation applied to createResource Result
swap the byte count and the mutation count createResourceTest and createEmptyResourceTest fail
element count 1 -> 0 createResourceTest fails

Both restored afterwards. google-java-format reports both changed files compliant.

@laughingman7743
laughingman7743 requested review from a team as code owners August 8, 2026 12:32

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request optimizes the 'createResource' method in 'MutateRowsBatchingDescriptor' by calling 'element.toProto()' only once instead of twice, preventing redundant proto constructions. It also adds corresponding unit tests to verify the resource creation behavior. There are no review comments, so I have no feedback to provide.

MutateRowsBatchingDescriptor.createResource() built the same
MutateRowsRequest.Entry twice for every element added to a bulk-mutation
batcher: once through countBytes() to read the serialized size, and once
directly to read the mutation count. RowMutationEntry.toProto() memoizes
nothing, so the second construction was pure overhead, and BatcherImpl.add()
calls createResource for every accepted element.

Build the proto once and read both values from it. Behaviour-preserving.
Measured, this halves the bytes allocated per element.

MutateRowsBatchingDescriptorTest had no coverage of createResource or
createEmptyResource; both are added.
@laughingman7743
laughingman7743 force-pushed the perf-bigtable-createresource-single-toproto branch from b844208 to 59d1d1a Compare August 8, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[java-bigtable] MutateRowsBatchingDescriptor.createResource() builds the entry proto twice for every batched element

1 participant