-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Check for code usage fluctuations in native images #36108
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package io.quarkus.it.jpa.postgresql; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.ExtendWith; | ||
|
||
import io.quarkus.test.junit.QuarkusIntegrationTest; | ||
import io.quarkus.test.junit.nativeimage.NativeBuildOutputExtension; | ||
|
||
@ExtendWith(NativeBuildOutputExtension.class) | ||
@QuarkusIntegrationTest | ||
public class ImageMetricsITCase { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do I get it right that idea is to just add a test of these 17 characters in every IT test case and since the general structure is IT tests will include all the various extensions it will automagically works - no extra setup? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if you follow the three steps mentioned in the description of this PR you should be good to go:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @maxandersen are you OK with this? |
||
@Test | ||
public void verifyImageMetrics() { | ||
NativeBuildOutputExtension buildOutput = new NativeBuildOutputExtension(); | ||
buildOutput.verifyImageMetrics(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Properties file used by ImageMetricsITCase | ||
image_details.total_bytes=86417288 | ||
image_details.total_bytes.tolerance=3 | ||
analysis_results.types.reachable=20067 | ||
analysis_results.types.reachable.tolerance=3 | ||
analysis_results.methods.reachable=99493 | ||
analysis_results.methods.reachable.tolerance=3 | ||
analysis_results.fields.reachable=29845 | ||
analysis_results.fields.reachable.tolerance=3 | ||
analysis_results.types.reflection=6394 | ||
analysis_results.types.reflection.tolerance=3 | ||
analysis_results.methods.reflection=4580 | ||
analysis_results.methods.reflection.tolerance=3 | ||
analysis_results.fields.reflection=143 | ||
analysis_results.fields.reflection.tolerance=3 | ||
analysis_results.types.jni=63 | ||
analysis_results.types.jni.tolerance=1 | ||
analysis_results.methods.jni=55 | ||
analysis_results.methods.jni.tolerance=1 | ||
analysis_results.fields.jni=68 | ||
analysis_results.fields.jni.tolerance=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Properties file used by ImageMetricsITCase | ||
image_details.total_bytes=91650760 | ||
image_details.total_bytes.tolerance=3 | ||
analysis_results.types.reachable=20387 | ||
analysis_results.types.reachable.tolerance=3 | ||
analysis_results.methods.reachable=100956 | ||
analysis_results.methods.reachable.tolerance=3 | ||
analysis_results.fields.reachable=29789 | ||
analysis_results.fields.reachable.tolerance=3 | ||
analysis_results.types.reflection=6522 | ||
analysis_results.types.reflection.tolerance=3 | ||
analysis_results.methods.reflection=4682 | ||
analysis_results.methods.reflection.tolerance=3 | ||
analysis_results.fields.reflection=163 | ||
analysis_results.fields.reflection.tolerance=3 | ||
analysis_results.types.jni=61 | ||
analysis_results.types.jni.tolerance=1 | ||
analysis_results.methods.jni=55 | ||
analysis_results.methods.jni.tolerance=1 | ||
analysis_results.fields.jni=59 | ||
analysis_results.fields.jni.tolerance=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package io.quarkus.it.jpa.postgresql; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.ExtendWith; | ||
|
||
import io.quarkus.test.junit.QuarkusIntegrationTest; | ||
import io.quarkus.test.junit.nativeimage.NativeBuildOutputExtension; | ||
|
||
@ExtendWith(NativeBuildOutputExtension.class) | ||
@QuarkusIntegrationTest | ||
public class ImageMetricsITCase { | ||
|
||
@Test | ||
public void verifyImageMetrics() { | ||
NativeBuildOutputExtension buildOutput = new NativeBuildOutputExtension(); | ||
buildOutput.verifyImageMetrics(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Properties file used by ImageMetricsITCase | ||
image_details.total_bytes=78626464 | ||
image_details.total_bytes.tolerance=3 | ||
analysis_results.types.reachable=19172 | ||
analysis_results.types.reachable.tolerance=3 | ||
analysis_results.methods.reachable=95204 | ||
analysis_results.methods.reachable.tolerance=3 | ||
analysis_results.fields.reachable=27139 | ||
analysis_results.fields.reachable.tolerance=3 | ||
analysis_results.types.reflection=5939 | ||
analysis_results.types.reflection.tolerance=3 | ||
analysis_results.methods.reflection=4401 | ||
analysis_results.methods.reflection.tolerance=3 | ||
analysis_results.fields.reflection=170 | ||
analysis_results.fields.reflection.tolerance=3 | ||
analysis_results.types.jni=63 | ||
analysis_results.types.jni.tolerance=1 | ||
analysis_results.methods.jni=55 | ||
analysis_results.methods.jni.tolerance=1 | ||
analysis_results.fields.jni=68 | ||
analysis_results.fields.jni.tolerance=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Properties file used by ImageMetricsITCase | ||
image_details.total_bytes=83036632 | ||
image_details.total_bytes.tolerance=3 | ||
analysis_results.types.reachable=19394 | ||
analysis_results.types.reachable.tolerance=3 | ||
analysis_results.methods.reachable=96465 | ||
analysis_results.methods.reachable.tolerance=3 | ||
analysis_results.fields.reachable=27025 | ||
analysis_results.fields.reachable.tolerance=3 | ||
analysis_results.types.reflection=6048 | ||
analysis_results.types.reflection.tolerance=3 | ||
analysis_results.methods.reflection=4495 | ||
analysis_results.methods.reflection.tolerance=3 | ||
analysis_results.fields.reflection=192 | ||
analysis_results.fields.reflection.tolerance=3 | ||
analysis_results.types.jni=61 | ||
analysis_results.types.jni.tolerance=1 | ||
analysis_results.methods.jni=55 | ||
analysis_results.methods.jni.tolerance=1 | ||
analysis_results.fields.jni=59 | ||
analysis_results.fields.jni.tolerance=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package io.quarkus.it.main; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.ExtendWith; | ||
|
||
import io.quarkus.test.junit.QuarkusIntegrationTest; | ||
import io.quarkus.test.junit.nativeimage.NativeBuildOutputExtension; | ||
|
||
@ExtendWith(NativeBuildOutputExtension.class) | ||
@QuarkusIntegrationTest | ||
public class ImageMetricsITCase { | ||
@Test | ||
public void verifyImageMetrics() { | ||
NativeBuildOutputExtension buildOutput = new NativeBuildOutputExtension(); | ||
buildOutput.verifyImageMetrics(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Properties file used by ImageMetricsITCase | ||
image_details.total_bytes=139447360 | ||
image_details.total_bytes.tolerance=3 | ||
analysis_results.types.reachable=30117 | ||
analysis_results.types.reachable.tolerance=3 | ||
analysis_results.methods.reachable=150060 | ||
analysis_results.methods.reachable.tolerance=3 | ||
analysis_results.fields.reachable=44502 | ||
analysis_results.fields.reachable.tolerance=3 | ||
analysis_results.types.reflection=8989 | ||
analysis_results.types.reflection.tolerance=3 | ||
analysis_results.methods.reflection=7393 | ||
analysis_results.methods.reflection.tolerance=3 | ||
analysis_results.fields.reflection=438 | ||
analysis_results.fields.reflection.tolerance=3 | ||
analysis_results.types.jni=64 | ||
analysis_results.types.jni.tolerance=1 | ||
analysis_results.methods.jni=55 | ||
analysis_results.methods.jni.tolerance=1 | ||
analysis_results.fields.jni=70 | ||
analysis_results.fields.jni.tolerance=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Properties file used by ImageMetricsITCase | ||
image_details.total_bytes=147268552 | ||
image_details.total_bytes.tolerance=3 | ||
# TODO: Switch to using analysis_results.types.reachable key once we drop support for GraalVM 22.3.0 | ||
analysis_results.classes.reachable=30415 | ||
analysis_results.classes.reachable.tolerance=3 | ||
analysis_results.methods.reachable=151296 | ||
analysis_results.methods.reachable.tolerance=3 | ||
analysis_results.fields.reachable=44325 | ||
analysis_results.fields.reachable.tolerance=3 | ||
# TODO: Switch to using analysis_results.types.reflection key once we drop support for GraalVM 22.3.0 | ||
analysis_results.classes.reflection=9118 | ||
analysis_results.classes.reflection.tolerance=3 | ||
analysis_results.methods.reflection=7741 | ||
analysis_results.methods.reflection.tolerance=3 | ||
analysis_results.fields.reflection=480 | ||
analysis_results.fields.reflection.tolerance=3 | ||
# TODO: Switch to using analysis_results.types.jni key once we drop support for GraalVM 22.3.0 | ||
analysis_results.classes.jni=62 | ||
analysis_results.classes.jni.tolerance=1 | ||
analysis_results.methods.jni=55 | ||
analysis_results.methods.jni.tolerance=1 | ||
analysis_results.fields.jni=61 | ||
analysis_results.fields.jni.tolerance=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
package io.quarkus.test.junit.nativeimage; | ||
|
||
import static io.quarkus.test.junit.IntegrationTestUtil.readQuarkusArtifactProperties; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import java.util.Locale; | ||
import java.util.Properties; | ||
|
||
import jakarta.json.Json; | ||
import jakarta.json.JsonObject; | ||
|
||
import org.junit.jupiter.api.Assertions; | ||
import org.junit.jupiter.api.Assumptions; | ||
import org.junit.jupiter.api.extension.BeforeAllCallback; | ||
import org.junit.jupiter.api.extension.ExtensionContext; | ||
|
||
import io.quarkus.deployment.pkg.steps.GraalVM; | ||
|
||
/** | ||
* This is a general utility to assert via | ||
* unit testing how many classes, methods, objects etc. have been included in a native-image. | ||
* <p> | ||
* For detailed information and explanations on the build output, visit | ||
* <a href="https://github.com/oracle/graal/blob/master/docs/reference-manual/native-image/BuildOutput.md">the upstream GraalVM | ||
* documentation</a>. | ||
*/ | ||
public class NativeBuildOutputExtension implements BeforeAllCallback { | ||
|
||
private static final String IMAGE_METRICS_TEST_PROPERTIES = "image-metrics.properties"; | ||
private static final String IMAGE_METRICS_DIR = "image-metrics"; | ||
private final JsonObject buildOutput; | ||
private static GraalVM.Version mandrelVersion; | ||
|
||
public NativeBuildOutputExtension() { | ||
this.buildOutput = getBuildOutput(); | ||
} | ||
|
||
public void verifyImageMetrics() { | ||
String version = mandrelVersion.getMajorMinorAsString(); | ||
String propertiesFileName = IMAGE_METRICS_DIR + "/" + version + "/" + IMAGE_METRICS_TEST_PROPERTIES; | ||
verifyImageMetrics(propertiesFileName); | ||
} | ||
|
||
public void verifyImageMetrics(String propertiesFileName) { | ||
Properties properties = getProperties(propertiesFileName); | ||
|
||
properties.forEach((key, value) -> { | ||
if (((String) key).endsWith(".tolerance")) { | ||
return; | ||
} | ||
String[] keyParts = ((String) key).split("\\."); | ||
String tolerance = properties.getProperty(key + ".tolerance"); | ||
assert tolerance != null : "tolerance not defined for " + key; | ||
assertValueWithinRange(Integer.parseInt((String) value), Integer.parseInt(tolerance), keyParts); | ||
}); | ||
} | ||
|
||
private Properties getProperties(String propertiesFileName) { | ||
Properties properties = new Properties(); | ||
try { | ||
InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(propertiesFileName); | ||
Assumptions.assumeTrue(resourceAsStream != null, | ||
"Could not find properties file matching the Mandrel version being used: " + propertiesFileName); | ||
properties.load(resourceAsStream); | ||
} catch (IOException e) { | ||
Assertions.fail("Could not load properties from " + propertiesFileName, e); | ||
} | ||
return properties; | ||
} | ||
|
||
private void assertValueWithinRange(int expectedValue, int tolerancePercentage, String... key) { | ||
JsonObject currentObject = buildOutput; | ||
for (int i = 0; i < key.length - 1; i++) { | ||
currentObject = currentObject.getJsonObject(key[i]); | ||
} | ||
String lastKey = key[key.length - 1]; | ||
int actualValue = currentObject.getInt(lastKey); | ||
Assertions.assertTrue(isNumberWithinRange(expectedValue, actualValue, tolerancePercentage), | ||
"Expected " + String.join(".", key) + " to be within range [" + expectedValue + " +- " + tolerancePercentage | ||
+ "%] but was " + actualValue); | ||
} | ||
|
||
private boolean isNumberWithinRange(int expectedNumberOfClasses, int actualNumberOfClasses, int tolerancePercentage) { | ||
final int lowerBound = expectedNumberOfClasses - (expectedNumberOfClasses * tolerancePercentage / 100); | ||
final int upperBound = expectedNumberOfClasses + (expectedNumberOfClasses * tolerancePercentage / 100); | ||
return actualNumberOfClasses >= lowerBound && actualNumberOfClasses <= upperBound; | ||
} | ||
|
||
private static JsonObject getBuildOutput() { | ||
final Path buildOutputPath = getBuildOutputPath(); | ||
try (InputStream inputStream = Files.newInputStream(buildOutputPath)) { | ||
return Json.createReader(inputStream).readObject(); | ||
} catch (Exception e) { | ||
throw new RuntimeException("Could not load build output", e); | ||
} | ||
} | ||
|
||
private static Path getBuildOutputPath() { | ||
final Path buildDirectory = locateNativeImageBuildDirectory(); | ||
final File[] buildOutput = buildDirectory.toFile().listFiles((dir, name) -> name.toLowerCase(Locale.ROOT) | ||
.endsWith("-build-output-stats.json")); | ||
Assertions.assertNotNull(buildOutput, "Could not identify the native image build output"); | ||
Assertions.assertEquals(1, buildOutput.length, "Could not identify the native image build output"); | ||
return buildOutput[0].toPath(); | ||
} | ||
|
||
private static Path locateNativeImageBuildDirectory() { | ||
Path buildPath = Paths.get("target"); | ||
final File[] files = buildPath.toFile().listFiles((dir, name) -> name.toLowerCase(Locale.ROOT) | ||
.endsWith("-native-image-source-jar")); | ||
Assertions.assertNotNull(files, "Could not identify the native image build directory"); | ||
Assertions.assertEquals(1, files.length, "Could not identify the native image build directory"); | ||
return files[0].toPath(); | ||
} | ||
|
||
@Override | ||
public void beforeAll(ExtensionContext extensionContext) throws Exception { | ||
mandrelVersion = getMandrelVersion(extensionContext); | ||
} | ||
|
||
private GraalVM.Version getMandrelVersion(ExtensionContext context) { | ||
Properties quarkusArtifactProperties = readQuarkusArtifactProperties(context); | ||
String fullVersion = quarkusArtifactProperties.getProperty("metadata.graalvm.version.full"); | ||
try { | ||
return GraalVM.Version.of(fullVersion.lines()); | ||
} catch (NumberFormatException e) { | ||
System.out.println( | ||
"WARNING: Unable to determine the GraalVM version with which the native binary was built. metadata.graalvm.version.full = " | ||
+ fullVersion); | ||
return GraalVM.Version.CURRENT; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you expect people to copy/paste this class in various modules, then edit the constants.
Perhaps it could be reworked to help reuse of code a little? I'd especially avoid having to copy paste the report name constants everywhere.
e.g. change
buildOutput.assertValueWithinRange(EXPECTED_IMAGE_SIZE, TOLERANCE_PERCENTAGE, "image_details", "total_bytes");
to
buildOutput.expectImageSize(long expectedzie, int tolerancePercentage)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I'm wondering if you really want to allow some margin of error for each of these. For example, does it make sense to have tolerante about EXPECTED_JNI_TYPES ?
With a dedicated method you'd be able to nudge the other developers in the right direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, it's much cleaner now and uses properties files for the expected value and the tolerance instead of code. This way it's more flexible and doesn't require code changes to add a new check.
Sure, for some cases we can be stricter. Happy to adjust the tolerance to whatever the Quarkus team would feel comfortable with (note that the stricter the checks the more the expected test failures in the CI).