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

gax: Excessive byte[] allocations with large read stream requests #3621

Closed
mina-asham opened this issue Feb 10, 2025 · 1 comment · Fixed by #3622
Closed

gax: Excessive byte[] allocations with large read stream requests #3621

mina-asham opened this issue Feb 10, 2025 · 1 comment · Fixed by #3622
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@mina-asham
Copy link
Contributor

Originally opened here: googleapis/java-bigtable#2480 then noticed it's not the right repo, so moving here for a fix.

Hi, recently we've been using the BigtableDataClient to send large requests using the readRows streaming method, we observed that with some large requests our CPUs increase to 100%, upon profiling it was mostly the GC running, running an allocation profile showed the following:
Image

Which is coming from: com.google.api.gax.rpc.StateCheckingResponseObserver#onResponse:

public final void onResponse(V response) {
  Preconditions.checkState(!isClosed, getClass() + " received a response after being closed.");
  onResponseImpl(response);
}

The string concatenation is common to be optimized into a StringBuilder by the JVM, so there is a ton of allocation coming from this bit: getClass() + " received a response after being closed.", but the exception is not being thrown here.

Pre-computing the error message every time is an issue, we need to only do that if the exception will be thrown.

Environment details

  1. Specify the API at the beginning of the title.
  2. OS type and version: Ubuntu 22
  3. Java version: OpenJDK17
  4. bigtable version(s): 2.60.0

Steps to reproduce

  1. Create a BigtableDataClient
  2. In a loop send massive requests (1K+ keys)
  3. Profile the JVM observe the time and allocations

Code example

BigtableDataSettings settings = BigtableDataSettings.newBuilder()
        .setMetricsProvider(NoopMetricsProvider.INSTANCE)
        .setProjectId("my-project")
        .setInstanceId("my-instance")
        .setAppProfileId("my-profile")
        .build();

BigtableDataClient client = BigtableDataClient.create(settings);
List<String> keys = List.of(); // 1K+ keys that exist
while (!Thread.interrupted()) {
    Query query = Query.create("my-table");
    keys.forEach(query::rowKey);
    client.readRows(query).stream().count(); // Just to consume the stream
}

Stack trace

See the screenshot above

External references such as API reference guides

N/A

Any additional information below

N/A

mina-asham added a commit to mina-asham/sdk-platform-java that referenced this issue Feb 10, 2025
- Creating the string message prematurely puts a lot of strain on the JVM and GC, switch to the format version of `Preconditions.checkState` which only constructs the string if we throw the exception
- Issue: googleapis#3621
@mina-asham
Copy link
Contributor Author

Given how small the change is, I have opened a PR for this: #3622

@zhumin8 zhumin8 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p3 Desirable enhancement or fix. May not be included in next release. labels Feb 10, 2025
@lqiu96 lqiu96 closed this as completed in f805e70 Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants