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

Tag GpuWindow child expressions for GPU execution #6057

Merged
merged 6 commits into from
Jul 26, 2022

Conversation

mythrocks
Copy link
Collaborator

@mythrocks mythrocks commented Jul 22, 2022

Fixes #5984.

This change tags the child expressions of a GpuWindowExecMeta, to ensure
that all dependency expressions initialized correctly.

Certain child expressions might not be completely initialized even after
GpuOverrides#wrapExpr() has been called. For instance, UnixTimeExprMeta
postpones some of its initialization until tagExprForGpu() is called.

By declaring windowExpressions, partitionSpec, orderSpec, etc. as
childExprs of GpuWindowExecMeta, one guarantees that the
GpuOverrides infrastructure will tag them for GPU execution.

Signed-off-by: MithunR mythrocks@gmail.com

Fixes NVIDIA#5984.

Currently, `UnixTimeExprMeta` postpones part of the initialization of its state
to `tagExprForGpu()`. This leads to situations where an incompletely initialized
`UnixTimeExprMeta` object is exercised at runtime, such as in NVIDIA#5984.

This change moves the initialization to the constructor, leaving `tagExprForGpu()`
to deal only with tagging.

Signed-off-by: MithunR <mythrocks@gmail.com>
@mythrocks mythrocks self-assigned this Jul 22, 2022
@mythrocks mythrocks marked this pull request as draft July 22, 2022 07:50
@mythrocks
Copy link
Collaborator Author

(Tentative fix. Still considering other options to sort out #5984.)

@jlowe jlowe added this to the Jul 11 - Jul 22 milestone Jul 22, 2022
@mythrocks
Copy link
Collaborator Author

I'll post a better solution to this shortly.

This reverts commit 0101417.

This is the wrong solution. It does not protect against a different,
unsupported time-format used in a window function.
This change tags the child expressions of a GpuWindowExecMeta, to ensure
that all dependency expressions initialized correctly.

Certain child expressions might not be completely initialized even after
`GpuOverrides#wrapExpr()` has been called. For instance, `UnixTimeExprMeta`
postpones some of its initialization until `tagExprForGpu()` is called.

By declaring `windowExpressions`, `partitionSpec`, `orderSpec`, etc. as
`childExprs` of `GpuWindowExecMeta`, one guarantees that the
`GpuOverrides` infrastructure will tag them for GPU execution.

Signed-off-by: MithunR <mythrocks@gmail.com>
@mythrocks mythrocks changed the title Move initialization of UnixTimeExprMeta to constructor Tag GpuWindow child expressions for GPU execution Jul 25, 2022
@mythrocks mythrocks added the bug Something isn't working label Jul 25, 2022
@mythrocks
Copy link
Collaborator Author

I'll post a better solution to this shortly.

The solution has been modified to have GpuWindowExecMeta get its child expressions tagged for GPU.
It was a mistake to consider that GpuOverrides#wrapExpr() produces fully constructed expressions; certain of them might not be fully initialized until after tagExprForGpu() is called.

@mythrocks mythrocks marked this pull request as ready for review July 25, 2022 22:01
@mythrocks mythrocks requested a review from jlowe July 26, 2022 07:20
@jlowe
Copy link
Member

jlowe commented Jul 26, 2022

build

@mythrocks mythrocks merged commit aa6d67a into NVIDIA:branch-22.08 Jul 26, 2022
@mythrocks
Copy link
Collaborator Author

Thank you for reviewing, @jlowe. I've merged this now.

@mythrocks mythrocks deleted the tentative-fix-5984 branch February 27, 2023 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] DATABRICKS: NullPointerException: format is null in 22.08 (works fine with 22.06)
2 participants