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

Support TakeOrderedAndProject #1579

Merged
merged 3 commits into from
Jan 27, 2021
Merged

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Jan 25, 2021

This fixes #1575 and fixes #103

I did not test topN with nested types going along for the ride (not being sorted on, but in the table that is being sorted), so I didn't turn them on. They should work, but it was not a requirement.

My initial performance tests showed a little bit of a performance gain over not supporting take ordered and project, but it was not a huge amount of data so it is hard to tell.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2 revans2 added the feature request New feature or request label Jan 25, 2021
@revans2 revans2 added this to the Jan 18 - Jan 29 milestone Jan 25, 2021
@revans2 revans2 self-assigned this Jan 25, 2021
@revans2
Copy link
Collaborator Author

revans2 commented Jan 25, 2021

build

docs/FAQ.md Outdated Show resolved Hide resolved
docs/FAQ.md Outdated Show resolved Hide resolved
docs/FAQ.md Outdated Show resolved Hide resolved
docs/FAQ.md Outdated Show resolved Hide resolved
docs/FAQ.md Outdated Show resolved Hide resolved
integration_tests/src/main/python/data_gen.py Show resolved Hide resolved
docs/FAQ.md Outdated
Comment on lines 43 to 45
plan often has more metrics than the CPU versions do, and when we tried to combine all of these
operations into a single stage the metrics where confusing to understand what was happening. Instead
we split the single stage up into multiple smaller parts so the metrics are clearer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
plan often has more metrics than the CPU versions do, and when we tried to combine all of these
operations into a single stage the metrics where confusing to understand what was happening. Instead
we split the single stage up into multiple smaller parts so the metrics are clearer.
plan often has more metrics than the CPU versions do, and when we tried to combine all of these
operations into a single stage the metrics were confusing to understand. Instead we split the single
stage into multiple smaller parts so the metrics are clearer.

docs/FAQ.md Outdated
entire stage in the plan. Code generation is typically used to reduce the cost of processing data
one row at a time. The GPU plan processes the data in a columnar format, so the costs are different.

* ColumnarToRow and RowToColumnar Transitions - The CPU version of Spark plans typically process data in a row based format. The main exception to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should ColumnarToRow and RowToColumnar be backquoted, like, for example, WholeStageCodeGen is in the previous section?

Suggested change
* ColumnarToRow and RowToColumnar Transitions - The CPU version of Spark plans typically process data in a row based format. The main exception to
* `ColumnarToRow` and `RowToColumnar` transitions - The CPU version of Spark plans typically process data in a row based format. The main exception to

docs/FAQ.md Outdated Show resolved Hide resolved
@revans2
Copy link
Collaborator Author

revans2 commented Jan 26, 2021

I addressed the review comments and updated the metrics a bit. I was seeing much linger times than I expected for GpuTopN total time and I found that I had iter.next() included in the total time, which resulted in also including the time it took for the upstream batch in some cases to be built.

@revans2
Copy link
Collaborator Author

revans2 commented Jan 26, 2021

build

@jlowe
Copy link
Member

jlowe commented Jan 27, 2021

build

@revans2
Copy link
Collaborator Author

revans2 commented Jan 27, 2021

@sameerz please take another look

@revans2 revans2 merged commit 5e9ca10 into NVIDIA:branch-0.4 Jan 27, 2021
@revans2 revans2 deleted the take_ordered branch January 27, 2021 21:27
gerashegalov pushed a commit to gerashegalov/spark-rapids that referenced this pull request Jan 29, 2021
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Document differences between CPU and GPU plans [FEA] GPU version of TakeOrderedAndProject
3 participants