Make AggregateFutureState fields package-private.
#7766
Merged
Conversation
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
![copybara-service[bot]](https://avatars.githubusercontent.com/in/44061?s=60&v=4){kind=small}
This ensures that they are accessible to methods like `AtomicIntegerFieldUpdater.newUpdater` no matter which `util.concurrent` class those methods are called from. Previously, we were trying to arrange for those methods to be called from `AggregateFutureState` itself—specifically, through methods like `remainingCountUpdaterFromWithinAggregateFutureState`. However, we're finding that optimizers sometimes inline those methods into callers in other classes, leading to the warning "SafeAtomicHelper is broken!" Rather than try to prevent inlining with `-dontinline` directives, we instead give in and expand the fields' visibility. We already did this for `AbstractFutureState` in cl/742334547, albeit for somewhat different reasons, so we can follow the same playbook here: Rename the fields to make them harder to use by accident (as in cl/741607075), and update our Proguard config for the rename (as in cl/742724817). At that point, we might as well inline methods like `remainingCountUpdaterFromWithinAggregateFutureState` ourselves just to simplify the code, so I've done so. (Note that even `private` fields "should" be accessible to nested classes, thanks to nestmates. However, that's [not the case with `-source 8 -target 8`](https://github.com/google/guava/blob/a429676a3bb68ac9b29cea0f15ae65065bdf5a44/guava/src/com/google/common/util/concurrent/AbstractFutureState.java#L397-L402), and apparently it's not the case for Android, as well, even without `-source 8 -target 8`.) RELNOTES=`util.concurrent`: Modified our fast paths to ensure that they continue to work when run through optimizers, such as those commonly used by Android apps. This fixes problems that some users may have seen since [Guava 33.4.5](https://github.com/google/guava/releases/tag/v33.4.5). (b8dcaed) PiperOrigin-RevId: 744855670
copybara-service
bot
force-pushed
the
test_744697976
branch
from
d99aa16 to
f9eff73
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make
AggregateFutureStatefields package-private.This ensures that they are accessible to methods like
AtomicIntegerFieldUpdater.newUpdaterno matter whichutil.concurrentclass those methods are called from.Previously, we were trying to arrange for those methods to be called from
AggregateFutureStateitself—specifically, through methods likeremainingCountUpdaterFromWithinAggregateFutureState. However, we're finding that optimizers sometimes inline those methods into callers in other classes, leading to the warning "SafeAtomicHelper is broken!"Rather than try to prevent inlining with
-dontinlinedirectives, we instead give in and expand the fields' visibility. We already did this forAbstractFutureStatein cl/742334547, albeit for somewhat different reasons, so we can follow the same playbook here: Rename the fields to make them harder to use by accident (as in cl/741607075), and update our Proguard config for the rename (as in cl/742724817). At that point, we might as well inline methods likeremainingCountUpdaterFromWithinAggregateFutureStateourselves just to simplify the code, so I've done so.(Note that even
privatefields "should" be accessible to nested classes, thanks to nestmates. However, that's not the case with-source 8 -target 8, and apparently it's not the case for Android, as well, even without-source 8 -target 8.)RELNOTES=
util.concurrent: Modified our fast paths to ensure that they continue to work when run through optimizers, such as those commonly used by Android apps. This fixes problems that some users may have seen since Guava 33.4.5. (b8dcaed)