-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[SPARK-21603][SQL]The wholestage codegen will be much slower then that is closed when the function is too long #18810
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
Closed
Closed
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
ca9eff6
The wholestage codegen will be slower when the function is too long
eatoncys 1b0ac5e
The wholestage codegen will be slower when the function is too long
eatoncys 5582f00
Modify annotation of existTooLongFunction
eatoncys 7c185c6
Modify annotation of existTooLongFunction
eatoncys 7e84753
Modify annotation of existTooLongFunction
eatoncys c4235dc
Modify annotation of existTooLongFunction
eatoncys 52da6b2
Modify annotation of existTooLongFunction
eatoncys d0c753a
count lines of function without comments
eatoncys d3238e9
Add benchmark with default value
eatoncys d44a2f8
Add benchmark with default value
eatoncys ce544a5
Modified the doc
eatoncys 08f5ddf
Align comments
eatoncys 4fbe5f8
Add test case for isTooLongGeneratedFunction
eatoncys 5c180ac
Add test case for isTooLongGeneratedFunction
eatoncys b83cd1c
Modified test case
eatoncys 6814047
Modified test case
eatoncys 8b32b54
Using withSQLConf
eatoncys 32813b0
Using withSQLConf
eatoncys b879dbf
Modified the default value to Int.Max
eatoncys 44ce894
Modified the default value to 8000/3
eatoncys File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add a test in which we create a query with long generated function, and check if whole-stage codegen is disabled for it.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can be tested by " max function length of wholestagecodegen" added in AggregateBenchmark.scala, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AggregateBenchmark is more like a benchmark than a test. It won't run every time. We need a test to prevent regression brought by future change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viirya, it is hard to check if whole-stage codegen is disabled or not for me, would you like to give me some suggestion, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can check if there is a
WholeStageCodegenExec
node in the physical plan of the query.WholeStageCodegenSuite
has few examples you can take a look.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I'll take a look later. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are multiple ways to verify it. One of the solution is using SQL metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine about your proposed way, but needs to simplify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gatorsmile Do you mean
pipelineTime
metric?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes