feat(firestore): Add support for 16MB documents#13478
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test class, ITLargeDocumentTest, to verify Firestore's handling of large documents, covering scenarios such as reading, querying, watching, transactions, pagination, and oversized payload rejection. The feedback suggests optimizing the generation of the 16MB payload in the oversized payload test by replacing the inefficient StringBuilder loop with a pre-allocated char array and Arrays.fill() to reduce CPU and memory overhead.
| int targetBytes = 16 * 1024 * 1024 + 102400; | ||
| StringBuilder builder = new StringBuilder(targetBytes); | ||
| for (int i = 0; i < targetBytes; i++) { | ||
| builder.append('a'); | ||
| } | ||
| String largePayload = builder.toString(); |
There was a problem hiding this comment.
Creating a 16MB string using a StringBuilder in a loop with over 16 million iterations is highly inefficient and can cause significant CPU overhead and garbage collection pressure during test execution.
Instead, you can pre-allocate a char array, fill it using Arrays.fill(), and construct the String directly. This is much faster and more memory-efficient.
int targetBytes = 16 * 1024 * 1024 + 102400;
char[] chars = new char[targetBytes];
Arrays.fill(chars, 'a');
String largePayload = new String(chars);
Supports 16MB documents out of the box, so only added tests. The tests only run if the
RUN_LARGE_DOCUMENT_TESTS = trueis set. These tests rely on a seeded database.Here's the command used for testing
FIRESTORE_RUN_LARGE_DOC_TESTS=true \ FIRESTORE_TARGET_BACKEND=NIGHTLY \ FIRESTORE_EDITION=ENTERPRISE \ FIRESTORE_DATABASE_ID=<db-name>\ GOOGLE_CLOUD_PROJECT=<project-id> \ GCLOUD_PROJECT=<project-id> \ mvn test -Dtest=ITLargeDocumentTest -Dsurefire.failIfNoSpecifiedTests=false