FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-601: [Gandiva] Synchronize some methods on the Projector by lriggs · Pull Request #602 · apache/arrow-java · GitHub

/ arrow-java Public

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

Merged
lidavidm merged 1 commit into
apache:mainfrom
lriggs:projectorSynch
Feb 21, 2025
ConversationCommits (1)ChecksFiles changed
Merged

GH-601: [Gandiva] Synchronize some methods on the Projector#602
lidavidm merged 1 commit into
apache:mainfrom
lriggs:projectorSynch

Conversation

lriggs commented Feb 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 changed the title GH-601 Synchronize some methods on the Projector. GH-601 Synchronize some methods on the Projector Feb 12, 2025

lidavidm commented Feb 12, 2025

Copy link
Copy Markdown
Member

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

kou changed the title GH-601 Synchronize some methods on the Projector GH-601: Synchronize some methods on the Projector Feb 12, 2025
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 commented Feb 12, 2025

Copy link
Copy Markdown
Member

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

lidavidm commented Feb 12, 2025

Copy link
Copy Markdown
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 commented Feb 20, 2025

Copy link
Copy Markdown
Member

Could you rebase on main?

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

This comment has been minimized.

Sign in to view

lriggs commented Feb 20, 2025

Copy link
Copy Markdown
Contributor Author

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

kou added the bug-fix PRs that fix a big. label Feb 20, 2025
lidavidm merged commit 370031e into apache:main Feb 21, 2025
lidavidm added this to the 18.3.0 milestone Feb 21, 2025
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lidavidm lidavidm approved these changes
laurentgo Awaiting requested review from laurentgo laurentgo is a code owner
wgtmac Awaiting requested review from wgtmac wgtmac is a code owner
kou Awaiting requested review from kou

Assignees

No one assigned

Labels

bug-fix PRs that fix a big.

Projects

None yet

Milestone

18.3.0

Development

Successfully merging this pull request may close these issues.

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

3 participants

Back | FazBrowse Home | New Git URL