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

GH-601: [Gandiva] Synchronize some methods on the Projector #602

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

lriggs
Copy link
Contributor

@lriggs lriggs commented Feb 12, 2025

Rationale for this change

Multiple threads can attempt to create the same llvm expression in Gandiva. This isn't allowed with the new JIT compiler, so synchronizing will prevent this scenario.

What changes are included in this PR?

Synchronize some methods to avoid adding duplicate llvm expressions.

Are these changes tested?

Yes, through unit tests in Gandiva.

Are there any user-facing changes?

No.

Closes GH-601

@kou kou changed the title GH-601 Synchronize some methods on the Projector. GH-601 Synchronize some methods on the Projector Feb 12, 2025
@lidavidm
Copy link
Member

@lriggs since you've been working on Gandiva lately, would you be able to help take a look at the changes here?

@kou kou changed the title GH-601 Synchronize some methods on the Projector GH-601: Synchronize some methods on the Projector Feb 12, 2025
@kou kou changed the title GH-601: Synchronize some methods on the Projector GH-601: [Gandiva] Synchronize some methods on the Projector Feb 12, 2025
@kou
Copy link
Member

kou commented Feb 12, 2025

We should rebase on main after we merge GH-595.

@lidavidm
Copy link
Member

@lriggs since you've been working on Gandiva lately, would you be able to help take a look at the changes here?

Ok I'm clearly not paying attention...you are lriggs 😬

As Kou states, let's rebase this after the other PR.

@kou
Copy link
Member

kou commented Feb 20, 2025

Could you rebase on main?

@lriggs lriggs requested a review from kou as a code owner February 20, 2025 18:06

This comment has been minimized.

@lriggs
Copy link
Contributor Author

lriggs commented Feb 20, 2025

I rebased but don't seem to have permissions to add the bug-fix label.

@kou kou added the bug-fix PRs that fix a big. label Feb 20, 2025
@lidavidm lidavidm merged commit 370031e into apache:main Feb 21, 2025
26 of 28 checks passed
@lidavidm lidavidm added this to the 18.3.0 milestone Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix PRs that fix a big.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Java][Gandiva] Synchronize some methods on Projector.
3 participants