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.

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

jitify 2 support #7372

Merged
merged 44 commits into from
Apr 5, 2021
Merged

jitify 2 support #7372

merged 44 commits into from
Apr 5, 2021

Conversation

cwharris
Copy link
Contributor

@cwharris cwharris commented Feb 11, 2021

This pr upgrades to jitify2. To accomplish this, we adopt jitify's new ProgramCache API, which handles in-memory and disk caching, allowing us to delete our jit caching code, and simplifying our jit kernel launches. Kernels and headers which were previously stringified at compile time and assembled together in cudf code at run time are now preprocessed using a new jitify_preprocess utility at compile time, which produces a single file per kernel that can be de-serialized in to a jitify program. kernel source files which have been serialized with jitifiy_preprocess include all associated headers, and jitify resolves these automatically at jit time. It also allows us to override individual headers on a per-compilation basis. This override is what we can use to inject runtime code (udfs), rather than manually concatenating strings.

I'm currently working on a bug where jitify is unable to obtain a file lock - disk caching is disable until that is fixed. Hoping to get that fixed here in the next few hours.

@github-actions github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Feb 11, 2021
@kkraus14 kkraus14 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 26, 2021
@cwharris cwharris changed the title jitify 2 support and launcher invocation cleanup jitify 2 support Mar 29, 2021
@cwharris cwharris marked this pull request as ready for review March 31, 2021 02:45
@cwharris cwharris requested review from a team as code owners March 31, 2021 02:45
@cwharris cwharris requested a review from devavret March 31, 2021 02:45
@cwharris
Copy link
Contributor Author

cwharris commented Apr 1, 2021

rerun tests

@codecov
Copy link

codecov bot commented Apr 2, 2021

Codecov Report

Merging #7372 (6084cb6) into branch-0.19 (7871e7a) will increase coverage by 0.88%.
The diff coverage is n/a.

❗ Current head 6084cb6 differs from pull request most recent head b8580e2. Consider uploading reports for the commit b8580e2 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #7372      +/-   ##
===============================================
+ Coverage        81.86%   82.74%   +0.88%     
===============================================
  Files              101      103       +2     
  Lines            16884    17700     +816     
===============================================
+ Hits             13822    14646     +824     
+ Misses            3062     3054       -8     
Impacted Files Coverage Δ
python/cudf/cudf/utils/dtypes.py 83.44% <0.00%> (-6.08%) ⬇️
python/cudf/cudf/core/column/lists.py 87.32% <0.00%> (-4.08%) ⬇️
python/cudf/cudf/core/column/decimal.py 92.92% <0.00%> (-1.95%) ⬇️
python/cudf/cudf/core/groupby/groupby.py 92.41% <0.00%> (-1.04%) ⬇️
python/cudf/cudf/utils/utils.py 85.36% <0.00%> (-0.07%) ⬇️
python/dask_cudf/dask_cudf/backends.py 89.58% <0.00%> (-0.05%) ⬇️
python/cudf/cudf/__init__.py 100.00% <0.00%> (ø)
python/cudf/cudf/io/feather.py 100.00% <0.00%> (ø)
python/cudf/cudf/core/__init__.py 100.00% <0.00%> (ø)
python/cudf/cudf/utils/ioutils.py 78.71% <0.00%> (ø)
... and 48 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dee9238...b8580e2. Read the comment docs.

@kkraus14 kkraus14 dismissed harrism’s stale review April 2, 2021 03:02

Changes addressed

Copy link
Contributor

@trxcllnt trxcllnt left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kkraus14
Copy link
Collaborator

kkraus14 commented Apr 2, 2021

I believe @cwharris has one more addition related to the in-memory cache before we should merge this

@cwharris
Copy link
Contributor Author

cwharris commented Apr 2, 2021

ready for re-review.

@kkraus14 kkraus14 requested review from devavret and trxcllnt April 2, 2021 04:34
@cwharris cwharris added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Apr 3, 2021
@kkraus14
Copy link
Collaborator

kkraus14 commented Apr 5, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 1cc41f5 into rapidsai:branch-0.19 Apr 5, 2021
shwina pushed a commit to shwina/cudf that referenced this pull request Apr 7, 2021
This pr upgrades to jitify2. To accomplish this, we adopt jitify's new `ProgramCache` API, which handles in-memory and disk caching, allowing us to delete our jit caching code, and simplifying our jit kernel launches. Kernels and headers which were previously stringified at compile time and assembled together in cudf code at run time are now preprocessed using a new `jitify_preprocess` utility at compile time, which produces a single file per kernel that can be de-serialized in to a jitify program. kernel source files which have been serialized with jitifiy_preprocess include all associated headers, and jitify resolves these automatically at jit time. It also allows us to override individual headers on a per-compilation basis. This override is what we can use to inject runtime code (udfs), rather than manually concatenating strings.

I'm currently working on a bug where jitify is unable to obtain a file lock - disk caching is disable until that is fixed. Hoping to get that fixed here in the next few hours.

Authors:
  - Christopher Harris (https://github.com/cwharris)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Devavret Makkar (https://github.com/devavret)
  - Keith Kraus (https://github.com/kkraus14)
  - Paul Taylor (https://github.com/trxcllnt)

URL: rapidsai#7372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants