Caffeine error at runtime when using native build

[aimanph23]

Describe the bug

"causedBy": {
--
  | "exception": {
  | "refId": 5,
  | "exceptionType": "java.lang.ClassNotFoundException",
  | "message": "com.github.benmanes.caffeine.cache.SSMSW",
  | "frames": [
  | {
  | "class": "com.oracle.svm.core.hub.ClassForNameSupport",
  | "method": "forName",
  | "line": 60
  | },
  | {
  | "class": "java.lang.ClassLoader",
  | "method": "loadClass",
  | "line": 160
  | },
  | {
  | "class": "com.github.benmanes.caffeine.cache.LocalCacheFactory",
  | "method": "newBoundedLocalCache",
  | "line": 95
  | },

Expected behavior
Should be working in Native build

Actual behavior
Only works on JVM build and local dev.
Build native is successful, but at runtime it triggers an error

To Reproduce
Steps to reproduce the behavior:

1. Error when a code is hit with caffeine implementation at runtime
2. No problem with JVM build and local dev runtime

Environment (please complete the following information):
Quarkus 1.5.2.Final
GraalVM 20.1.0

[geoand]

cc @gwenneg

[geoand]

Can you give some more information on how to reproduce the problem?

[aimanph23]

Hi @geoand,

The problem was fix just a while ago by using a configuration file to register class for reflection since this is a 3rd party jar (caffeine-2.8.4.jar) and we cannot use @RegisterForReflection

Add resources\reflection-config.json

[
  {
    "name" : "com.github.benmanes.caffeine.cache.SSMSW",
    "allDeclaredConstructors" : true,
    "allPublicConstructors" : true,
    "allDeclaredMethods" : true,
    "allPublicMethods" : true,
    "allDeclaredFields" : true,
    "allPublicFields" : true
  }
]

And in resources\application.properties

quarkus.native.additional-build-args =-H:ReflectionConfigurationFiles=reflection-config.json

[geoand]

That's good to know.

Just one point: You can use @RegisterForReflection to register a third party class for reclection, see the targets method of the annotarion.

@gwenneg I assume we should be registering this class for reflection in the cache extension?

[aimanph23]

Thanks @geoand for that option

Btw these are the classes that was imported

import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;

[gwenneg]

@geoand: com.github.benmanes.caffeine.cache.SSMSW should probably be registered for reflection in CaffeineProcessor.

[ben-manes]

A similar issue was raised in ben-manes/caffeine#434. I am not familiar with GraalVM, so my question is naive:

Should libraries provide a manifest of for reflection, e.g. reflection-config.json, and would that be automatically picked up by Graal? If so, we could code generate this resource file in addition to the classes. Of course that would also mean including all of the generated classes, rather than the minimal subset an application is using. We use reflection because explicit switch/new was expensive to class load.

[geoand]

@ben-manes the question is very good! I can answer it, but I am sure @Sanne has a lot more points to add, so I'll pass it on to him :)

[Sanne]

Hi @ben-manes !

That is certainly an option, but I would recommend to not include such reflection instructions directly within Caffeine - store them in a separate artifact at least, so that they are opt-in as several users would prefer to control their own options, and these are purely "additive" today - which implies that if you include such directives directly in the Caffeine jar users would be unable to remove the ones they don't want to have.

As an example of how you could do such a thing, for Hibernate ORM I've created a separate jar - this is meant to help any other user of Hibernate ORM + GraalVM:

• https://github.com/hibernate/hibernate-orm/tree/master/hibernate-graalvm

One important thing to bear in mind: when you register a type X for reflection, then the GraalVM compiler is forced to consider this type as "included", even though the application is actually never using this thing (the compiler will not be able to proof this type as unused and will have to assume that there's possibilities for this code to be needed).

That is a sub-optimal choice, especially if there are multiple implementations for the same core contract: as you well know from JIT optimisations, having a megamorphic call sites will result in less efficient code - for a combination of many reasons. Since there is no JIT in the native images, and since reflection is confusing the compiler, you risk paying the cost associated with a megamorphic call all the times.
Code size is also impacted of course, although in the case of Caffeine I suppose there is only minimal risk of some specific caching algorithm to "drag in" a significant dependency tree. But then also bear in mind that additional code could have an impact on the efficiency of the code actually being used, so code size amplifies the efficiency problem.

In general in Quarkus we try to do much better than the static list that I have in hibernate-graalvm by leveraging the build-time analysis: we can infer what is going to be used by looking at the code which is being built, at its configuration, its annotations, or even possibly its bytecode, and decide based on that input how to "guide" the GraalVM native-image compilation step; for example automatically generating the minimal set of Reflections to enable.

That's what a Quarkus extension is for: to help making such optimisations by plugging into the analysis phase and outputting the optimisations that a specific library is needing.

Having said that, simply enabling registration of "SSMSW" to be constructed reflectively for all applications is a viable solution, although not the ideal solution. We could go for this as a stop-gap solution, but in the longer term I'd like to see either:

• either Caffeine won't need reflection for this, so that the GraalVM compiler can do its magic automatically
• or we identify a way to have the Quarkus extension automatically infer which components of Caffeine are being used

Do you think either of these could be feasible?

N.B. the Hibernate ORM extension combines some highly optimised choices with a "static list" of reflective restrations: as in some cases it's not worth it, or on a long term wishlist, and we're being pragmatic about it.

[ben-manes]

Thank you for the thoughtful explanation! The example and why it should optional, if even added, is very helpful.

We currently generate ~400 classes to minimize the per-configuration memory overhead by reducing the number of cache / entry fields. This dropped from ~700 classes in 2.6.1, which was ~750kb of compressed bytecode bloat (ben-manes/caffeine#110). Traditional server-side apps don't care about larger jars with unloaded classes and that still seems preferable compared to having dozens of unused fields on a map entry, which wastes memory for large caches.

either Caffeine won't need reflection for this, so that the GraalVM compiler can do its magic automatically or we identify a way to have the Quarkus extension automatically infer which components of Caffeine are being used

Originally we generated a string switch statement whose block returned the specialized type. The factory classes were oddly slow to class load due to bytecode verification. Possibly moving to a numeric offset to a Class[] array might have helped, but switching to reflection was the most straightforward way to cut the fat and prior to AOT becoming popular.

The generated classes are a private implementation detail, so a risk is that we'll generate a new naming pattern. For example an ask is to have pluggable key/value equivalence (Infinispan wanted this for byte[] keys, others for weak key interners). Casual development of this feature hints that it will replace entry variants for soft/weak references with factory parameters, which might reduce the number of entry classes with a few additional cache-level fields. That would be a semver minor release (new feature), but you might consider a major release due to breaking your static class list.

I'm not sure what the optimal strategy is:

1. Is codegen of unloaded classes still preferable to heavy weight objects? (I believe so)
2. Would reverting back to a static class listing defeat Graal's optimization, since the selection is still a runtime choice? (I believe so)
3. Does changing the names of internal classes cause downstream pain for Graal users? (I believe so)

For now I've been happy enough ignoring the problem and not burning cycles on it, given this is a weekend hobby project after all. If we can devise something that works better then I'd be happy to make some changes to improve developer QOL.

[Sanne]

Interesting puzzle, I'm not sure what to recommend as this is a new context; we'll probably need to do some experiments.

[ @galderz since you're now a GraalVM expert with both Infinispan and Caffeine experience, this might be of interest to you too? ]

I'll try to answer your points but I'm not sure, I hope to be able to do some experiments this summer to give you a better answer.

Is codegen of unloaded classes still preferable to heavy weight objects? (I believe so)

I believe you're right for the regular JIT, as the memory layout isn't optimised at runtime. Interestingly, the dead code elimination and constant folding of GraalVM in native image might be able to do a better job, as I believe it doesn't have to respect the same memory layout of the regular JVM, e.g. it could remove unused/constant fields.

Would reverting back to a static class listing defeat Graal's optimization, since the selection is still a runtime choice?

This could actually work, but only if the function mapping from the user's code to the actual implementation code is simple enough to analyse for the compiler to not have to give up. It would also need to be able to infer that the input of such a choice is a constant (or a subset of all possible options). I'm not sure what could qualify for being "simple enough" though, this is where I think it would be cool to do some experiments.

Does changing the names of internal classes cause downstream pain for Graal users?

It would if we had to rely on reflection, or obscure rules mapping to reflection. In the case of Quarkus this is ok though: we had to accept that such rules and optimisations can break even just because of internal changes: such compiler optimisations require coupling to internals so we have to accept that any micro-upgrade of a component might need to revisit such constants.

So if you have to change the naming structure occasionally that's not worrying me - we'll just adapt. Of course it would be more worrying if your classes were random-generated at each release, I hope that's not the plan :)

suggestion :
what if Caffeine had an API we could invoke to "warmup" the kinds of caches that the application is going to need we make available?

Essentially a framework like Quarkus could benefit from being able to invoke something similar to the LocalCacheFactory, but which allows us to split the cache initialization in two phases, like in pseudo code:

 //to run during static initialization of a Quarkus application
 CacheFactory cf = LocalCacheCodePreparer.forconfigurations( Set<Caffeine> builders );

 //to run at runtime:
 LocalCache actualCache = cf.newBoundedLocalCache( Caffeine, cacheloader, async );

And this last method would throw an exception if and when the passed parameters don't match any of the configurations that have been passed to the first method.

N.B. we can transfer the cf instance with all the state it contains from what is generated in a static initialization block which we trigger during the build of the application into the actual produced binary. So if the CacheFactory had a map with methodhandles pointing to the constructors of each generated class it needs to be able to construct, the compiler would be able to convert such state.

I realize this is very rough; in particular I haven't thought about how the API should look like to use such a CacheFactory - would be great if this could be passed into the Caffeine instance so to set an alternative internal cache-builder strategy.

[ben-manes]

what if Caffeine had an API we could invoke to "warmup" the kinds of caches that the application is going to need we make available?

That's an interesting idea and seems like the most viable approach, as we won't rely on compiler magic working (and then breaking due to a seemingly innocent change). We'd need to iterate on the API to deemphasize so as to minimize conceptual weight for normal users who shouldn't need to learn about it. A top-level class sounds important, so I'd be curious if we could sneak it into something else.

Your idea has characteristics of CaffeineSpec which lacks configuring instance types, like a RemovalListener, that mostly don't matter for our specialized constructions. That's for simple parsing of an externalized configuration and used as Caffeine.from(spec)...build(loader), but custom formats (hocon, yaml) might just rebuild this utility as they see fit. Assuming they didn't, we could have it preload the class so that AOT's PGO observed it for runtime usage, as you said.

CaffeineSpec as-is might be too limiting, requiring that we either evolve it or add some pre-init method to the Caffeine builder to offer a more flexible configuration. However that does beg the question of whether modifying an externalized cache configuration could cause the app to fail because it didn't observe the right classes during static initialization? In your cache options, that might be when switching from expireAfterAccess to expireAfterWrite due to a misunderstanding of those settings without rebuilding.

[bnazare]

@geoand: com.github.benmanes.caffeine.cache.SSMSW should probably be registered for reflection in CaffeineProcessor.

That would probably work but only for @aimanph23's specific scenario. I'm facing a similar issue but using a simpler cache configuration. In my case I had to add two different classes to "reflect-config.json", like so:

{
  "name":"com.github.benmanes.caffeine.cache.SSW",
  "allDeclaredConstructors":true
},
{
  "name":"com.github.benmanes.caffeine.cache.PSW",
  "allDeclaredConstructors":true
}

Not only did I have to configure reflection by hand but I had to figure out the actual classes by trial and error. Both are things that I was not expecting to have to do when using an official Quarkus extension.

Btw, here's my cache configuration in case it's useful:

quarkus.cache.caffeine."my-cache".initial-capacity=100
quarkus.cache.caffeine."my-cache".expire-after-write=10M