Reduce cudf library size #7583
Merged
Reduce cudf library size #7583
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
CMake
|
The runtime difference in benchmarks is a little interesting. I'd be curious to see results from a few other benchmarks. |
Given the background in #3044 I can run the Dispatcher benchmark, anything else you are interested in? |
Present day Jake is less convinced than past Jake that the dispatcher benchmark will be revelatory. There's no actual work being done in any of the device code in the benchmark that would be impacted. I'd maybe try some big ticket ones like join or sort. |
JOIN_BENCHSORT_BENCHTYPE_DISPATCHER_BENCH |
|
So those results just look like noise to me. Roughly +/- 5%. I think it's safe to conclude there is no runtime performance impact. |
|
How can I verify that |
Try a debug build without it? |
|
The reduction bench results are a bit concerning -- they look much less like noise than the other benchmarks. |
|
To say more definitively, if you collect 10 iterations of each benchmark, you can use the |
robertmaynard
force-pushed
the
reduce_cudf_binary_size
branch
from
0ae9aa8 to
83248f3
Compare
|
Here is the comparison of running the reduction benchmark 10 times. https://gist.github.com/robertmaynard/2cfb4ba27afcd7e52b2deccce3b78c29#file-results-txt Given the size, I have also uploaded the benchmark data so people can use |
|
@robertmaynard compare.py is capable of producing human-readable (human-pastable) output, not just JSON, so you don't have to make us go run it ourselves. :) |
Here is the raw compare results: |
|
What units are the time values in for the benchmarks? milliseconds, microseconds, nanoseconds? |
Interesting the |
harrism
approved these changes
Mar 22, 2021
jrhemstad
approved these changes
Mar 22, 2021
kkraus14
approved these changes
Mar 22, 2021
|
rerun tests |
harrism
added
improvement
|
I think this is non-breaking even with the change to release_assert -- breaks should only be internal. |
|
Oh yeah, this test is going to fail since |
robertmaynard
force-pushed
the
reduce_cudf_binary_size
branch
from
83248f3 to
1943e4e
Compare
|
@gpucibot merge |
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7583 +/- ##
===============================================
+ Coverage 81.86% 82.47% +0.60%
===============================================
Files 101 101
Lines 16884 17397 +513
===============================================
+ Hits 13822 14348 +526
+ Misses 3062 3049 -13
Continue to review full report at Codecov.
|
robertmaynard
added a commit
to robertmaynard/cuml
that referenced
this pull request
Apr 1, 2021
By explicitly telling nvcc's fatbin pass to always compress device code we can ensure that our binaries are the smallest possible size. See rapidsai/cudf#7583 for additional context.
robertmaynard
added a commit
to robertmaynard/cugraph
that referenced
this pull request
Apr 1, 2021
By explicitly telling nvcc's fatbin pass to always compress device code we can ensure that our binaries are the smallest possible size. See rapidsai/cudf#7583 for additional context.
rapids-bot bot
pushed a commit
to rapidsai/cugraph
that referenced
this pull request
Apr 2, 2021
By explicitly telling nvcc's fatbin pass to always compress device code we can ensure that our binaries are the smallest possible size. See rapidsai/cudf#7583 for additional context. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Rick Ratzel (https://github.com/rlratzel) - Brad Rees (https://github.com/BradReesWork) URL: #1503
rapids-bot bot
pushed a commit
to rapidsai/cuml
that referenced
this pull request
Apr 5, 2021
By explicitly telling nvcc's fatbin pass to always compress device code we can ensure that our binaries are the smallest possible size. See rapidsai/cudf#7583 for additional context. Authors: - Robert Maynard (https://github.com/robertmaynard) Approvers: - Dante Gama Dessavre (https://github.com/dantegd) URL: #3702
rapids-bot bot
pushed a commit
to rapidsai/cuspatial
that referenced
this pull request
Apr 8, 2021
Decrease library size by using fatbin compression. cuDF PR for context: rapidsai/cudf#7583 Authors: - Dillon Cullinan (https://github.com/dillon-cullinan) Approvers: - Paul Taylor (https://github.com/trxcllnt) - Keith Kraus (https://github.com/kkraus14) URL: #373
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR combines two major changes that improve cudf's final binary size.
The optimizations are:
__FILE__,__LINE__, etc when in release mode.With this PR we take cudf when built for
ALLarchs from1.3GBto329MB.Since this has such a dramatic change, I did some analysis on library sizes, compile times, runtime startup times, and
runtime performance.
Library sizes
We see that compression and the removal of the asserts while staying with whole compilation ( no
-rdc) is best for performance. RDC taking more space makes sense to me as it has a less agressive optimizer and therefore generates moresass/ptx.
Compile times
Compile times are interesting as we see that compression has no negative effect. I expect that the reduced IO ( from smaller binaries ) offsets the time spent compressing.
Runtime starup times for
all archs:The goal here was to time the tests to see if we had any measurable differences.
The
DISPATCHER_TEST * 12was selected as the runtime~0.3swould hopefully highligh any performance differences, and therefore running it 12 times wouldmagnify the difference and wash out any system load impacts.
I think from these rather crude measurements we are safe to assume no massive runtime loading time costs.
Runtime performance for
all archs:I used the REDUCTION_BENCH as my baseline for detecting any performance changes. The benchmarks executed on a V100 on a shared lab machine.
REDUCTION_BENCH ./compare benchmarks branch_0.19 this_compression_branch
It looks like the compression has a minor cost? I would have thought the removal of the
release_assertwould have improved performance. But maybereduceisn't using it?