Skip to content

Make GeneratorStrategy.generate unreachable on native #29521

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

Conversation

sdeleuze
Copy link
Contributor

@sdeleuze sdeleuze commented Nov 18, 2022

Allows to save around 0.5M of RSS memory by avoiding to make those classes reachable:

org.springframework.cglib.core.Block
org.springframework.cglib.core.ClassEmitter
org.springframework.cglib.core.ClassEmitter$1
org.springframework.cglib.core.ClassEmitter$2
org.springframework.cglib.core.ClassEmitter$3
org.springframework.cglib.core.ClassEmitter$FieldInfo
org.springframework.cglib.core.ClassInfo
org.springframework.cglib.core.ClassNameReader
org.springframework.cglib.core.ClassNameReader$1
org.springframework.cglib.core.ClassTransformer
org.springframework.cglib.core.CodeEmitter
org.springframework.cglib.core.CodeEmitter$State
org.springframework.cglib.core.DebuggingClassWriter
org.springframework.cglib.core.DefaultGeneratorStrategy
org.springframework.cglib.core.DuplicatesPredicate
org.springframework.cglib.core.EmitUtils
org.springframework.cglib.core.EmitUtils$1
org.springframework.cglib.core.EmitUtils$10
org.springframework.cglib.core.EmitUtils$11
org.springframework.cglib.core.EmitUtils$12
org.springframework.cglib.core.EmitUtils$13
org.springframework.cglib.core.EmitUtils$14
org.springframework.cglib.core.EmitUtils$15
org.springframework.cglib.core.EmitUtils$16
org.springframework.cglib.core.EmitUtils$2
org.springframework.cglib.core.EmitUtils$3
org.springframework.cglib.core.EmitUtils$4
org.springframework.cglib.core.EmitUtils$5
org.springframework.cglib.core.EmitUtils$6
org.springframework.cglib.core.EmitUtils$ArrayDelimiters
org.springframework.cglib.core.Local
org.springframework.cglib.core.LocalVariablesSorter
org.springframework.cglib.core.LocalVariablesSorter$State
org.springframework.cglib.core.MethodInfo
org.springframework.cglib.core.MethodInfoTransformer
org.springframework.cglib.core.MethodWrapper
org.springframework.cglib.core.MethodWrapper$MethodWrapperKey
org.springframework.cglib.core.ReflectUtils$1
org.springframework.cglib.core.ReflectUtils$2
org.springframework.cglib.core.ReflectUtils$3
org.springframework.cglib.core.RejectModifierPredicate
org.springframework.cglib.core.VisibilityPredicate
org.springframework.cglib.proxy.BridgeMethodResolver
org.springframework.cglib.proxy.BridgeMethodResolver$BridgedFinder
org.springframework.cglib.proxy.BridgeMethodResolver$BridgedFinder$1
org.springframework.cglib.proxy.Enhancer$1
org.springframework.cglib.proxy.Enhancer$2
org.springframework.cglib.proxy.Enhancer$3
org.springframework.cglib.proxy.Enhancer$4
org.springframework.cglib.proxy.Enhancer$5
org.springframework.cglib.proxy.Enhancer$6
org.springframework.cglib.proxy.MethodInterceptorGenerator$1
org.springframework.cglib.proxy.MethodInterceptorGenerator$2
org.springframework.cglib.proxy.NoOpGenerator
org.springframework.cglib.reflect.FastClassEmitter
org.springframework.cglib.reflect.FastClassEmitter$1
org.springframework.cglib.reflect.FastClassEmitter$2
org.springframework.cglib.reflect.FastClassEmitter$3
org.springframework.cglib.reflect.FastClassEmitter$4
org.springframework.cglib.reflect.FastClassEmitter$GetIndexCallback
org.springframework.cglib.transform.TransformingClassGenerator
org.springframework.context.annotation.ConfigurationClassEnhancer$BeanFactoryAwareGeneratorStrategy
org.springframework.context.annotation.ConfigurationClassEnhancer$BeanFactoryAwareGeneratorStrategy$1

@sdeleuze sdeleuze added type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Nov 18, 2022
@sdeleuze sdeleuze added this to the 6.0.1 milestone Nov 18, 2022
@sdeleuze sdeleuze requested a review from jhoeller November 18, 2022 16:05
@sdeleuze sdeleuze self-assigned this Nov 18, 2022
@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 21, 2022
Copy link
Contributor

@jhoeller jhoeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A dependency on Spring's own NativeDetector should rather be avoided there, preserving the independent nature of the CGLIB fork. We could replace that check with a duplicated (System.getProperty("org.graalvm.nativeimage.imagecode") != null), I suppose.

Also, I wonder whether we could put that check into the ClassNotFoundException catch block above, failing hard when the load attempt did not find a corresponding class in a native image. In terms of a clearer exception, this might be quite helpful. However, not sure whether it helps with making the remaining code path unreachable for the GraalVM reachability algorithm?

This change provides also more information to the user
about the missing generated class when that happens.

Closes spring-projectsgh-29521
@sdeleuze sdeleuze force-pushed the cglib-proxy-native-optimization branch from 52b292e to b430862 Compare November 21, 2022 12:10
@sdeleuze
Copy link
Contributor Author

As discussed, I updated the PR to throw a more useful error message, removed the NativeDetector usage and kept the location of the check because otherwise we are forced to update the if statement as well to achieve code removal at build time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants