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.

Sign up for GitHub

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

Jump to bottom

[BugFix] concurrently rewrite jit after prepare/open original expr in filter #41541

Merged
merged 10 commits into from
Feb 27, 2024
Merged

[BugFix] concurrently rewrite jit after prepare/open original expr in filter #41541

merged 10 commits into from
Feb 27, 2024

Conversation

fzhedu
Copy link
Contributor

@fzhedu fzhedu commented Feb 23, 2024 •
edited
Loading

Why I'm doing:

What I'm doing:

Fixes https://github.com/StarRocks/StarRocksTest/issues/6261

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.2
    • 3.1
    • 3.0
    • 2.5

@fzhedu fzhedu requested a review from a team as a code owner February 23, 2024 07:05
@mergify mergify bot assigned fzhedu Feb 23, 2024
stdpain
stdpain previously approved these changes Feb 27, 2024
View reviewed changes
@fzhedu fzhedu changed the title [BugFix] jit can't repreapre/reopen some exprs,like UDF, dict [BugFix] rewrite jit after prepare/open original expr in filter Feb 27, 2024
@fzhedu
[BugFix] jit can't repreapre/reopen some exprs,like UDF, dict
34ee4f4 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
add tests
0eed46a 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
revert some invalid codes
879366a 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
format
6c01fd7 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
format
bb179b6 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
fix clang-tidy
4103070 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
clean the code
2fe1e4b 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@github-actions GitHub Actions
Copy link

[BE Incremental Coverage Report]

❌ fail : 2 / 47 (04.26%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 be/src/exec/pipeline/scan/olap_scan_context.cpp 0 15 00.00% [23, 24, 25, 27, 28, 29, 30, 31, 34, 36, 37, 39, 151, 164, 165]
🔵 be/src/exec/pipeline/scan/olap_scan_context.h 0 14 00.00% [47, 54, 56, 57, 58, 59, 61, 62, 63, 64, 66, 68, 90, 168]
🔵 be/src/exprs/expr.cpp 0 5 00.00% [229, 230, 231, 233, 234]
🔵 be/src/exprs/expr_context.cpp 0 11 00.00% [205, 206, 207, 209, 210, 211, 212, 214, 216, 217, 219]
🔵 be/src/exprs/jit/jit_expr.cpp 2 2 100.00% []

@fzhedu fzhedu changed the title [BugFix] rewrite jit after prepare/open original expr in filter [BugFix] concurrently rewrite jit after prepare/open original expr in filter Feb 27, 2024
stdpain
stdpain previously approved these changes Feb 27, 2024
View reviewed changes
stdpain
stdpain reviewed Feb 27, 2024
View reviewed changes
be/src/exec/pipeline/scan/olap_scan_context.h

private:
// TODO: use c++20 barrier after upgrading gcc
class Barrier {
Copy link
Contributor

Choose a reason for hiding this comment

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

std::call_once is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really, if there are more than one expr, we can concurrently jit them, as jit may take tens of ms time.

@fzhedu
update tests
690bc4d 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
update tests
e669695 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu
rename
9f70cef 
Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
@fzhedu fzhedu enabled auto-merge (squash) February 27, 2024 13:00
@github-actions GitHub Actions
Copy link

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@kangkaisen kangkaisen disabled auto-merge February 27, 2024 13:58
@kangkaisen kangkaisen merged commit c13aa1b into StarRocks:main Feb 27, 2024
45 of 46 checks passed
Seaven pushed a commit to Seaven/starrocks that referenced this pull request Feb 28, 2024
@fzhedu @Seaven
[BugFix] concurrently rewrite jit after prepare/open original expr in…
b26cc05 
… filter (StarRocks#41541)

Signed-off-by: Zhuhe Fang <fzhedu@gmail.com>
Signed-off-by: Seaven <seaven_7@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silverbullet233 silverbullet233 silverbullet233 approved these changes

@stdpain stdpain stdpain approved these changes

Assignees

@fzhedu fzhedu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fzhedu @silverbullet233 @stdpain @kangkaisen