Copilot commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3567459894
##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -153,6 +187,19 @@ private static long defaultMaxDecompressLength() {
Math.max(1L, Runtime.getRuntime().maxMemory() /
DEFAULT_MAX_DECOMPRESS_HEAP_FRACTION));
}
+ /**
+ * Calculate the default maximum number of zero-byte-encoded array elements
to
+ * allocate at once, as a fraction of the maximum heap. Such elements
consume no
+ * input bytes, so the usual "bytes remaining" bound does not apply and the
+ * allocation must instead be capped relative to the available memory.
+ *
+ * @return the calculated default max zero-byte element count.
+ */
Review Comment:
defaultMaxCollectionAllocation()’s Javadoc also uses "zero-byte-encoded"
wording. Since the implementation is intended for schemas whose *minimum
encoded size* is zero (including recursion-breaking cases), the comment and
`@return` text should use the same terminology for accuracy and consistency.
##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -83,6 +90,33 @@ public class SystemLimitException extends
AvroRuntimeException {
public static final long MAX_DECOMPRESS_LENGTH =
getLongLimitFromProperty(MAX_DECOMPRESS_LENGTH_PROPERTY,
defaultMaxDecompressLength());
+ /**
+ * System property declaring the maximum number of array elements whose
minimum
+ * encoded size is zero (e.g. {@code null}, a zero-length fixed, a record
whose
+ * fields are all zero-byte, or a recursive schema conservatively treated as
a 0
+ * minimum) to allocate at once: {@value}.
+ */
+ public static final String MAX_COLLECTION_ALLOCATION_PROPERTY =
"org.apache.avro.limits.collectionItems.maxAllocation";
+
+ /**
+ * Fraction of the maximum heap a single decoded collection of zero-byte
+ * elements may occupy by default. Keeps the backing allocation below the
heap
+ * so a small payload declaring a huge block count cannot exhaust the JVM.
+ */
+ private static final long DEFAULT_MAX_COLLECTION_ALLOCATION_HEAP_FRACTION =
4;
+
+ /**
+ * Estimated bytes retained per pre-allocated collection slot (a single
object
+ * reference), used to translate the heap budget into an element count.
+ */
+ private static final long BYTES_PER_COLLECTION_SLOT = 8;
+
+ /**
+ * Maximum number of zero-byte-encoded array elements to allocate at once.
+ * Recomputed from the system property (or the heap) by {@link
#resetLimits()}.
+ */
+ private static long maxCollectionAllocation =
defaultMaxCollectionAllocation();
Review Comment:
The internal Javadoc still says "zero-byte-encoded" elements, but this limit
is applied when the schema’s *minimum encoded size* is zero (see
isZeroByteSchema/minBytesPerElement, which can be 0 for recursion-breaking).
Reword this comment to match the predicate so future maintainers aren’t misled
about when the cap applies.
##########
lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java:
##########
@@ -20,18 +20,24 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
+import java.io.BufferedInputStream;
Review Comment:
The java.io imports are not in sorted order (BufferedInputStream should come
before the ByteArray* imports). This can trip Spotless/Checkstyle and makes
diffs noisier over time; please reorder them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
