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
Simplify DefaultConnectionPool.OpenConcurrencyLimiter.tryLock
#702
Conversation
fed5c85
to
5971c21
Compare
5971c21
to
cca5b0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide some context as to the purpose of this change? It seems like a behavioral change, but the PR title seems to indicate that it's not.
driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java
Show resolved
Hide resolved
cca5b0e
to
09b3f3f
Compare
driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java
Show resolved
Hide resolved
driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java
Outdated
Show resolved
Hide resolved
driver-core/src/main/com/mongodb/internal/connection/DefaultConnectionPool.java
Outdated
Show resolved
Hide resolved
@jyemin Now that you know there is a commit message, is this request still actual? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
When locking for the purpose of using the wait/signal approach, one does not have to account the time spent acquiring the lock towards the timeout. While doing so may reduce the non-deterministic duration by which a timeout is exceeded by any implementation of a timeout, such overtime is usually negligible, unless the lock is highly contended and/or checking the condition before starting to wait is time-consuming. The Java SE example for `java.util.concurrent.locks.Condition.awaitNanos` ignores the time spent acquiring the lock. Doing so potentially reduces responsiveness to timeouts, but does not affect other performance characteristics, so I believe such simplification is worth it and I should have taken this path initially. JAVA-3927
d1d9ad4
to
3bce9e7
Compare
…odb#702) When locking for the purpose of using the wait/signal approach, one does not have to account the time spent acquiring the lock towards the timeout. While doing so may reduce the non-deterministic duration by which a timeout is exceeded by any implementation of a timeout, such overtime is usually negligible, unless the lock is highly contended and/or checking the condition before starting to wait is time-consuming. The Java SE example for `java.util.concurrent.locks.Condition.awaitNanos` ignores the time spent acquiring the lock. Doing so potentially reduces responsiveness to timeouts, but does not affect other performance characteristics, so I believe such simplification is worth it and I should have taken this path initially. JAVA-3927
TODO: rebase on top of
master
after merging #699, then request for review.JAVA-3927