8321620: Optimize JImage decompressors #16556

Glavo · 2023-11-08T11:55:22Z

This PR significantly speeds up decompressing resources in Jimage while significantly reducing temporary memory allocations in the process.

This will improve startup speed for runtime images generated using jlink --compress 1 and jlink --compress 2 .

I generated a runtime image containing javac using jlink --compress 1 --add-modules jdk.compiler and tested the time it took to compile a simple HelloWorld program 20 times using perf stat -r20 javac /dev/shm/HelloWorld.java, this PR reduces the total time taken from 17830ms to 13598ms (31.12% faster).

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8321620: Optimize JImage decompressors (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16556/head:pull/16556
$ git checkout pull/16556

Update a local copy of the PR:
$ git checkout pull/16556
$ git pull https://git.openjdk.org/jdk.git pull/16556/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16556

View PR using the GUI difftool:
$ git pr show -t 16556

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16556.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-11-08T11:56:34Z

👋 Welcome back Glavo! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-11-08T11:57:34Z

@Glavo The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

Glavo · 2023-11-08T11:57:57Z

I generated runtime images using jlink --compress (1|2) --add-modules java.se,jdk.unsupported,jdk.management and then ran the following JMH benchmark:

@Warmup(iterations = 10, time = 2)
@Measurement(iterations = 5, time = 3)
@Fork(value = 1, jvmArgsAppend = {"-XX:+UseG1GC", "-Xms8g", "-Xmx8g", "--add-exports=java.base/jdk.internal.jimage=ALL-UNNAMED"})
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Benchmark)
public class Decompress {

    private static final ImageReader READER = ImageReaderFactory.getImageReader();
    private static final ImageLocation LOC = READER.findLocation("java.base", "java/lang/String.class");

    @Benchmark
    public ByteBuffer test() {
        return READER.getResourceBuffer(LOC);
    }

}

This is the result:

String Share
                                                            (Baseline)                        (Current)
Benchmark                           Mode  Cnt       Score       Error   Units        Score       Error   Units
Decompress.test                     avgt    5  184243.403 ± 3643.983   ns/op     35176.514 ±   507.618   ns/op   (-80.91%)
Decompress.test:gc.alloc.rate       avgt    5    3263.730 ±   64.431  MB/sec      3143.058 ±    45.330  MB/sec
Decompress.test:gc.alloc.rate.norm  avgt    5  630544.422 ±    0.008    B/op    115936.081 ±     0.001    B/op   (-81.61%)
Decompress.test:gc.count            avgt    5      10.000             counts         9.000              counts
Decompress.test:gc.time             avgt    5      14.000                 ms        13.000                  ms


Zip

Benchmark                           Mode  Cnt       Score       Error   Units        Score       Error   Units
Decompress.test                     avgt    5  194237.534 ±  1026.180   ns/op   152855.728 ± 16058.780   ns/op   (-21.30%)
Decompress.test:gc.alloc.rate       avgt    5    1197.700 ±     6.306  MB/sec      464.278 ±    47.465  MB/sec
Decompress.test:gc.alloc.rate.norm  avgt    5  243953.338 ±     5.810    B/op    74376.291 ±     2.175    B/op   (-69.51%)
Decompress.test:gc.count            avgt    5       2.000              counts        1.000              counts
Decompress.test:gc.time             avgt    5       4.000                  ms        3.000                  ms

The results show that memory allocation is reduced by more than 70% while decompression speed is significantly improved.

Glavo · 2023-11-08T18:24:30Z

I ran the tier1 and tier2 tests and there were no new errors.

src/java.base/share/classes/jdk/internal/jimage/decompressor/StringSharingDecompressor.java

liach · 2023-11-09T02:07:57Z

src/java.base/share/classes/jdk/internal/jimage/decompressor/StringSharingDecompressor.java

+        bytesOut[bytesOutOffset++] = (byte) ((count >> 8) & 0xff);
+        bytesOut[bytesOutOffset++] = (byte) (count & 0xff);


Probably use ByteArray.setUnsignedShort:

Suggested change

bytesOut[bytesOutOffset++] = (byte) ((count >> 8) & 0xff);

bytesOut[bytesOutOffset++] = (byte) (count & 0xff);

ByteArray.setUnsignedShort(bytesOut, bytesOutOffset, count);

bytesOutOffset += 2;

Same remark elsewhere.

These classes need to be compiled with --release 8, so I can't use the ByteArray.

Glavo · 2023-11-09T23:02:11Z

I generated a runtime image containing javac using jlink --compress (0|1) --add-modules jdk.compiler and tested the time it took to compile a simple HelloWorld program 20 times using perf stat -r20 javac /dev/shm/HelloWorld.java. This is the result:

Baseline, No Compress: 10829ms

String Share:

Baseline: 17830ms
This PR: 13598ms

Zip:

Baseline: 12350ms
This PR: 12279ms

You can see that in this test, this PR made the runtime image compressed using string share 30% faster.

minborg · 2023-11-20T17:18:11Z

src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java

@@ -344,6 +345,23 @@ public String getString(int offset) {
        return ImageStringsReader.stringFromByteBuffer(strings, offset);
    }

+    int getStringMUTF8(int offset, byte[] bytesOut, int bytesOutOffset) {
+        if (offset < 0 || offset >= strings.limit()) {


Can we use Objects::checkIndex here instead?

Can we use Objects::checkIndex here instead?

No, because these classes need to be compiled with --release 8.

Javadoc:

/** * @implNote This class needs to maintain JDK 8 source compatibility. * * It is used internally in the JDK to implement jimage/jrtfs access, * but also compiled and delivered as part of the jrtfs.jar to support access * to the jimage file provided by the shipped JDK by tools running on JDK 8. */

minborg · 2023-11-20T17:19:14Z

src/java.base/share/classes/jdk/internal/jimage/decompressor/StringSharingDecompressor.java

-        out.writeShort(count);
+                                   int offset, long originalSize) throws IOException {
+        if (originalSize > Integer.MAX_VALUE) {
+            throw new OutOfMemoryError("Required array size too large");


Is this the correct exception type?

Is this the correct exception type?

I'm not sure, but I think it shouldn't matter, since the exception is never really thrown here for a valid jimage file.

It's common to throw OutOfMemoryError when a requested array size would be too large, either due overflow or because some other limitation (see StringUTF16::newBytesFor or String::replace for some examples).

cl4es · 2023-11-20T23:36:51Z

I'll need some time to review this thoroughly but skimming through it it seems like a very nice enhancement, making --compress more appealing - great work!

One gotcha in this area is that jlink by default does not recreate the CDS archive. Startup might gain from generating out the default CDS archive (java -Xshare:dump), but not doing so is a reasonable thing to do if you're aiming for minimal static footprint. But to make comparisons fair you need to take care to compare apples to apples: is the uncompressed baseline a similar jlink image generated without --compress? Or the default JDK image (uncompressed, with CDS)?

stsypanov · 2023-11-21T15:08:25Z

test/jdk/tools/jlink/plugins/CompressIndexesTest.java

        }
    }

    private void check(byte[] flow, List<byte[]> arrays) {
-        List<Integer> d = CompressIndexes.decompressFlow(flow);
+        List<Integer> d = Arrays.stream(CompressIndexes.decompressFlow(flow)).boxed().toList();


Can we rewrite this as

int[] d = CompressIndexes.decompressFlow(flow); int[] dd = new int[arrays.length]; for (int j = 0; j < arrays.length; j++) { byte[] b = arrays[j]; dd[j] = CompressIndexes.decompress(b, 0); } if (!Arrays.equals(d, dd) { //... }

Glavo · 2023-11-23T09:06:24Z

But to make comparisons fair you need to take care to compare apples to apples: is the uncompressed baseline a similar jlink image generated without --compress? Or the default JDK image (uncompressed, with CDS)?

The uncompressed runtime image used for comparison was also generated using jlink, without using CDS, so the comparison should be fair.

Glavo · 2023-12-08T21:35:34Z

Does anyone want to take a look at this PR?

cl4es · 2023-12-08T22:38:56Z

Does anyone want to take a look at this PR?

RFE: https://bugs.openjdk.org/browse/JDK-8321620 - update the bug ID and this should PR should reach a wider audience. I'll have some time next week to help review.

mlbridge · 2023-12-08T22:43:34Z

Webrevs

00: Full (c9e96385)

bridgekeeper · 2024-01-06T01:49:01Z

@Glavo This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Glavo · 2024-01-06T04:17:43Z

/open

openjdk · 2024-01-06T04:18:36Z

@Glavo This pull request is already open

Glavo · 2024-01-08T18:29:14Z

Can anyone review this PR?

cl4es

I had to merge with master to get jlink to work, perhaps some local issue since Classfile API has been moving around.

While the improvement to --compress 1 are impressive, this compression mode is effectively deprecated. I'm not sure your improvements to it here will be enough to reverse that decision (it's still a bit behind on all measures, no?). Perhaps it would be better to split out those changes and move forward with and focus this on the zip decompressor enhancements?

Glavo · 2024-01-12T16:59:40Z

@cl4es

While the improvement to --compress 1 are impressive, this compression mode is effectively deprecated.

I noticed the deprecation warning before creating this PR, so I did some research.

This warning was introduced here: #11617. In the discussion of that PR I learned that the option --compress 1 was deprecated because it was not compression but string sharing. The string sharing plugin is not actually deprecated, it's just that the command line option may be renamed in the future.

mlchung · 2024-01-12T18:17:23Z

The plan [1] is to remove the old compression values 0, 1, 2 in the future and only support the zip compression --compress zip-[0-9], i.e. the string sharing plugin will be removed as there isn't any known customer usage of this plugin. If you are aware of any customer using it, we can reevaluate.

[1] https://bugs.openjdk.org/browse/JDK-8301124?focusedId=14562770&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14562770

Glavo · 2024-01-12T18:34:03Z

The plan [1] is to remove the old compression values 0, 1, 2 in the future and only support the zip compression --compress zip-[0-9], i.e. the string sharing plugin will be removed as there isn't any known customer usage of this plugin. If you are aware of any customer using it, we can reevaluate.

[1] https://bugs.openjdk.org/browse/JDK-8301124?focusedId=14562770&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14562770

I expected that few people use this option, because compared to zip, it is not superior in file size or decompression speed. However, I did some experiments in the JApp project and found that the file size using a mixture of string sharing + zstd compression was significantly smaller than using only zstd, so I thought it might be meaningful to study it further.

Anyway, these are things for the future and I want this PR to stay simple, so I'll just drop these modifications.

Glavo · 2024-01-12T18:48:43Z

I created a new PR: #17405

I want to leave my changes to the string sharing plugin here so I can pick them up in the future.

Glavo added 3 commits November 8, 2023 07:09

opt ZipDecompressor

239dfc4

opt StringSharingDecompressor

8dc836b

Delete CompressIndexes.readInt(DataInputStream)

8809c6c

openjdk bot added the core-libs core-libs-dev@openjdk.org label Nov 8, 2023

liach reviewed Nov 9, 2023

View reviewed changes

Glavo added 4 commits November 10, 2023 00:33

opt decompressFlow

98cc79a

use Byte.toUnsignedInt

be6fe45

Small optimization

007077d

getStringMUTF8

c9e9638

minborg reviewed Nov 20, 2023

View reviewed changes

stsypanov reviewed Nov 21, 2023

View reviewed changes

Glavo changed the title ~~Optimize JImage decompressors~~ 8321620: Optimize JImage decompressors Dec 8, 2023

openjdk bot added the rfr Pull request is ready for review label Dec 8, 2023

cl4es reviewed Jan 12, 2024

View reviewed changes

Glavo closed this Jan 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8321620: Optimize JImage decompressors #16556

8321620: Optimize JImage decompressors #16556

Glavo commented Nov 8, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Nov 8, 2023

openjdk bot commented Nov 8, 2023

Glavo commented Nov 8, 2023 •

edited

Loading

Glavo commented Nov 8, 2023

liach Nov 9, 2023

Glavo Nov 9, 2023 •

edited

Loading

Glavo commented Nov 9, 2023 •

edited

Loading

minborg Nov 20, 2023

Glavo Nov 20, 2023

minborg Nov 20, 2023

Glavo Nov 20, 2023

cl4es Nov 20, 2023

cl4es commented Nov 20, 2023 •

edited

Loading

stsypanov Nov 21, 2023

Glavo commented Nov 23, 2023 •

edited

Loading

Glavo commented Dec 8, 2023

cl4es commented Dec 8, 2023

mlbridge bot commented Dec 8, 2023

bridgekeeper bot commented Jan 6, 2024

Glavo commented Jan 6, 2024

openjdk bot commented Jan 6, 2024

Glavo commented Jan 8, 2024

cl4es left a comment •

edited

Loading

Glavo commented Jan 12, 2024 •

edited

Loading

mlchung commented Jan 12, 2024

Glavo commented Jan 12, 2024

Glavo commented Jan 12, 2024

		bytesOut[bytesOutOffset++] = (byte) ((count >> 8) & 0xff);
		bytesOut[bytesOutOffset++] = (byte) (count & 0xff);

8321620: Optimize JImage decompressors #16556

8321620: Optimize JImage decompressors #16556

Conversation

Glavo commented Nov 8, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewing

Webrev

bridgekeeper bot commented Nov 8, 2023

openjdk bot commented Nov 8, 2023

Glavo commented Nov 8, 2023 • edited Loading

Glavo commented Nov 8, 2023

liach Nov 9, 2023

Choose a reason for hiding this comment

Glavo Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Glavo commented Nov 9, 2023 • edited Loading

minborg Nov 20, 2023

Choose a reason for hiding this comment

Glavo Nov 20, 2023

Choose a reason for hiding this comment

minborg Nov 20, 2023

Choose a reason for hiding this comment

Glavo Nov 20, 2023

Choose a reason for hiding this comment

cl4es Nov 20, 2023

Choose a reason for hiding this comment

cl4es commented Nov 20, 2023 • edited Loading

stsypanov Nov 21, 2023

Choose a reason for hiding this comment

Glavo commented Nov 23, 2023 • edited Loading

Glavo commented Dec 8, 2023

cl4es commented Dec 8, 2023

mlbridge bot commented Dec 8, 2023

Webrevs

bridgekeeper bot commented Jan 6, 2024

Glavo commented Jan 6, 2024

openjdk bot commented Jan 6, 2024

Glavo commented Jan 8, 2024

cl4es left a comment • edited Loading

Choose a reason for hiding this comment

Glavo commented Jan 12, 2024 • edited Loading

mlchung commented Jan 12, 2024

Glavo commented Jan 12, 2024

Glavo commented Jan 12, 2024

Glavo commented Nov 8, 2023 •

edited by openjdk bot

Loading

Glavo commented Nov 8, 2023 •

edited

Loading

Glavo Nov 9, 2023 •

edited

Loading

Glavo commented Nov 9, 2023 •

edited

Loading

cl4es commented Nov 20, 2023 •

edited

Loading

Glavo commented Nov 23, 2023 •

edited

Loading

cl4es left a comment •

edited

Loading

Glavo commented Jan 12, 2024 •

edited

Loading