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

Add support for using the arena allocator #871

Merged
merged 7 commits into from
Sep 30, 2020
Merged

Add support for using the arena allocator #871

merged 7 commits into from
Sep 30, 2020

Conversation

rongou
Copy link
Collaborator

@rongou rongou commented Sep 28, 2020 •
edited
Loading

@rongou
add support for using the arena allocator
5ae9a99 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
@jlowe jlowe added this to the Sep 28 - Oct 9 milestone Sep 28, 2020
@jlowe jlowe added the performance A performance related task/issue label Sep 28, 2020
@rongou
review feedback
58264d2 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
revans2
revans2 previously approved these changes Sep 28, 2020
View reviewed changes
@rongou
switch to strategy
0e44eea 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
@rongou
clarify doc
fdb06a2 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
@rongou rongou requested a review from jlowe September 29, 2020 00:53
abellina
abellina previously approved these changes Sep 29, 2020
View reviewed changes
@wbo4958
Copy link
Collaborator

wbo4958 commented Sep 29, 2020

Hi @rongou, Could I ask what's the difference for the strategy of ARENA and DEFAULT?

@JustPlay
Copy link

Hi @rongou, Could I ask what's the difference for the strategy of ARENA and DEFAULT?

arena will use rmm's arena_memory_resouce, default will use pool_memory_resource

@rongou
deprecate pooling config
fb90394 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
@rongou
review feedback
3b3620e 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
jlowe
jlowe reviewed Sep 29, 2020
View reviewed changes
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuDeviceManager.scala Outdated
init
case c =>
logError(s"RMM pool set to '$c' is not supported.")
init
Copy link
Member

Choose a reason for hiding this comment

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

This should throw IllegalArgumentException or something similar, otherwise this is still proceeding (albeit with a log message in an executor log that the user probably will not immediately see).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@rongou
throw exception on invalid pool
c595a4a 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
@jlowe
Copy link
Member

jlowe commented Sep 29, 2020

build

@github-actions
Copy link

👎 Promotion blocked, new vulnerability found

Vulnerability report

Component Vulnerability Description Severity
Apache ZooKeeper CVE-2020-7712 This affects the package json before 10.0.0. It is possible to inject arbritary commands using the parseLookup function. HIGH

@pxLi
Copy link
Collaborator

pxLi commented Sep 30, 2020

build

1 similar comment
@rongou
Copy link
Collaborator Author

rongou commented Sep 30, 2020

build

@abellina
Copy link
Collaborator

@rongou do we want to make ARENA the default allocator, even without PTDS?

@rongou
Copy link
Collaborator Author

rongou commented Sep 30, 2020

@abellina I think the plan is to enable PTDS by default. Without PTDS, the arena allocator performs similarly to the original pool.

@abellina
Copy link
Collaborator

abellina commented Sep 30, 2020 •
edited
Loading

Ok, for me just making sure that ARENA was tested with or without PTDS, that was the main thing. Thanks @rongou

@revans2 revans2 merged commit 2603a95 into NVIDIA:branch-0.3 Sep 30, 2020
@jlowe jlowe mentioned this pull request Oct 1, 2020
Closed
sperlingxx pushed a commit to sperlingxx/spark-rapids that referenced this pull request Nov 20, 2020
@rongou @sperlingxx
Add support for using the arena allocator (NVIDIA#871)
463d4cb 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
@rongou rongou deleted the add-arena branch May 11, 2021 17:50
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@rongou
Add support for using the arena allocator (NVIDIA#871)
0e76f2b 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@rongou
Add support for using the arena allocator (NVIDIA#871)
eed34e8 
Signed-off-by: Rong Ou <rong.ou@gmail.com>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
@nvauto
Update submodule cudf to 2e86077a057e685f22cbe1472652d16d88192b7a (NV…
656d4f5 
…IDIA#871)

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revans2 revans2 revans2 left review comments

@JustPlay JustPlay JustPlay left review comments

@jlowe jlowe jlowe approved these changes

@abellina abellina abellina left review comments

Assignees
No one assigned
Labels
performance A performance related task/issue
Projects
None yet
Milestone
Sep 28 - Oct 9
Development

Successfully merging this pull request may close these issues.

[FEA] config option to enable RMM arena memory resource
7 participants
@rongou @wbo4958 @JustPlay @jlowe @pxLi @abellina @revans2