Migrate off Unsafe (guava-android)

This is the next step now that we've migrated guava-jre off (#6806).

Some recent notes on Android: #6806 (comment)

Part of the work here is to have a fallback at all for a case or two that is missing one.

Then, ideally, we'd avoid even trying to use Unsafe in cases where it would produce a warning. But we don't want to regress performance on Android or ideally even the JVM.

One other small note: Currently, the Android flavor has no module-info. Today, if we were to give it one, we should really have a "full" requires line for jdk.unsupported. After this issue, though, we could mirror the JRE variant's static line.

I posted some thoughts in https://issuetracker.google.com/issues/134377851#comment3.

Rename fields to make them harder to use by accident.

Make `AbstractFutureState` fields package-private.

Introduce an `AtomicHelper` test simple enough to run under Android.

Make the `guava-android` copy of `AbstractFuture` try `VarHandle` whe…

I got a start on this in 33.4.7: Even guava-android will now try VarHandle (only on the JVM, of course) before Unsafe.

There is still work to do, both to avoid generating warnings from other classes and to make Striped64 work at all when Unsafe becomes unavailable.

Then 33.4.8 changed guava-android to avoid VarHandle entirely again and to try:

on the JVM: AtomicReferenceFieldUpdater before Unsafe. On most JVMs (though perhaps not, say, Java 8?), I'm hoping that AtomicReferenceFieldUpdater performs identically to Unsafe (as we've seen in our benchmarks), though VarHandle looks slightly faster than both. (So 33.4.8 has no performance regression relative to 33.4.6, though a tiny regression relative to 33.4.7 because of the tiny improvement of that release relative to 33.4.6.)
on Android: Unsafe before AtomicReferenceFieldUpdater. With the very limited Android testing I did (in internal bug 392974826), I found that Unsafe was faster than AtomicReferenceFieldUpdater. (So Android performance should be unchanged across versions except for any difference in class-initialization time as a result of the different approaches we've taken to deciding which implementations to try.)

I still need to eventually do the things from my previous post.

There is also Android's libcore.io.Memory. It's an internal API, but I don't know how different that makes it than Unsafe. We could investigate using it, as I believe Protobuf does. That might let us remove our usage of Unsafe. (I'd need to refresh my memory on whether we'd need to also first drop support for Java 8 someday.) I'd have to look into how to use it for poking at arbitrary fields of objects.

A possible advantage to that is that Unsafe prevents us from using --release. Granted, we also can't use --release because we conditionally use APIs from newer versions of Java. But we could start using it if we were to create a proper multi-release jar. Granted, that might be true even today (except that we still wouldn't use --release 8 specifically). I'd probably want to double-check with some Android folks, though, just to make sure that a multi-release jar is still unlikely to cause problems there.

I'd have to look into how to use [libcore.io.Memory] for poking at arbitrary fields of objects.

Hmm, Protobuf might still be using Unsafe for objectFieldOffset.

My impression is that Protobuf might be using Memory because, probably nearly a decade ago now, it had to support old versions of Android that didn't offer the Unsafe methods that it needed (while presumably still offering objectFieldOffset). But time has passed, and Guava might not need the same methods as Protobuf did. (After all, Guava has been using Unsafe on Android for years—though I suppose we have had a fallback in place....)

I think that Protobuf may also have had reasons that even AtomicReferenceFieldUpdater wasn't a great option for them. It works fine for us. Well, I mean, it has Android performance concerns, including for us. But I think that Android had additional concerns around code size.

Upgrade guava version to 33.4.8 (#12219)

tl;dr At this point, we might choose only to add some runtime checks to avoid even trying Unsafe so as to avoid what I'm hoping is the only remaining issue, some warnings (never runtime errors).

(Anyone, please correct me if you know otherwise.)

I'm confused about what I said about Striped64 above: Striped64 is used only from LongAdder, which is in turn used only from LongAddables, which has a fallback. (That's true in both cache and hash.) But then what was the failure I referred earlier this year? I might have been confused by a still-existing failure in LongAdderTest because modern Java attached a cause to the NoClassDefFoundError—a feature that I am very happy about, did not expect to ever be confused by, and remain happy about even if it turns out that I was confused :)—with the cause coming from the initial class-initialization attempt inside AbstractCache (which may have led me to believe that AbstractCache was broken). [edit: Yes, I found an old internal post of mine, and it looks very likely that it was that.] Or I may have been confused by an error I'm seeing from from automated testing that examines all classes in the package. Or maybe I got mixed up with a non-Striped64-related error from one of the packages.

I had also expressed worry about the fallback logic in LittleEndianByteArray. But I'm not sure why: The kind of problem I've been worried about is google/truth#333 (comment). But I'm now thinking this is different: The Truth problem was about when a class is missing; the Unsafe problem (at least for now!) is about a case in which an exception is thrown during static initialization. Clearly static initialization cannot be performed "early" the way that class loading can.

So, based on that and on what I'm seeing in current runs of Guava's own tests with --sun-misc-unsafe-memory-access=deny: I suspect that guava-android has appropriate fallback logic for all its Unsafe usages. The remaining work, then, may be "just" the other half of what I said earlier in this thread, the part about avoiding generating even warnings. If we wanted to enough, we could also investigate inserting fast paths that do not use Unsafe (typically by using AtomicReferenceFieldUpdater or similar—not VarHandle) so as to improve performance for Guava users who use guava-android under JVMs where Unsafe is unavailable and who care about performance in these cases. But I wouldn't want to make those paths the universal default, since I wouldn't want to hurt performance under Android. Given that, we'd need the same kind of runtime version check that we'd need to avoid warnings in the first place, plus we'd need the additional implementation. So clearly the runtime check is the priority.

Upgrade guava version to 33.4.8 (grpc#12219)

Navigation Menu

Repository navigation

Migrate off Unsafe (guava-android) #7742

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Migrate off Unsafe (guava-android) #7742

Description

Activity

cpovirk commented on Mar 27, 2025

cpovirk commented on Apr 8, 2025

cpovirk commented on Apr 22, 2025

cpovirk commented on Aug 5, 2025

cpovirk commented on Aug 5, 2025

cpovirk commented on Sep 12, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions