HyukjinKwon commented on code in PR #59210:
URL: https://github.com/apache/spark/pull/59210#discussion_r4171484989
##########
core/src/main/scala/org/apache/spark/util/SizeEstimator.scala:
##########
@@ -344,11 +352,19 @@ object SizeEstimator extends Logging {
try {
if (field.trySetAccessible()) { // Enable future get()'s on this
field
pointerFields = field :: pointerFields
+ } else if (cls != classOf[Reference[_]]) {
+ // The field's package is not opened to Spark (e.g. java.math,
java.time), so
+ // reflection can't read it, but Unsafe can. Fields of
java.lang.ref.Reference are
+ // left unvisited: they lead to weakly reachable objects and
JVM-wide reference
+ // queues rather than to memory owned by the object.
+ unsafePointerFieldOffsets =
+ Platform.objectFieldOffset(field) :: unsafePointerFieldOffsets
}
} catch {
- // If the field isn't accessible, we can still record the pointer
size
- // but can't know more about the field, so ignore it
- case _: SecurityException =>
+ // If the field can't be read (Unsafe rejects fields of hidden
classes and records),
+ // we can still record the pointer size but can't know more about
the field, so
+ // ignore it
+ case _: SecurityException | _: UnsupportedOperationException =>
Review Comment:
**Nit (P3):** This catch is now the only thing that stops `estimate()` from
throwing when `Unsafe` rejects a field, but none of the new fixtures reaches
it. A standard JDK lambda does: on JDK 25 with the suite's `--add-opens`,
`java.util.function.Predicate.not((_: AnyRef) => true)` is a hidden class in
`java.util.function` (not opened), `trySetAccessible()` returns false for its
captured `arg$1`, and `objectFieldOffset` throws
`UnsupportedOperationException: can't get field offset on a hidden class`. If
this alternative were dropped or the call moved out of the `try`, estimating
any graph containing such a lambda would throw (failing cache insertion in
`MemoryStore`), and `SizeEstimatorSuite` would still pass. Could you add an
assertion that estimating such an object succeeds?
**Verification:**
- **Regression:** A SizeEstimator test estimates a closed-package JDK lambda
(hidden class) under the suite's default configuration, and fails if the
Unsafe-rejection exception escapes estimate.
--
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]
