Skip to content

Commit

Permalink
Replace build-time initialization by constant fields
Browse files Browse the repository at this point in the history
This commit leverages a subset of @philwebb initial experimentation
to compute at build time the value of specific boolean static fields
in native images. This enhancement is implemented for now as a
GraalVM feature.

The goal here is to keep an optimized footprint via build time code
removal without leveraging build-time class initialization which is known
for the blocking compatibility issues it introduces due to its viral nature.

For now, the static fields initialized at build time with native are:
 - NativeDetector#imageCode
 - Fields with a name ending by "Present" in "org.springframework" package
   typically used for classpath check with ClassUtils#isPresent

Closes gh-28624
  • Loading branch information
sdeleuze committed Jun 24, 2022
1 parent dc4ae55 commit 22a750f
Show file tree
Hide file tree
Showing 16 changed files with 413 additions and 15 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ classes/
buildSrc/build
/spring-*/build
/spring-core/kotlin-coroutines/build
/spring-core/graalvm/build
/framework-bom/build
/integration-tests/build
/src/asciidoc/build
Expand Down
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,8 @@ configure(allprojects) { project ->
dependency "org.glassfish:jakarta.el:4.0.2"
dependency "org.glassfish.tyrus:tyrus-container-servlet:2.0.1"
dependency "org.eclipse.persistence:org.eclipse.persistence.jpa:3.0.2"

dependency "org.graalvm.nativeimage:svm:22.1.0.1"
}
generatedPomCustomization {
enabled = false
Expand Down
2 changes: 1 addition & 1 deletion framework-bom/framework-bom.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ group = "org.springframework"

dependencies {
constraints {
parent.moduleProjects.sort { "$it.name" }.each {
parent.moduleProjects.findAll{ it.name != 'spring-core-graalvm' }.sort { "$it.name" }.each {
api it
}
}
Expand Down
16 changes: 10 additions & 6 deletions gradle/docs.gradle
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
ext {
documentedProjects = moduleProjects.findAll { it.name != 'spring-core-graalvm' }
}

configurations {
asciidoctorExt
}
Expand All @@ -24,7 +28,7 @@ task api(type: Javadoc) {
title = "${rootProject.description} ${version} API"

dependsOn {
moduleProjects.collect {
documentedProjects.collect {
it.tasks.getByName("jar")
}
}
Expand All @@ -33,7 +37,7 @@ task api(type: Javadoc) {
// ensure the javadoc process can resolve types compiled from .aj sources
project(":spring-aspects").sourceSets.main.output
)
classpath += files(moduleProjects.collect { it.sourceSets.main.compileClasspath })
classpath += files(documentedProjects.collect { it.sourceSets.main.compileClasspath })
}

options {
Expand All @@ -48,7 +52,7 @@ task api(type: Javadoc) {
addBooleanOption('Xdoclint:syntax', true) // only check syntax with doclint
addBooleanOption('Werror', true) // fail build on Javadoc warnings
}
source moduleProjects.collect { project ->
source documentedProjects.collect { project ->
project.sourceSets.main.allJava
}
maxMemory = "1024m"
Expand Down Expand Up @@ -180,7 +184,7 @@ task schemaZip(type: Zip) {
description = "Builds -${archiveClassifier} archive containing all " +
"XSDs for deployment at https://springframework.org/schema."
duplicatesStrategy DuplicatesStrategy.EXCLUDE
moduleProjects.each { module ->
documentedProjects.each { module ->
def Properties schemas = new Properties();

module.sourceSets.main.resources.find {
Expand Down Expand Up @@ -230,7 +234,7 @@ task distZip(type: Zip, dependsOn: [docsZip, schemaZip]) {
into "${baseDir}/schema"
}

moduleProjects.each { module ->
documentedProjects.each { module ->
into ("${baseDir}/libs") {
from module.jar
if (module.tasks.findByPath("sourcesJar")) {
Expand All @@ -243,4 +247,4 @@ task distZip(type: Zip, dependsOn: [docsZip, schemaZip]) {
}
}

distZip.mustRunAfter moduleProjects.check
distZip.mustRunAfter documentedProjects.check
2 changes: 2 additions & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ include "spring-context"
include "spring-context-indexer"
include "spring-context-support"
include "spring-core"
include "spring-core-graalvm"
project(':spring-core-graalvm').projectDir = file('spring-core/graalvm')
include "spring-core-test"
include "spring-expression"
include "spring-instrument"
Expand Down
48 changes: 48 additions & 0 deletions spring-core/graalvm/spring-core-graalvm.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
description = "Spring Core GraalVM feature"

configurations {
classesOnlyElements {
canBeConsumed = true
canBeResolved = false
}
}

artifacts {
classesOnlyElements(compileJava.destinationDirectory)
classesOnlyElements(sourceSets.main.resources.srcDirs)
}

tasks.withType(JavaCompile) {
options.compilerArgs += [
"--add-modules",
"jdk.internal.vm.ci",
"--add-exports",
"jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED"
]
}

eclipse.classpath.file {
whenMerged {
entries.find{ it.path ==~ '.*JRE_CONTAINER.*' }.each {
it.entryAttributes['module'] = true
it.entryAttributes['add-exports'] = 'jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED'
it.entryAttributes['limit-modules'] = 'java.se,jdk.accessibility,jdk.attach,jdk.compiler,jdk.httpserver,jdk.jartool,jdk.jconsole,jdk.jdi,jdk.management,jdk.sctp,jdk.security.auth,jdk.security.jgss,jdk.unsupported,jdk.dynalink,jdk.incubator.foreign,jdk.incubator.vector,jdk.javadoc,jdk.jfr,jdk.jshell,jdk.management.jfr,jdk.net,jdk.nio.mapmode,jdk.unsupported.desktop,jdk.jsobject,jdk.xml.dom,jdk.internal.vm.ci'
}
}
}

dependencies {
compileOnly("org.graalvm.nativeimage:svm")
}

tasks.withType(PublishToMavenRepository) {
enabled = false
}

tasks.withType(PublishToMavenLocal) {
enabled = false
}

tasks.withType(Javadoc) {
enabled = false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.aot.graalvm;

import com.oracle.svm.core.annotate.AutomaticFeature;
import com.oracle.svm.hosted.FeatureImpl.DuringSetupAccessImpl;
import org.graalvm.compiler.debug.DebugContext;
import org.graalvm.nativeimage.hosted.Feature;

/**
* GraalVM {@link Feature} that substitutes field values that match a certain pattern
* with constants without causing build-time initialization.
*
* @author Phillip Webb
* @author Sebastien Deleuze
* @since 6.0
*/
@AutomaticFeature
class ConstantFieldFeature implements Feature {

@Override
public void duringSetup(DuringSetupAccess access) {
duringSetup((DuringSetupAccessImpl) access);
}

private void duringSetup(DuringSetupAccessImpl access) {
DebugContext debug = access.getDebugContext();
try (DebugContext.Scope scope = debug.scope("ConstantFieldFeature.duringSetup")) {
debug.log("Installing constant field substitution processor : " + scope);
ClassLoader applicationClassLoader = access.getApplicationClassLoader();
ConstantFieldSubstitutionProcessor substitutionProcessor =
new ConstantFieldSubstitutionProcessor(debug, applicationClassLoader);
access.registerSubstitutionProcessor(substitutionProcessor);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.aot.graalvm;

import java.lang.reflect.Field;
import java.util.regex.Pattern;

import com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor;
import com.oracle.svm.core.meta.SubstrateObjectConstant;
import com.oracle.svm.core.util.UserError;
import jdk.vm.ci.meta.JavaConstant;
import jdk.vm.ci.meta.JavaKind;
import jdk.vm.ci.meta.ResolvedJavaField;
import jdk.vm.ci.meta.ResolvedJavaType;
import org.graalvm.compiler.debug.DebugContext;

/**
* {@link SubstitutionProcessor} to compute at build time the value of the
* boolean static fields identified by {@link #patterns} in order to allow
* efficient code shrinking without using class build time initialization.
*
* @author Phillip Webb
* @author Sebastien Deleuze
* @since 6.0
*/
class ConstantFieldSubstitutionProcessor extends SubstitutionProcessor {

This comment has been minimized.

Copy link
@christianwimmer

christianwimmer Jun 28, 2022

An easier way to do the same thing should actually be to use a NodePlugin that constant-folds field accesses during bytecode parsing. See for example https://github.com/oracle/graal/blob/89e4cfc7aeea69970b60c64cd075ceb2a104e864/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/EarlyConstantFoldLoadFieldPlugin.java#L70

But that of course is then still highly dependent on internals of the image generator, i.e., it will need to be fixed often when GraalVM changes.

This comment has been minimized.

Copy link
@sdeleuze

sdeleuze Jun 29, 2022

Author Contributor

Thanks for the hint, I think API stability is important for us so we may stick to the current arrangement. cc @philwebb

This comment has been minimized.

Copy link
@christianwimmer

christianwimmer Jun 29, 2022

Well, this current solution is equally dependent on internals, so from that point of view both solutions are equally bad. Don't expect that this feature will work with future GraalVM releases. It might be broken already in the upcoming 22.2 release because ConstantReadableJavaField does not implement the new AnnotationWrapper interface.

This comment has been minimized.

Copy link
@sdeleuze

sdeleuze Jun 30, 2022

Author Contributor

Indeed it is + there is another breakage due to the switch to module system, let's discuss that with @vjovanov, you and others asap.

This comment has been minimized.

Copy link
@christianwimmer

christianwimmer Jun 30, 2022

I have been thinking about a proper "field value replacer" API for quite a while. Not in the context of use cases like yours, but to replace the current non-API RecomputeFieldValue annotation. But I think we can make your use case work with such an API too. I think it is important that Spring gets to a point where it uses only stable native image API, i.e., does not have any import of anything in com.oracle.svm.

This comment has been minimized.

Copy link
@sdeleuze

sdeleuze Jul 1, 2022

Author Contributor

That would be ideal. Short term, it seems we have fixed the compatbility issues with GraalVM 22.2. A nice outcome would be to get stable API for that in time for GraalVM 22.3 in order to use it for Spring Framework 6 and Spring Boot 3 GA. cc @jhoeller


// Later should be an explicit signal, like an annotation or even a Java keyword
private static Pattern[] patterns = {
Pattern.compile(Pattern.quote("org.springframework.core.NativeDetector#imageCode")),
Pattern.compile(Pattern.quote("org.springframework.") + ".*#.*Present"),
};

private final ThrowawayClassLoader throwawayClassLoader;


ConstantFieldSubstitutionProcessor(DebugContext debug, ClassLoader applicationClassLoader) {
this.throwawayClassLoader = new ThrowawayClassLoader(applicationClassLoader);
}


@Override
public ResolvedJavaField lookup(ResolvedJavaField field) {
ResolvedJavaType declaringClass = field.getDeclaringClass();
if (field.getType().getJavaKind() == JavaKind.Boolean && field.isStatic()) {
String fieldIdentifier = declaringClass.toJavaName() + "#" + field.getName();
for (Pattern pattern : patterns) {
if (pattern.matcher(fieldIdentifier).matches()) {
JavaConstant constant = lookupConstant(declaringClass.toJavaName(), field.getName());
if (constant != null) {
// TODO Use proper logging only when --verbose is specified when https://github.com/oracle/graal/issues/4669 will be fixed
System.out.println("Field " + fieldIdentifier + " set to " + constant.toValueString() + " at build time");
return new ConstantReadableJavaField(field, constant);
}
}
}
}
return super.lookup(field);
}

private JavaConstant lookupConstant(String className, String fieldName) {
try {
Class<?> throwawayClass = this.throwawayClassLoader.loadClass(className);
Field field = throwawayClass.getDeclaredField(fieldName);
field.setAccessible(true);
Object value = field.get(null);
if (!(value instanceof Boolean)) {
throw UserError.abort("Unable to get the value of " + className + "." + fieldName);
}
return SubstrateObjectConstant.forBoxedValue(JavaKind.Boolean, value);
}
catch (Exception ex) {
throw new IllegalStateException("Unable to read value from " + className + "." + fieldName, ex);
}
}

}
Loading

0 comments on commit 22a750f

Please sign in to comment.