Skip to content
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

Registering all Caffeine cache implementations comes with a 14 MB penalty #12961

Open
gsmet opened this issue Oct 26, 2020 · 5 comments
Open
Labels
kind/bug Something isn't working triage/regression

Comments

@gsmet
Copy link
Member

gsmet commented Oct 26, 2020

As soon as we have the Caffeine extension around, we pay a 14 MB native executable penalty since we registered all cache implementations for reflection in #12621 .

I think we need a way to get the cache implementations that will be used given a particular cache configuration.

I don't know how this could work exactly but we definitely cannot pay a 14 MB penalty just for Caffeine (and we usually use only a few of the cache implementations).

/cc @ben-manes @Sanne

@gsmet gsmet added kind/bug Something isn't working triage/regression labels Oct 26, 2020
@gsmet
Copy link
Member Author

gsmet commented Oct 26, 2020

This is a regression in 1.9.0.Final.

@Sanne
Copy link
Member

Sanne commented Oct 26, 2020

I had made a suggestion in the previous issue: - #10420 (comment)

I still believe it's the better approach, and more inline with the Quarkus spirit: analyze the application and transparently pull in only the code paths which are going to be needed.

@ben-manes
Copy link

ben-manes commented Oct 26, 2020

  1. Can the cache configuration be changed via external configuration (e.g. add expiration)? If so, then the compilation might not have those configuration classes afterwards.
  2. Is there a major benefit to not use Caffeine.build during the initialization phase (even if merely discarded)? I wonder whether a pre-touch would be that much better compared to allocating the full instance.

@Sanne
Copy link
Member

Sanne commented Oct 26, 2020

It's always hard to decide if we should give or not give the option to make such adjustements via configuration after the build, and there isn't a universal right answer.

My personal take is that we should allow people to embrace the restrictions of not being able to have certain configuration angles, especially when the benefits can be high.

It might also depend on the intended use of Caffeine: if it's one of the other frameworks, it might be able to optimise by hinting that it definitely isn't going to need certain types of caches - but if the Cache client is end user's / application code this might be harder.
The role of a Quarkus extension is to coordinate the various needs - so for example if one extension needs expiration and the other doesn't, it can generate the right bits on behalf of all needs.

The GraalVM compiler is quite smart in understanding which code paths are "dead code", but it might need a little help here - for example if the classes are loaded via Reflection it will give up, and if they are registered for loading via reflection then it's not allowed to remove any. So that's why I think it might be worthwhile to experiment with a different API to directly load the right Cache implementations via straight-forward explicit code. Sorry I couldn't experiment with this :)

Ironically the very reason for Caffeine to generate such code might be the source of the problem here, and perhaps in GraalVM it's not even beneficial as it should be able to fold those constants and optimise better.

@ben-manes
Copy link

Thanks @Sanne. What about (2) as a hint? I'm not sure if a new API is needed or just invoking the builder is good enough, e.g. exactly how they differ and why that difference is very beneficial.

sophokles73 added a commit to bosch-io/hono that referenced this issue Jan 6, 2021
Upgraded to version 1.10.5. The Quarkus Caffeine extension no longer
registers all cache implementation classes for reflection during native
builds. Thus, the implementation classes required by Hono's adapters had
to be added manually to the reflection-config.json files.
See quarkusio/quarkus#12961

Signed-off-by: Kai Hudalla <kai.hudalla@bosch.io>
sophokles73 added a commit to eclipse-hono/hono that referenced this issue Jan 8, 2021
Upgraded to version 1.10.5. The Quarkus Caffeine extension no longer
registers all cache implementation classes for reflection during native
builds. Thus, the implementation classes required by Hono's adapters had
to be added manually to the reflection-config.json files.
See quarkusio/quarkus#12961

Signed-off-by: Kai Hudalla <kai.hudalla@bosch.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working triage/regression
Projects
None yet
Development

No branches or pull requests

3 participants