Skip to content

jitify 2 support - #7372

Merged
rapids-bot[bot] merged 44 commits into
NVIDIA:branch-0.19from
cwharris:jitify2
Apr 5, 2021
Merged

jitify 2 support#7372
rapids-bot[bot] merged 44 commits into
NVIDIA:branch-0.19from
cwharris:jitify2

Conversation

@cwharris

@cwharris cwharris commented Feb 11, 2021

Copy link
Copy Markdown
Contributor

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 review from codereport and devavret March 31, 2021 02:45
@cwharris
cwharris requested review from devavret and harrism April 1, 2021 19:20
@cwharris

cwharris commented Apr 1, 2021

Copy link
Copy Markdown
Contributor Author

rerun tests

Comment thread cpp/src/transform/jit/kernel.cu Outdated
@codecov

codecov Bot commented Apr 2, 2021

Copy link
Copy Markdown

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

@trxcllnt trxcllnt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 👍

@kkraus14

kkraus14 commented Apr 2, 2021

Copy link
Copy Markdown
Contributor

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

@cwharris

cwharris commented Apr 2, 2021

Copy link
Copy Markdown
Contributor Author

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

kkraus14 commented Apr 5, 2021

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 1cc41f5 into NVIDIA: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: NVIDIA#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