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

Move to a pool of threadsafecontexts #44605

Merged
merged 2 commits into from
Apr 10, 2022
Merged

Move to a pool of threadsafecontexts #44605

merged 2 commits into from
Apr 10, 2022

Conversation

pchintalapudi
Copy link
Member

Moving to a pool of contexts gets us closer to running compilation on multiple threads once we can get rid of the codegen lock. However, this series of PRs is starting to veer into territory of potential locking issues, so review with an eye for those errors would be appreciated.
I don't believe there are locking issues here because although we have differences in the order in which we unlock locks (due to RAII and explicit unlock calls coexisting), we don't really call any functions between our unlock calls which could reacquire an out-of-order lock.

Depends on #44600 to remove imaging_mode.

@pchintalapudi pchintalapudi changed the base branch from master to pc/imaging-mode March 14, 2022 04:47
@pchintalapudi pchintalapudi force-pushed the pc/imaging-mode branch 3 times, most recently from bfbee62 to a91ff11 Compare April 4, 2022 19:24
@pchintalapudi pchintalapudi changed the base branch from pc/imaging-mode to master April 4, 2022 23:46
@pchintalapudi pchintalapudi force-pushed the pc/context-pool branch 2 times, most recently from 5de4749 to c57ca9f Compare April 6, 2022 23:25
@pchintalapudi pchintalapudi marked this pull request as ready for review April 7, 2022 18:11
@pchintalapudi pchintalapudi requested a review from vtjnash April 7, 2022 18:11
@pchintalapudi pchintalapudi added compiler:codegen Generation of LLVM IR and native code compiler:llvm For issues that relate to LLVM labels Apr 7, 2022
@@ -1165,10 +1176,6 @@ void JuliaOJIT::RegisterJITEventListener(JITEventListener *L)
}
#endif

orc::ThreadSafeContext &JuliaOJIT::getContext() {
Copy link
Member

Choose a reason for hiding this comment

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

🎉

@vtjnash
Copy link
Member

vtjnash commented Apr 7, 2022

differences in the order in which we unlock locks

unlock order does not matter, only which locks you are already holding when attempting to acquire a new lock

@pchintalapudi pchintalapudi added merge me PR is reviewed. Merge when all tests are passing and removed merge me PR is reviewed. Merge when all tests are passing labels Apr 7, 2022
@pchintalapudi pchintalapudi added the merge me PR is reviewed. Merge when all tests are passing label Apr 8, 2022
@pchintalapudi pchintalapudi merged commit 992b261 into master Apr 10, 2022
@pchintalapudi pchintalapudi deleted the pc/context-pool branch April 10, 2022 18:47
@pchintalapudi pchintalapudi removed the merge me PR is reviewed. Merge when all tests are passing label Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code compiler:llvm For issues that relate to LLVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants