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

[CORE] Minor code cleanups for TreeMemoryConsumer #8254

Merged
merged 11 commits into from
Dec 18, 2024
Merged

[CORE] Minor code cleanups for TreeMemoryConsumer #8254

merged 11 commits into from
Dec 18, 2024

Conversation

zhztheplayer
Copy link
Member

@zhztheplayer zhztheplayer commented Dec 17, 2024 •
edited
Loading

Includes:

  1. Code cleanups for TreeMemoryConsumer
    • Rephrase memory shared mode to legacy mode as shared is in a way misleading (what are shared, shared what, etc.). legacy means the behavior will be aligned with vanilla Spark. The new word is less confusing.
    • Untangle the weak caching code of memory consumer and the factories.
  2. Minor follow-ups for [GLUTEN-8128][VL] Retry borrowing when granted size is less than requested in multi-slot and shared mode #8132

@github-actions github-actions bot added the CORE works for Gluten Core label Dec 17, 2024
@apache apache deleted a comment from github-actions bot Dec 17, 2024
@apache apache deleted a comment from github-actions bot Dec 17, 2024
@zhztheplayer
Copy link
Member Author

@kecookier Would you help review? Thanks!

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

kecookier
kecookier approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@kecookier kecookier left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@kecookier
Copy link
Contributor

@zhztheplayer You may need to rebase f63c29a this commit to fix the CI.

@zhztheplayer
Copy link
Member Author

@zhztheplayer You may need to rebase f63c29a this commit to fix the CI.

Will not proceed until CI passed. Thanks for the review!

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@zhztheplayer zhztheplayer changed the title [VL] Minor code cleanups for TreeMemoryConsumer [CORE] Minor code cleanups for TreeMemoryConsumer Dec 17, 2024
@FelixYBW
Copy link
Contributor

Run Gluten Clickhouse CI on x86

@zhztheplayer zhztheplayer merged commit ef7ccad into apache:main Dec 18, 2024
47 checks passed
@zhztheplayer zhztheplayer mentioned this pull request Dec 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FelixYBW FelixYBW FelixYBW approved these changes

@kecookier kecookier kecookier approved these changes

Assignees
No one assigned
Labels
CORE works for Gluten Core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhztheplayer @kecookier @FelixYBW