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

Remove holding Poh lock #8838

Merged
merged 1 commit into from
Mar 13, 2020
Merged

Conversation

carllin
Copy link
Contributor

@carllin carllin commented Mar 13, 2020

Problem

Poh lock is held during expensive operations by statements of the type if let Some(_) = poh.lock().unwrap()...

Summary of Changes

Rework

Fixes #

@carllin carllin added the v1.0 label Mar 13, 2020
@carllin carllin requested a review from sakridge March 13, 2020 20:28
@carllin carllin added the automerge Merge this Pull Request automatically once CI passes label Mar 13, 2020
Copy link
Member

@sakridge sakridge left a comment

Choose a reason for hiding this comment

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

Hmm. I wouldn't expect it to be held, but good find if so..

@codecov
Copy link

codecov bot commented Mar 13, 2020

Codecov Report

Merging #8838 into master will decrease coverage by <.1%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #8838     +/-   ##
========================================
- Coverage      80%     80%   -0.1%     
========================================
  Files         265     265             
  Lines       57546   57549      +3     
========================================
+ Hits        46072   46073      +1     
- Misses      11474   11476      +2

@solana-grimes solana-grimes merged commit 53b8d0d into solana-labs:master Mar 13, 2020
mergify bot pushed a commit that referenced this pull request Mar 13, 2020
automerge

(cherry picked from commit 53b8d0d)
@carllin
Copy link
Contributor Author

carllin commented Mar 13, 2020

@sakridge hehe yeah, the lock doesn't go out of scope until the end of the block. For example, in this example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=eb85cd168d706fb955dd40701c79c0d8, main never returns

@carllin
Copy link
Contributor Author

carllin commented Mar 13, 2020

@sakridge I wonder if we have this pattern anywhere else in the codebase xD, with RwLock's as well

@mvines
Copy link
Member

mvines commented Mar 13, 2020

Same, that's super unexpected. Maybe a compiler bug even?

@carllin
Copy link
Contributor Author

carllin commented Mar 13, 2020

@carllin
Copy link
Contributor Author

carllin commented Mar 13, 2020

solana-grimes pushed a commit that referenced this pull request Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants