bhollis-dbx opened a new pull request, #57321:
URL: https://github.com/apache/spark/pull/57321
### What changes were proposed in this pull request?
`from_protobuf` / `to_protobuf` used with a binary `FileDescriptorSet` hold
theirdescriptor behind a `@transient lazy val`, and building it parses the
bytes into a large `FileDescriptor` object graph -- the tree that holds the
(potentially millions of) `FieldDescriptor` instances. There is no caching, so
each task instance in an executor JVM parses the set independently. Under high
fan-out (many partitions -> many concurrent tasks in one JVM) with large
descriptor sets (hundreds of KB, thousands of transitive fields), that produces
many redundant, concurrently-retained copies of the same tree, driving
sustained executor GC pressure.
This PR adds a process-wide bounded cache at the `parseFileDescriptorSet`
layer -- the layer that materializes the `FieldDescriptor` graph -- so
concurrent task instances share one parsed graph per `FileDescriptorSet`
instead of each rebuilding it. Because both `buildDescriptorFromFDS` and the
`convertAnyFieldsToJson` `buildTypeRegistry(bytes)` path
go through this layer, a given set is parsed at most once per JVM
regardless of entry point. Building a `DescriptorWithExtensions` on top of the
cached graph is cheap and happens once per task-instance init, so it is not
itself cached; the extension-support flag is read per call, so a shared parse
still yields the flag-appropriate result.
The cache is keyed on a content hash of the descriptor bytes (which keeps
the key small and avoids pinning the byte array) and does not cache failed
parses. It uses `NonFateSharingCache` (SPARK-43300) so that a task cancelled
while populating an entry does not cause spurious failures in other tasks
blocked on the same key. Its size is bounded by a new internal config,
`spark.sql.protobuf.descriptorCacheSize` (default 8; setting it to 0 disables
the cache as a kill-switch).
### Why are the changes needed?
To eliminate redundant, concurrently-retained copies of large parsed
descriptor graphs and the resulting executor GC pressure for descriptor-heavy
`from_protobuf` / `to_protobuf` workloads.
### Does this PR introduce _any_ user-facing change?
No. A new internal config is added, but behavior is unchanged.
### How was this patch tested?
New unit tests in `ProtobufFunctionsSuite` covering: repeated builds on the
same bytes sharing one cached parse, distinct bytes cached separately, the
`buildTypeRegistry(bytes)` path sharing the parse, the extension-support flag
being honored per call over a shared parse, the disabled (size 0) kill-switch,
an unknown message name surfacing the domain exception, and a failed parse not
being cached (and surfacing the domain exception rather than a Guava wrapper).
Existing `from_protobuf` / `to_protobuf` round-trip coverage exercises
correctness with the cache enabled.
### Was this patch authored or co-authored using generative AI tooling?
Yes, drafted with assistance from generative AI tooling.
--
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]
