-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
[Question] Can mutex() be simplified this way? #3381
Comments
It avoids an additional volatile read of the field once it's determined to
be non-null.
…On Fri, Feb 8, 2019, 12:48 PM qiutongs ***@***.*** wrote:
https://github.com/google/guava/blob/838560034dfaa1afdf51a126afe6b8b8e6cce3dd/guava/src/com/google/common/util/concurrent/RateLimiter.java#L189-L200
Basically, my confusion is why the local variable "mutex" is needed.
private Object mutex() {
if (mutexDoNotUseDirectly == null) {
synchronized (this) {
if (mutexDoNotUseDirectly == null) {
mutexDoNotUseDirectly = new Object();
}
}
}
return mutexDoNotUseDirectly;
}
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3381>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEVOGLttPeLzBA5hAxCUvvnxpB6dPks5vLbiGgaJpZM4awy17>
.
|
@JakeWharton So the original code reads "mutexDoNotUseDirectly" on lines 190, 193, 195 (3 times in worst case). But my code reads "mutexDoNotUseDirectly" four times in worst case. Is that the thing we try to optimize with the local variable? |
Yes. Also the common case becomes 1 not 2.
…On Fri, Feb 8, 2019, 1:05 PM qiutongs ***@***.*** wrote:
@JakeWharton <https://github.com/JakeWharton>
Thank you for quick reply! Let me make sure I understand you correctly.
The additional volatile read you meant is the one in the "return" statement
in my code?
So the original code reads "mutexDoNotUseDirectly" on lines 190, 193, 195
(3 times in worst case). But my code reads "mutexDoNotUseDirectly" four
times in worst case.
Is that the thing we try to optimize with the local variable?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3381 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEbcKI9WXIDIZ6p5sBaC8pBD06m_tks5vLbyFgaJpZM4awy17>
.
|
It makes sense. Thank you! |
guava/guava/src/com/google/common/util/concurrent/RateLimiter.java
Lines 189 to 200 in 8385600
Basically, my confusion is why the local variable "mutex" is needed.
The text was updated successfully, but these errors were encountered: