Add UDF compiler skeleton #434

wjxiz1992 · 2020-07-27T09:58:47Z

This change adds a udf-compiler plugin without detailed compiler
implementations
To keep previous commit history and for a better review, this change
removes the detailed compiler implementation based on previous commits
This change keeps the unit test framework but removes test cases.
Compiler implementations will come in next MR

wjxiz1992 · 2020-07-27T10:08:27Z

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala

+  }
+
+  def attemptToReplaceExpression(plan: LogicalPlan, exp: Expression): Expression = {
+    exp


Entrance of compiler. implementations will come in next PR.

abellina · 2020-07-27T13:35:31Z

@wjxiz1992 for me I did want to keep history, but I think it can be squashed. There's a lot of repetition that can be made into a single commit by that same author. If others think history is not as important here, that's fine we can go with a simpler approach.

Note there are some commits at the beginning of this PR (like the metrics changes), and in the middle (like the one about splitting host/device vectors) that need to be removed all-together.

abellina · 2020-07-27T13:43:31Z

Ok I do see what happened here, so there were a number of commits, and then a new one that basically redo's everything. Yeah this is a tough one, and I am not sure we can preserve anything. At this stage @wjxiz1992 I think what would work best is to squash all commits and have a single one to introduce the skeleton.

When you think one of the commits was a collaborative effort between other authors, perhaps we can use: https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line

Thoughts?

udf-compiler/pom.xml

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala

wjxiz1992 · 2020-07-28T10:31:51Z

Ok I do see what happened here, so there were a number of commits, and then a new one that basically redo's everything. Yeah this is a tough one, and I am not sure we can preserve anything. At this stage @wjxiz1992 I think what would work best is to squash all commits and have a single one to introduce the skeleton.

When you think one of the commits was a collaborative effort between other authors, perhaps we can use: https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line

Thoughts?

I've squashed the huge commits and leave only one to describe this PR with coauthors added. I'll wait for Sean @seanprime7 to confirm if he think it's fine.

seanprime7 · 2020-07-28T17:22:39Z

Ok I do see what happened here, so there were a number of commits, and then a new one that basically redo's everything. Yeah this is a tough one, and I am not sure we can preserve anything. At this stage @wjxiz1992 I think what would work best is to squash all commits and have a single one to introduce the skeleton.
When you think one of the commits was a collaborative effort between other authors, perhaps we can use: https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line
Thoughts?

I've squashed the huge commits and leave only one to describe this PR with coauthors added. I'll wait for Sean @seanprime7 to confirm if he think it's fine.

I think it is okay to squash commits in this MR.

wjxiz1992 · 2020-07-29T07:35:22Z

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala

+
+  override def apply(plan: LogicalPlan): LogicalPlan = {
+    val conf = new RapidsConf(plan.conf)
+    if (conf.isUdfCompilerEnabled) {


Control whether to use the compiler or not.

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala

revans2

Just as a side note going forward we will require that all commits have a signoff with them.

revans2

Please note that going forward all commits will need a sign off with them.

See https://github.com/NVIDIA/spark-rapids/blob/branch-0.2/CONTRIBUTING.md#sign-your-work

revans2 · 2020-07-29T15:26:32Z

build

revans2 · 2020-07-30T13:28:55Z

The changes look good, but the build appears to be failing for no fault of your own. Could you please upmerge to the latest code so we can get a clean build?

abellina

Builds for me locally. This only replaces UDFs in project, and there are other cases where we'd want to do this. But I think we can handle that later.

Also the udf profile is ON by default, and the jar is not being added to the dist jar, I think we can figure that out later too.

But yeah verify is failing bc of the scaladoc issues, not related to the PR.

- This change adds a udf-compiler plugin without detailed compiler implementations - To keep previous commit history and for a better review, this change removes the detailed compiler implementation based on previous commits - This change keeps the unit test framework but removes test cases. - Compiler implementations will come in next MR Co-authored-by: Sean Lee <[email protected]> Co-authored-by: Nicholas Edelman <[email protected]> Co-authored-by: Alessandro Bellina <[email protected]> Signed-off-by: wjxiz1992 <[email protected]>

abellina · 2020-07-30T14:01:34Z

build

wjxiz1992 · 2020-07-30T16:16:22Z

The changes look good, but the build appears to be failing for no fault of your own. Could you please upmerge to the latest code so we can get a clean build?

Thanks for Alessandro's help, rebased to latest codes. CI passed.

abellina · 2020-07-30T16:17:24Z

thanks @wjxiz1992

- This change adds a udf-compiler plugin without detailed compiler implementations - To keep previous commit history and for a better review, this change removes the detailed compiler implementation based on previous commits - This change keeps the unit test framework but removes test cases. - Compiler implementations will come in next MR Co-authored-by: Sean Lee <[email protected]> Co-authored-by: Nicholas Edelman <[email protected]> Co-authored-by: Alessandro Bellina <[email protected]> Signed-off-by: wjxiz1992 <[email protected]> Co-authored-by: Alessandro Bellina <[email protected]> Co-authored-by: Sean Lee <[email protected]> Co-authored-by: Nicholas Edelman <[email protected]>

Signed-off-by: spark-rapids automation <[email protected]>

wjxiz1992 requested a review from abellina July 27, 2020 10:02

wjxiz1992 commented Jul 27, 2020

View reviewed changes

revans2 reviewed Jul 27, 2020

View reviewed changes

udf-compiler/pom.xml Show resolved Hide resolved

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala Outdated Show resolved Hide resolved

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala Outdated Show resolved Hide resolved

wjxiz1992 requested a review from seanprime7 July 28, 2020 08:10

wjxiz1992 force-pushed the udf-compiler branch from 20a876f to 7b9665c Compare July 28, 2020 10:18

wjxiz1992 self-assigned this Jul 28, 2020

wjxiz1992 commented Jul 29, 2020

View reviewed changes

wjxiz1992 requested a review from revans2 July 29, 2020 07:38

revans2 reviewed Jul 29, 2020

View reviewed changes

udf-compiler/src/main/scala/com/nvidia/spark/udf/Plugin.scala Outdated Show resolved Hide resolved

revans2 approved these changes Jul 29, 2020

View reviewed changes

revans2 previously approved these changes Jul 29, 2020

View reviewed changes

seanprime7 previously approved these changes Jul 30, 2020

View reviewed changes

abellina previously approved these changes Jul 30, 2020

View reviewed changes

wjxiz1992 dismissed stale reviews from abellina, seanprime7, and revans2 via 48b15fb July 30, 2020 14:00

wjxiz1992 force-pushed the udf-compiler branch from 06448bd to 48b15fb Compare July 30, 2020 14:00

abellina approved these changes Jul 30, 2020

View reviewed changes

abellina merged commit c707377 into NVIDIA:branch-0.2 Jul 30, 2020

sameerz added the feature request New feature or request label Jul 31, 2020

wjxiz1992 deleted the udf-compiler branch December 18, 2020 03:48

gerashegalov mentioned this pull request Jul 14, 2023

Factor out modules shared among shim profiles [databricks] #8711

Merged

tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023

Update submodule cudf to b68ecd1 (NVIDIA#434)

61303ff

Signed-off-by: spark-rapids automation <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add UDF compiler skeleton #434

Add UDF compiler skeleton #434

wjxiz1992 commented Jul 27, 2020

wjxiz1992 Jul 27, 2020

abellina commented Jul 27, 2020 •

edited

Loading

abellina commented Jul 27, 2020 •

edited

Loading

wjxiz1992 commented Jul 28, 2020

seanprime7 commented Jul 28, 2020 •

edited

Loading

wjxiz1992 Jul 29, 2020

revans2 left a comment

revans2 left a comment

revans2 commented Jul 29, 2020

revans2 commented Jul 30, 2020

abellina left a comment •

edited

Loading

abellina commented Jul 30, 2020

wjxiz1992 commented Jul 30, 2020

abellina commented Jul 30, 2020

Add UDF compiler skeleton #434

Add UDF compiler skeleton #434

Conversation

wjxiz1992 commented Jul 27, 2020

wjxiz1992 Jul 27, 2020

Choose a reason for hiding this comment

abellina commented Jul 27, 2020 • edited Loading

abellina commented Jul 27, 2020 • edited Loading

wjxiz1992 commented Jul 28, 2020

seanprime7 commented Jul 28, 2020 • edited Loading

wjxiz1992 Jul 29, 2020

Choose a reason for hiding this comment

revans2 left a comment

Choose a reason for hiding this comment

revans2 left a comment

Choose a reason for hiding this comment

revans2 commented Jul 29, 2020

revans2 commented Jul 30, 2020

abellina left a comment • edited Loading

Choose a reason for hiding this comment

abellina commented Jul 30, 2020

wjxiz1992 commented Jul 30, 2020

abellina commented Jul 30, 2020

abellina commented Jul 27, 2020 •

edited

Loading

abellina commented Jul 27, 2020 •

edited

Loading

seanprime7 commented Jul 28, 2020 •

edited

Loading

abellina left a comment •

edited

Loading