Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[GLUTEN-5320][VL] Reduce driver memory footprint by postpone the creation and serialization of LocalFilesNode #5321

Closed
wants to merge 1 commit into from
Closed

[GLUTEN-5320][VL] Reduce driver memory footprint by postpone the creation and serialization of LocalFilesNode #5321

wants to merge 1 commit into from

Conversation

WangGuangxin
Copy link
Contributor

What changes were proposed in this pull request?

Currently, driver generate GlutenPartition based on spark's FilePartitions, and then convert to LocalFilesNode and serialized to byte array in pb format.
This will double the driver memory, because the FilePartitions are not destroyed after convert to LocalFilesNodes.
When there are many file splits ( file status) , the impact is significant.

For example, in one of our case, there are total 48 hdfs paths to list, 7039474 files under them. With vanilla spark, it can work with driver memory = 20G, but failed in Gluten.

From the gc log, we can find that Gluten has more String and Byte[] objects than vanilla spark.

Vanilla Spark Full GC objects

 num     #instances         #bytes  class name
----------------------------------------------
   1:      42535479     8856286272  [C
   2:      42538104     1020914496  java.lang.String
   3:       7044015      563521200  java.net.URI
   4:       7039474      506842128  org.apache.hadoop.fs.LocatedFileStatus
   5:         13412      332304008  [B
   6:       7039474      281578960  org.apache.spark.sql.execution.datasources.PartitionedFile
   7:       7040016      225280512  scala.collection.mutable.LinkedHashSet$Entry
   8:       7039542      225265344  scala.collection.mutable.LinkedEntry
   9:       7039479      225263328  org.apache.hadoop.fs.permission.FsPermission
  10:          1412      151374272  [Lscala.collection.mutable.HashEntry;
  11:           145      125501688  [Lorg.apache.hadoop.fs.FileStatus;
  12:       7039625      112634000  org.apache.hadoop.fs.Path
  13:         55673       42854960  [Ljava.lang.Object;
  14:        146968       30759312  [Lorg.apache.spark.sql.execution.datasources.PartitionedFile;
  15:          2462       27069520  [J
  16:       1004712       24113088  java.util.concurrent.ConcurrentSkipListMap$Node
  17:        146968       16460416  org.apache.spark.scheduler.ResultTask
  18:        791929       12670864  scala.Some

Gluten Full GC objects (before this patch)

num     #instances         #bytes  class name
----------------------------------------------
   1:      70600217     9596405088  [C
   2:        153749     2117256784  [B
   3:      70603033     1694472792  java.lang.String
   4:      28210146      902724672  java.util.HashMap$Node
   5:       7056556      564282560  [Ljava.util.HashMap$Node;
   6:       7044001      563520080  java.net.URI
   7:       7039474      506842128  org.apache.hadoop.fs.LocatedFileStatus
   8:       7054771      338629008  java.util.HashMap
   9:       7039496      225263872  scala.collection.mutable.LinkedEntry
  10:       7039479      225263328  org.apache.hadoop.fs.permission.FsPermission
  11:       7040463      168971112  java.lang.Long
  12:        777126      135040840  [Ljava.lang.Object;
  13:       7039578      112633248  org.apache.hadoop.fs.Path
  14:          1332       67224064  [Lscala.collection.mutable.HashEntry;
  15:            97       56405176  [Lorg.apache.hadoop.fs.FileStatus;
  16:        748173       17956152  java.util.ArrayList
  17:        593611       14246664  scala.collection.immutable.$colon$colon
  18:          1919        9036728  [J

Gluten Full GC objects (after this patch)

num     #instances         #bytes  class name
----------------------------------------------
   1:      50009922    11752807376  [C
   2:      49812651     1195503624  java.lang.String
   3:       7043968      563517440  java.net.URI
   4:       7039474      506842128  org.apache.hadoop.fs.LocatedFileStatus
   5:       7039474      394210544  org.apache.spark.util.HadoopFSUtils$SerializableFileStatus
   6:         26766      259720056  [B
   7:       7039479      225263328  org.apache.hadoop.fs.permission.FsPermission
   8:       7039572      112633152  org.apache.hadoop.fs.Path
   9:         45775       68452656  [Ljava.lang.Object;
  10:       1573313       50346016  scala.collection.mutable.LinkedHashSet$Entry
  11:          1304       33665792  [Lscala.collection.mutable.HashEntry;
  12:         14435       15252040  [I
  13:            13        6756208  [Lorg.apache.hadoop.fs.FileStatus;
  14:        167935        5373920  java.util.concurrent.ConcurrentHashMap$Node
  15:        122916        3933312  java.util.Hashtable$Entry
  16:         31958        3531872  java.lang.Class
  17:         97118        3107776  scala.collection.mutable.ArrayBuilder$ofRef
  18:         97117        3107744  java.net.URI$Parser

(Fixes: #5320)

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2024

#5320

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2024

Run Gluten Clickhouse CI

@WangGuangxin
Copy link
Contributor Author

There are still some cases to fix, for example:

  1. velox backend with iceberg format
  2. clickhouse backend(which is not planed in this PR)
    But it works now on velox backend with parquet/orc format.
    Appreciate your comments in advance if you have some concerns about the interface change. cc @zhztheplayer @Yohahaha @ulysses-you @liujiayi771

liujiayi771
liujiayi771 reviewed Apr 8, 2024
View reviewed changes
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/IteratorApiImpl.scala
@@ -44,6 +42,7 @@ import org.apache.spark.util.ExecutorManager
import java.lang.{Long => JLong}
import java.nio.charset.StandardCharsets
import java.time.ZoneOffset
import java.util
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not introduce this package. Just use JArrayList.

Yohahaha
Yohahaha reviewed Apr 8, 2024
View reviewed changes
gluten-core/src/main/java/org/apache/gluten/substrait/rel/RawSplitInfo.java
Comment on lines +63 to +65
public List<String> preferredLocations() {
return Arrays.asList(filePartition.preferredLocations());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

val preferredLocations =
          SoftAffinity.getFilePartitionLocations(f)

please keep origin logic.

Yohahaha
Yohahaha reviewed Apr 8, 2024
View reviewed changes
gluten-core/src/main/scala/org/apache/gluten/backendsapi/IteratorApi.scala
@@ -91,4 +91,6 @@ trait IteratorApi {
numOutputRows: SQLMetric,
numOutputBatches: SQLMetric,
scanTime: SQLMetric): RDD[ColumnarBatch]

def toLocalFilesNodeByteArray(p: GlutenRawPartition): Array[Array[Byte]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we add a new SplitInfo object file and move this method into it with toSplitInfoByteArray? then other backends could use it more easily, and avoid add this method in IteratorApi which seems unrelated.

@Yohahaha
Copy link
Contributor

Yohahaha commented Apr 8, 2024

thank you for the improvements, this idea works for me, just few comments.

@github-actions GitHub Actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale stale label May 24, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 3, 2024

This PR was auto-closed because it has been stalled for 10 days with no activity. Please feel free to reopen if it is still valid. Thanks.

@github-actions github-actions bot closed this Jun 3, 2024
@Yohahaha
Copy link
Contributor

Yohahaha commented Jun 3, 2024

@WangGuangxin are you still working on this PR?

@WangGuangxin
Copy link
Contributor Author

@WangGuangxin are you still working on this PR?

@Yohahaha I'll rework on this this week.

@Yohahaha Yohahaha mentioned this pull request Jul 24, 2024
Closed
@Yohahaha
Copy link
Contributor

Hi @WangGuangxin
Since the current PR has not been updated for a while, I have submitted a new PR based on your code, addressing conflicts and comments. You can find it here #6572.

Feel free to request close my PR if yours is ready to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yohahaha Yohahaha Yohahaha left review comments

@liujiayi771 liujiayi771 liujiayi771 left review comments

Assignees
No one assigned
Labels
stale stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[VL] Reduce driver memory footprint by postpone the creation and serialization of LocalFilesNode
3 participants
@WangGuangxin @Yohahaha @liujiayi771