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

Add compiled binary operation #8192

Merged
merged 86 commits into from
Jul 13, 2021

Conversation

karthikeyann
Copy link
Contributor

@karthikeyann karthikeyann commented May 10, 2021

Add compiled binary operations (instead of jit).
(column_view lhs OP column_view rhs) => output_type
(scalar lhs OP column_view rhs) => output_type
(column_view lhs OP scalar rhs) => output_type

This PR adds compiled binary operations for fixed_width types.
string operations are already compiled.

Partially addresses #7801

Approach:

Each triple dispatcher compilation takes 60 seconds.
So, the device side dispatch is done in 2 ways.

  1. for types with common_type<Out, Lhs, Rhs>,single device side dispatch with nested individual type dispatch.
  2. double side dispatch for non-common types, with nested output type dispatch. (compiled faster than device side triple dispatch).

Each operation compile time will be close to 30 seconds.
Adds ~4 minutes compilation time (ninja -j 12) (1.5 minutes for cu files, util.cpp takes 2.5 minutes)

Identifying common_type is done in host.
Operations functor is defined only for types supported.
and for operation not supported, no operation is done in device code.

  • Identifying supported operations and types in host.
  • host side operation dispatch - different file for different operation to parallelize compile time.
  • All jit supported operations support added as compiled.
    - [ ] Coalesce, redirect Userdefined to jit.
  • scalar support added.

Unit tests:

  • tests added for numeric types and chrono type combinations for all supported operators
    It can't test all types. compilation time would be huge.
    while, covering all valid combinations of types in all operations, while keeping it minimal.

Benchmark:

  • jit binops benchmark
  • compiled binops benchmark

@karthikeyann karthikeyann added feature request New feature or request 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels May 10, 2021
@github-actions github-actions bot added the CMake CMake build issue label May 10, 2021
@codecov
Copy link

codecov bot commented May 10, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.08@7823a18). Click here to learn what that means.
The diff coverage is n/a.

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

@@               Coverage Diff               @@
##             branch-21.08    #8192   +/-   ##
===============================================
  Coverage                ?   10.67%           
===============================================
  Files                   ?      109           
  Lines                   ?    18667           
  Branches                ?        0           
===============================================
  Hits                    ?     1993           
  Misses                  ?    16674           
  Partials                ?        0           

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 7823a18...ac6edb1. Read the comment docs.

@karthikeyann karthikeyann changed the title Add compiled binary operation Add compiled binary operation [skip-ci] May 11, 2021
Comment on lines 23 to 30
// has common type
template <typename AlwaysVoid, typename... Ts>
struct has_common_type_impl : std::false_type {
};

template <typename... Ts>
struct has_common_type_impl<std::void_t<std::common_type_t<Ts...>>, Ts...> : std::true_type {
};
Copy link
Contributor

Choose a reason for hiding this comment

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

detail namespace on these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. moved few more *_impl into detail namespace.

@karthikeyann karthikeyann added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond labels Jul 13, 2021
@karthikeyann
Copy link
Contributor Author

@gpucibot merge

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 feature request New feature or request Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Convert Binary Operations to use device-side type dispatch rather than JIT
9 participants