From b131e2c71a5dbbbb5c228b85b676faad6d790a23 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 13 Oct 2026 03:51:03 +0000 Subject: [PATCH] perf(bigquery): [T2-4] avoid defensive HashMap copy for ImmutableMap in InsertAllRequest.RowToInsert --- .../cloud/bigquery/InsertAllRequest.java | 59 ++++++++++++++++++- .../cloud/bigquery/InsertAllRequestTest.java | 38 ++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/InsertAllRequest.java b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/InsertAllRequest.java index ce93800987f7..6d82f1c158c7 100644 --- a/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/InsertAllRequest.java +++ b/java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/InsertAllRequest.java @@ -85,12 +85,20 @@ public static class RowToInsert implements Serializable { private final Map content; RowToInsert(String id, Map content) { + this(id, content, true); + } + + private RowToInsert(String id, Map content, boolean defensiveCopy) { this.id = id; if (content instanceof ImmutableMap) { this.content = ImmutableMap.copyOf(content); - } else { + } else if (content.isEmpty()) { + this.content = ImmutableMap.of(); + } else if (defensiveCopy) { this.content = Collections.unmodifiableMap(new HashMap<>(content)); + } else { + this.content = Collections.unmodifiableMap(content); } } @@ -153,6 +161,34 @@ public static RowToInsert of(String id, Map content) { public static RowToInsert of(Map content) { return new RowToInsert(null, checkNotNull(content)); } + + /** + * Creates a row to be inserted with associated id without defensively copying {@code content}. + * + *

The caller must not mutate {@code content} after passing it to this method. The returned + * row wraps {@code content} in an unmodifiable view when it is not already an {@link + * ImmutableMap}. + * + * @param id id of the row, used to identify duplicates + * @param content the actual content of the row + */ + public static RowToInsert ofWithoutCopy(String id, Map content) { + return new RowToInsert(checkNotNull(id), checkNotNull(content), false); + } + + /** + * Creates a row to be inserted without associated id and without defensively copying {@code + * content}. + * + *

The caller must not mutate {@code content} after passing it to this method. The returned + * row wraps {@code content} in an unmodifiable view when it is not already an {@link + * ImmutableMap}. + * + * @param content the actual content of the row + */ + public static RowToInsert ofWithoutCopy(Map content) { + return new RowToInsert(null, checkNotNull(content), false); + } } public static final class Builder { @@ -243,6 +279,27 @@ public Builder addRow(Map content) { return this; } + /** + * Adds a row to be inserted with associated id without defensively copying {@code content}. + * + *

The caller must not mutate {@code content} after passing it to this method. + */ + public Builder addRowWithoutCopy(String id, Map content) { + addRow(RowToInsert.ofWithoutCopy(id, content)); + return this; + } + + /** + * Adds a row to be inserted without an associated id and without defensively copying {@code + * content}. + * + *

The caller must not mutate {@code content} after passing it to this method. + */ + public Builder addRowWithoutCopy(Map content) { + addRow(RowToInsert.ofWithoutCopy(content)); + return this; + } + /** * Sets whether to insert all valid rows of a request, even if invalid rows exist. If not set * the entire insert request will fail if it contains an invalid row. diff --git a/java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/InsertAllRequestTest.java b/java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/InsertAllRequestTest.java index d687e75b3447..0ed0651d12e9 100644 --- a/java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/InsertAllRequestTest.java +++ b/java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/InsertAllRequestTest.java @@ -229,6 +229,44 @@ public void testNullOK() { assertThat(row.getContent()).containsExactly("foo", null); } + @Test + public void testWithoutCopy() { + Map map1 = new HashMap<>(); + map1.put("key", "val1"); + Map map2 = new HashMap<>(); + map2.put("key", "val2"); + + InsertAllRequest.RowToInsert rowWithoutId = InsertAllRequest.RowToInsert.ofWithoutCopy(map1); + assertThat(rowWithoutId.getId()).isNull(); + assertThat(rowWithoutId.getContent()).isEqualTo(CONTENT1); + assertThrows( + UnsupportedOperationException.class, () -> rowWithoutId.getContent().put("k", "v")); + + InsertAllRequest.RowToInsert rowWithId = + InsertAllRequest.RowToInsert.ofWithoutCopy("id1", map1); + assertThat(rowWithId.getId()).isEqualTo("id1"); + assertThat(rowWithId.getContent()).isEqualTo(CONTENT1); + assertThrows(UnsupportedOperationException.class, () -> rowWithId.getContent().put("k", "v")); + + InsertAllRequest requestWithoutId = + InsertAllRequest.newBuilder(TABLE_ID) + .addRowWithoutCopy(map1) + .addRowWithoutCopy(map2) + .setIgnoreUnknownValues(IGNORE_UNKNOWN_VALUES) + .setSkipInvalidRows(SKIP_INVALID_ROWS) + .build(); + compareInsertAllRequest(INSERT_ALL_REQUEST1, requestWithoutId); + + InsertAllRequest requestWithId = + InsertAllRequest.newBuilder(TABLE_ID) + .addRowWithoutCopy("id1", map1) + .addRowWithoutCopy("id2", map2) + .setIgnoreUnknownValues(IGNORE_UNKNOWN_VALUES) + .setSkipInvalidRows(SKIP_INVALID_ROWS) + .build(); + compareInsertAllRequest(INSERT_ALL_REQUEST3, requestWithId); + } + private void compareInsertAllRequest(InsertAllRequest expected, InsertAllRequest value) { assertThat(expected).isEqualTo(value); assertThat(expected.toString()).isEqualTo(value.toString());