david-mollitor-db opened a new pull request, #59210: URL: https://github.com/apache/spark/pull/59210
### What changes were proposed in this pull request? `SizeEstimator` walks an object graph with reflection. When `Field.trySetAccessible()` fails for a reference field, which happens for classes in JDK packages that Spark doesn't open with `--add-opens` (such as `java.math` and `java.time`), it counts the pointer but never visits the referenced object. That object, and everything it references, is left out of the estimate. This PR reads such fields through `Unsafe`, which Spark already depends on in `Platform`: - `Platform` gets two thin wrappers, `objectFieldOffset(Field)` and `getObject(Object, long)`. - `SizeEstimator.getClassInfo` records the field's offset when `trySetAccessible()` fails, and `visitSingleObject` reads those fields with `Platform.getObject`. - Fields declared in `java.lang.ref.Reference` (`referent`, `queue`, `next`, `discovered`) stay unvisited, as today: they lead to weakly reachable objects and JVM-wide reference queues rather than to memory owned by the object. Fields that `Unsafe` rejects (hidden classes, records) are also still skipped. Fields that reflection can already read are handled exactly as before. ### Why are the changes needed? `MemoryStore` weighs deserialized cached blocks with `SizeEstimator`, so values whose internals live in closed JDK packages are under-counted: - a `java.math.BigDecimal` that carries a `BigInteger` (every value with precision above 18, and smaller values built from a `BigInteger`) is missing the `BigInteger` and its `int[]` magnitude; - a `java.time.LocalDateTime`, the external type of `TIMESTAMP_NTZ`, is missing its `LocalDate` and `LocalTime`. Under-counting lets the storage pool hold more than it accounts for. Measured against JOL's `GraphLayout.totalSize()` (itself checked against `Instrumentation.getObjectSize`) on JDK 25 with Spark's executor `--add-opens`, estimate / actual for one partition of 10,000 rows: | Row shape | Before | After | |---|---|---| | `Row(Long, BigDecimal)`, `DECIMAL(38,10)` | 0.617 | 1.000 | | `Row(Long, BigDecimal)`, `DECIMAL(18,2)` built from a `BigInteger` | 0.645 | 1.000 | | `InternalRow(Long, Decimal)`, `DECIMAL(38,10)` | 0.695 | 1.000 | | `Row(Long, LocalDateTime, LocalDate)` | 0.733 | 1.000 | | same, all timestamps at midnight | 0.846 | 1.000 | The midnight case shows that shared objects are still counted once: every midnight `LocalDateTime` references the cached `LocalTime.MIDNIGHT`, and the estimate matches. Shapes without objects from closed packages (tuples, case classes, `Row`s of primitives and strings, nested collections, `UnsafeRow`) are unchanged. With `-XX:+UseCompactObjectHeaders`, the decimal shapes are also 1.000; the timestamp shapes are 1.065 and 1.074 because of a separate, pre-existing over-count of small arrays under compact object headers. ### Does this PR introduce _any_ user-facing change? No API change. Size estimates for objects that reference values in closed JDK packages now include those values, which is closer to their actual footprint. This changes memory accounting for deserialized cached blocks and for size-tracked collections built on `SizeEstimator`. ### How was this patch tested? - Added `objects whose fields reflection can't read` to `SizeEstimatorSuite`: exact sizes for a `BigDecimal` with a 117-bit unscaled value (112 bytes: `BigDecimal` + `BigInteger` + `int[4]`) and for a `LocalDateTime` (72 bytes), and a check that a `WeakReference`'s referent is still not counted. - The JOL comparison above, run with a local harness that is not part of this PR. - WIP: the test suites have not been run locally yet. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) This pull request and its description were written by Isaac. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
