You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "zhztheplayer (via GitHub)" <gi...@apache.org> on 2024/10/31 08:14:19 UTC

[I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

zhztheplayer opened a new issue, #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750

   ### Description
   
   So far `ColumnarBuildSideRelation` is allocated on Spark JVM heap memory.
   
   ```
   case class ColumnarBuildSideRelation(output: Seq[Attribute], batches: Array[Array[Byte]]) {
     ...
   }
   ```
   
   It appears that we can replace `batches: Array[Array[Byte]]` with an off-heap allocated container to move the memory usage to off-heap. This could avoid some of the heap OOM issues.


-- 
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: commits-unsubscribe@gluten.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "z-anderson (via GitHub)" <gi...@apache.org>.
z-anderson commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2458641132

   Hi, I would like to do this ticket to learn more about Gluten. Could you please point me to where an off-heap allocated container is used in the code? Thank you. 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "Zand100 (via GitHub)" <gi...@apache.org>.
Zand100 commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2467115178

   Could you please review the draft https://github.com/apache/incubator-gluten/pull/7885? 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "Zand100 (via GitHub)" <gi...@apache.org>.
Zand100 commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2475237284

   Could you please review https://github.com/apache/incubator-gluten/pull/7944 ? Should `ColumnarBuildSideRelation` still use the object handle (for example https://github.com/apache/incubator-gluten/blob/main/backends-velox/src/main/scala/org/apache/spark/sql/execution/ColumnarBuildSideRelation.scala#L67) and construct the `ColumnarBatch` (I think that becomes on-heap again)? 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2451321106

   Do you mean we can allocate the batch in Spark's offheap memory? if so it's a good solution.


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2458740535

   Thank you in advance for helping @Zand100 .
   
   You can refer to vanilla Spark's code [UnsafeHashedRelation](https://github.com/apache/spark/blob/4771638b3a6873e329bb3bdde5abeeeac059acb8/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala#L207) where `BytesToBytesMap` can be allocated from off-heap. Probably we can adopt a similar approach in Gluten.
   
   So I can assign this ticket to you, I suppose?


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zjuwangg (via GitHub)" <gi...@apache.org>.
zjuwangg commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2505682053

   ## Move ColumnarBuildSideRelation's memory occupation to Spark off-heap 
   
   We are very glad to see the discussion here. In our production environment, we have also been troubled by the out-of-memory (OOM) problem caused by the broadcast build relation using the heap memory for a long time. We adopted a similar approach as proposed by @zhztheplayer and made more optimizations (such as dividing large batches into small batches) in our production scenario.     
   We would like to share our approach and contribute it back in the following weeks.
   
   ### Current gluten implement
   
   
   ![image-3](https://github.com/user-attachments/assets/c6964c5e-cf6f-4851-9122-1d2b26bf072d)
   
   
   * Currently, when the ColumnarBuildSideRelation is broadcasted on the driver, the ColumnarBatch is deserialized and stored in the on-heap memory in the data structure `batches: Array[Array[Byte]]`.
   * On the executor, when the ColumnarBuildSideRelation is constructed, the batches: `Array[Array[Byte]]` still remains in on-heap memory. In some extreme situations, this will consume a large amount of heap memory. Moreover, the batch in `batches: Array[Array[Byte]] ` needs to be copied to off-heap memory through JNI, which will also be a waste of CPU resources and memory.
   
   ### Proposed design
   It went through two rounds of iterative development in our inner environment.
   
   #### Round1: using unsafe offheap to store broadcast batches on executor
   ![image](https://github.com/user-attachments/assets/f279d850-a30c-48b3-9d56-69e53728b493)
   
   
   * Introduce `BytesArrayInOffheap` to store broadcasted data in offheap memory.
   * On executor side, the broadcast data is first placed on off-heap, and during deserialize process one batch is copied/decoded each time. In this way, the memory occupation of OnHeap has become 1/N of the original (where N is the number of batches).
   
   However, it is obvious that there is room for improvement to avoid the extra copying between on-heap and off-heap memory. Additionally, another problem emerges where a certain batch in `batches: Array[Array[Byte]]` can be extremely large, which usually leads to out-of-memory (OOM) in off-heap memory. Consequently, we carried out our second round of optimization.
   
   
   #### Round2: avoid extra copy between heap/offheap and serialize more small batched to construct  `batches: Array[Array[Byte]]`
   ![image-5](https://github.com/user-attachments/assets/09d033a1-a5e3-4051-bc9e-42929fec93af)
   
   * In `BroadcastUtils#serializeStream`, support split `batches: Iterator[ColumnarBatch]` into more small batches
   * On executor side, the broadcast data is directly read from offheap. We pass one batch in `BytesArrayInOffheap`  memory address and size to underlying deserializer.
   
   
   ### Implement Steps
   
   We will introudce a config in GlutenConfig which controls whether use offheap to store the broadcast data. And when all related code get merged and after all things be done, we can remove the config and make this as default behavior.
   
   Briefly Steps:
    * Introduce `BytesArrayInOffheap`、 `UnsafeColumnarBuildSideRelation` and `spark.gluten.sql.BroadcastBuildRelationUseOffheap.enabled` in GlutenConfig to Implement what we have done in Step1
    * Add more interface `BroadcastUtils#serializeStream` and refactor the deserialize/serialize the process 
       * support split single large batch into more small batche
       * support direct read from offheap when doing deserialize in native code
    * Make the broadcast using offheap as the default behavior.
   
   
   cc @WangGuangxin @weiting-chen 
   


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2451429015

   CC @kecookier @NEUpanning 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2467161601

   @Zand100 
   
   Not necessarily to use `BytesToBytesMap` in Gluten if it's not for our use case. We can create another type of binary container implementing a Spark `MemoryConsumer` per our own demand.


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "Zand100 (via GitHub)" <gi...@apache.org>.
Zand100 commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2461242966

   Thank you! 
   
   Just to check I'm on the right track, I'm trying to use the `BytesToBytesMap` instead of `Array[Array[Bytes]]` in https://github.com/apache/incubator-gluten/blob/main/backends-velox/src/main/scala/org/apache/spark/sql/execution/ColumnarBuildSideRelation.scala#L39. Is that right?
   
   Yes, you can assign the ticket to me. 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zjuwangg (via GitHub)" <gi...@apache.org>.
zjuwangg commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2514416315

   > Thank you for the comprehensive design @zjuwangg !. Let's move forward with an initial patch.
   > 
   > Would you like to place the content in a Google doc in which everyone can comment? Thanks. You can send a mail including the Google doc link to [dev@gluten.apache.org](mailto:dev@gluten.apache.org).
   
   Initial patch can be founded here https://github.com/apache/incubator-gluten/pull/8127, and I also draft a design google doc https://docs.google.com/document/d/1eZNWPUEdiz2JPJfhyVn9hrk6SqJFRNzOMZm6u5Yredk/edit?tab=t.0#heading=h.1wu7kc4pvnqd. Eager to hear your thoughts and opinions!


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "Zand100 (via GitHub)" <gi...@apache.org>.
Zand100 commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2469367770

   Thank you! Could you please review the draft of the new binary container? https://github.com/apache/incubator-gluten/pull/7902


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2451909071

   > Do you mean we can allocate the batch in Spark's offheap memory? if so it's a good solution.
   
   Correct, I assume it could be straightforward to simlify change the allocation to off-heap. It's not similar with Gazelle's case which used to be tricky.


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "Zand100 (via GitHub)" <gi...@apache.org>.
Zand100 commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2458642458

   Hi, I would like to do this ticket to learn more about Gluten. Could you please point me to where an off-heap allocated container is used in the code? Thank you.


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2459200139

   @Zand100 You may consider to upstream the PR to upstream spark as well. It's another solution about offheap/onheap conflict, that we move all Spark's large memory allocation to offheap once offheap is enabled. 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "Zand100 (via GitHub)" <gi...@apache.org>.
Zand100 commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2466971785

   Hi, how should I handle constructing a `ColumnarBuildSideRelation` for the `HashedRelationBroadcastMode`  case? 
   
   ```
   val fromBroadcast = from.asInstanceOf[Broadcast[HashedRelation]]
   val fromRelation = fromBroadcast.value.asReadOnlyCopy()
   ```
   https://github.com/apache/incubator-gluten/blob/main/backends-velox/src/main/scala/org/apache/spark/sql/execution/BroadcastUtils.scala#L97
   
   If `fromBroadcast` is already an `UnsafeHashedRelation`, then it's already using a `BytesToBytesMap`, which is perfect. But if `fromBroadcast` is `LongHashedRelation` or `GeneralHashedRelation `, then should I convert it to `UnsafeHashedRelation`? 
   
   


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [I] [VL] Move `ColumnarBuildSideRelation`'s memory occupation to Spark off-heap [incubator-gluten]

Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on issue #7750:
URL: https://github.com/apache/incubator-gluten/issues/7750#issuecomment-2506954764

   Thank you for the comprehensive design @zjuwangg !. Let's move forward with an initial patch.
   
   Would you like to place the content in a Google doc in which everyone can comment? Thanks. You can send a mail including the Google doc link to dev@gluten.apache.org.


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org