Skip to content

Commit

Permalink
8346239: Improve memory efficiency of JimageDiffGenerator
Browse files Browse the repository at this point in the history 
Reviewed-by: mbaesken
  • Loading branch information
@jerboaa
jerboaa committed Jan 8, 2025
1 parent cbabc04 commit f696d9c
Show file tree
Hide file tree
Showing 16 changed files with 74 additions and 20 deletions.
59 changes: 54 additions & 5 deletions src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/JimageDiffGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
*/
package jdk.tools.jlink.internal.runtimelink;

import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
Expand All @@ -49,6 +52,7 @@ public class JimageDiffGenerator {
public interface ImageResource extends AutoCloseable {
public List<String> getEntries();
public byte[] getResourceBytes(String name);
public InputStream getResource(String name);
}

/**
Expand All @@ -71,7 +75,6 @@ public List<ResourceDiff> generateDiff(ImageResource base, ImageResource image)
resources.addAll(image.getEntries());
baseResources = base.getEntries();
for (String item: baseResources) {
byte[] baseBytes = base.getResourceBytes(item);
// First check that every item in the base image exist in
// the optimized image as well. If it does not, it's a removed
// item in the optimized image.
Expand All @@ -82,19 +85,18 @@ public List<ResourceDiff> generateDiff(ImageResource base, ImageResource image)
ResourceDiff.Builder builder = new ResourceDiff.Builder();
ResourceDiff diff = builder.setKind(ResourceDiff.Kind.REMOVED)
.setName(item)
.setResourceBytes(baseBytes)
.setResourceBytes(base.getResourceBytes(item))
.build();
diffs.add(diff);
continue;
}
// Verify resource bytes are equal if present in both images
boolean contentEquals = Arrays.equals(baseBytes, image.getResourceBytes(item));
if (!contentEquals) {
if (!compareStreams(base.getResource(item), image.getResource(item))) {
// keep track of original bytes (non-optimized)
ResourceDiff.Builder builder = new ResourceDiff.Builder();
ResourceDiff diff = builder.setKind(ResourceDiff.Kind.MODIFIED)
.setName(item)
.setResourceBytes(baseBytes)
.setResourceBytes(base.getResourceBytes(item))
.build();
diffs.add(diff);
}
Expand All @@ -112,4 +114,51 @@ public List<ResourceDiff> generateDiff(ImageResource base, ImageResource image)
return diffs;
}

/**
* Compare the contents of the two input streams (byte-by-byte).
*
* @param is1 The first input stream
* @param is2 The second input stream
* @return {@code true} iff the two streams contain the same number of
* bytes and each byte of the streams are equal. {@code false}
* otherwise.
*/
private boolean compareStreams(InputStream is1, InputStream is2) {
byte[] buf1 = new byte[1024];
byte[] buf2 = new byte[1024];
int bytesRead1, bytesRead2 = 0;
try {
try (is1; is2) {
while ((bytesRead1 = is1.read(buf1)) != -1 &&
(bytesRead2 = is2.read(buf2)) != -1) {
if (bytesRead1 != bytesRead2) {
return false;
}
if (bytesRead1 == buf1.length) {
if (!Arrays.equals(buf1, buf2)) {
return false;
}
} else {
for (int i = 0; i < bytesRead1; i++) {
if (buf1[i] != buf2[i]) {
return false;
}
}
}
}
// ensure we read both to the end
if (bytesRead1 == -1) {
bytesRead2 = is2.read(buf2);
if (bytesRead2 != -1) {
return false;
}
return true;
}
}
} catch (IOException e) {
throw new UncheckedIOException("IO exception when comparing bytes", e);
}
return false;
}

}
6 changes: 6 additions & 0 deletions src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/ResourcePoolReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

package jdk.tools.jlink.internal.runtimelink;

import java.io.InputStream;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -56,4 +57,9 @@ public byte[] getResourceBytes(String name) {
return pool.findEntry(name).orElseThrow().contentBytes();
}

@Override
public InputStream getResource(String name) {
return pool.findEntry(name).orElseThrow().content();
}

}
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/AddOptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m AddOptionsTest
* @run main/othervm -Xmx1g AddOptionsTest
*/
public class AddOptionsTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/BasicJlinkMissingJavaBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m BasicJlinkMissingJavaBase
* @run main/othervm -Xmx1g BasicJlinkMissingJavaBase
*/
public class BasicJlinkMissingJavaBase extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/BasicJlinkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m BasicJlinkTest false
* @run main/othervm -Xmx1g BasicJlinkTest false
*/
public class BasicJlinkTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/CustomModuleJlinkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m CustomModuleJlinkTest
* @run main/othervm -Xmx1g CustomModuleJlinkTest
*/
public class CustomModuleJlinkTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/GenerateJLIClassesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m GenerateJLIClassesTest
* @run main/othervm -Xmx1g GenerateJLIClassesTest
*/
public class GenerateJLIClassesTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/JavaSEReproducibleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m JavaSEReproducibleTest
* @run main/othervm -Xmx1g JavaSEReproducibleTest
*/
public class JavaSEReproducibleTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/KeepPackagedModulesFailTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m KeepPackagedModulesFailTest
* @run main/othervm -Xmx1g KeepPackagedModulesFailTest
*/
public class KeepPackagedModulesFailTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/ModifiedFilesExitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m ModifiedFilesExitTest
* @run main/othervm -Xmx1g ModifiedFilesExitTest
*/
public class ModifiedFilesExitTest extends ModifiedFilesTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/ModifiedFilesWarningTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m ModifiedFilesWarningTest
* @run main/othervm -Xmx1g ModifiedFilesWarningTest
*/
public class ModifiedFilesWarningTest extends ModifiedFilesTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/MultiHopTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m MultiHopTest
* @run main/othervm -Xmx1g MultiHopTest
*/
public class MultiHopTest extends AbstractLinkableRuntimeTest {

Expand Down
3 changes: 1 addition & 2 deletions test/jdk/tools/jlink/runtimeImage/PackagedModulesVsRuntimeImageLinkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm/timeout=1200 -Xmx1400m PackagedModulesVsRuntimeImageLinkTest
* @run main/othervm/timeout=1200 -Xmx1g PackagedModulesVsRuntimeImageLinkTest
*/
public class PackagedModulesVsRuntimeImageLinkTest extends AbstractLinkableRuntimeTest {

Expand All @@ -76,7 +76,6 @@ void runTest(Helper helper, boolean isLinkableRuntime) throws Exception {
.output(helper.createNewImageDir("java-se-jmodfull"))
.addMods("java.se").call().assertSuccess();

System.out.println("Now comparing jmod-less and jmod-full) images");
compareRecursively(javaSEruntimeLink, javaSEJmodFull);
}

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/PatchedJDKModuleJlinkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m PatchedJDKModuleJlinkTest
* @run main/othervm -Xmx1g PatchedJDKModuleJlinkTest
*/
public class PatchedJDKModuleJlinkTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/SystemModulesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m SystemModulesTest
* @run main/othervm -Xmx1g SystemModulesTest
*/
public class SystemModulesTest extends AbstractLinkableRuntimeTest {

Expand Down
2 changes: 1 addition & 1 deletion test/jdk/tools/jlink/runtimeImage/SystemModulesTest2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
* jdk.jlink/jdk.tools.jimage
* @build tests.* jdk.test.lib.process.OutputAnalyzer
* jdk.test.lib.process.ProcessTools
* @run main/othervm -Xmx1400m SystemModulesTest2
* @run main/othervm -Xmx1g SystemModulesTest2
*/
public class SystemModulesTest2 extends AbstractLinkableRuntimeTest {

Expand Down

3 comments on commit f696d9c

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

@jerboaa
Copy link
Contributor Author

@jerboaa jerboaa commented on f696d9c Jan 8, 2025

Choose a reason for hiding this comment

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

/backport jdk24u

@openjdk
Copy link

@openjdk openjdk bot commented on f696d9c Jan 8, 2025

Choose a reason for hiding this comment

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

@jerboaa the backport was successfully created on the branch backport-jerboaa-f696d9c5-master in my personal fork of openjdk/jdk24u. To create a pull request with this backport targeting openjdk/jdk24u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit f696d9c5 from the openjdk/jdk repository.

The commit being backported was authored by Severin Gehwolf on 8 Jan 2025 and was reviewed by Matthias Baesken.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk24u:

$ git fetch https://github.com/openjdk-bots/jdk24u.git backport-jerboaa-f696d9c5-master:backport-jerboaa-f696d9c5-master
$ git checkout backport-jerboaa-f696d9c5-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk24u.git backport-jerboaa-f696d9c5-master

⚠️ @jerboaa You are not yet a collaborator in my fork openjdk-bots/jdk24u. An invite will be sent out and you need to accept it before you can proceed.

Please sign in to comment.