[CORE] Defer Protobuf serialization of SplitInfos in GlutenPartitions#10662
[CORE] Defer Protobuf serialization of SplitInfos in GlutenPartitions#10662marin-ma merged 1 commit intoapache:mainfrom
Conversation
|
Run Gluten Clickhouse CI on x86 |
kevinwilfong
force-pushed
the
serialize_split
branch
from
a1c4d58 to
d175bfe
Compare
|
Run Gluten Clickhouse CI on x86 |
kevinwilfong
force-pushed
the
serialize_split
branch
from
d175bfe to
2e2ecae
Compare
|
Run Gluten Clickhouse CI on x86 |
|
It looks like tests are failing due to #10671 (not related to this change) |
marin-ma
left a comment
marin-ma
left a comment
There was a problem hiding this comment.
LGTM. Thanks!
Just one question regarding the solution of this performance issue: Does it mean the driver memory can be decreased with this patch because java serialisation only serialise the same object only once?
|
@marin-ma Thanks for the review!
I suspect the reason is that Spark Java serializes the GlutenPartitions as needed and does not hold the serialized values in memory for a long time. In Gluten, we're currently Protobuf serializing the SplitInfos when we create the GlutenPartitions, and I see a large number of these GlutenPartitions getting held in the Driver's memory while the query is running, so the serialized SplitInfos all exist together at the same time. If Spark is Java serializing the GlutenPartitions only when a Task is ready to execute, and evicts the serialized value from memory as soon as it's been sent to the Executor, with this change we'll only end up with a relatively small number of serialized values present in the Driver's memory at the same time (proportional to the number of Executors). |
|
We have another pr to decrease driver memory pressure, just post here to see if we can apply. |
…apache#10662) (cherry picked from commit 0d170be)
…apache#10662) (cherry picked from commit 0d170be)
3 participants
What changes are proposed in this pull request?
Today the GlutenPartition objects contain an array of byte arrays which are the Protobuf serialized ReadRel.read_type objects from the SplitInfos. The GlutenPartitions are Java serialized and sent to the Executors responsible for their respective Tasks. Looking through the code it appears we Protobuf serialize the SplitInfos so we can easily pass them across the JNI boundary.
We see the serialized SplitInfos can consume a significant amount of memory in the Driver, this is because as SplitInfo objects their state can share references tot he same objects, but once serialized they share nothing, which explodes their size in memory.
If we Java serialize the SplitInfo objects like the rest of the GlutenPartition state, and Protobuf serialize them as part of the Task, this can significantly save driver memory. The cost is a little additional memory in the Task, the size of the SplitInfo objects for a single GlutenPartition which should be trivial, and a little additional CPU instead of Protobuf serializing in the Driver and Java serializing the array of byte arrays, we Java serialize the array of SplitInfos, and on the Task we pay the additional cost of Java deserializing an array of SplitInfos and Protobuf serializing them, overall the difference is just the additional cost of Java serializing the SplitInfos instead of byte arrays.
How was this patch tested?
Ran the existing unit tests.
Verified locally that a query with particularly high Driver memory due to serialized SplitInfos saw a significant reduction.