8349378: Build splashscreen lib with SIZE optimization

[MBaesken]

The splashscreen lib is currently built with LOW optimization.
This might be fine because it is not very performance critical (and LOW is not really low when looking at the opt-flags used).
But building it with SIZE optimization makes it 10-20 % smaller on some platforms which helps to reduce image size.

current settings (LOW optimization) :

2.5M /aix_ppc64/jdk-opt/images/jdk/lib/libsplashscreen.so (not split debuginfo file on AIX currently)

468K /macosaarch64/jdk-opt/images/jdk/lib/libsplashscreen.dylib
1.6M /macosaarch64/jdk-opt/images/jdk/lib/libsplashscreen.dylib.dSYM
388K /macosintel64/jdk-opt/images/jdk/lib/libsplashscreen.dylib
1.5M /macosintel64/jdk-opt/images/jdk/lib/libsplashscreen.dylib.dSYM

368K /linux_aarch64/jdk-opt/images/jdk/lib/libsplashscreen.so
1.7M /linux_aarch64/jdk-opt/images/jdk/lib/libsplashscreen.debuginfo
376K /linux_alpine_x86_64/jdk-opt/images/jdk/lib/libsplashscreen.so
1.8M /linux_alpine_x86_64/jdk-opt/images/jdk/lib/libsplashscreen.debuginfo
500K /linux_ppc64le/jdk-opt/images/jdk/lib/libsplashscreen.so
1.7M /linux_ppc64le/jdk-opt/images/jdk/lib/libsplashscreen.debuginfo
364K /linux_x86_64/jdk-opt/images/jdk/lib/libsplashscreen.so
1.7M /linux_x86_64/jdk-opt/images/jdk/lib/libsplashscreen.debuginfo

new settings (SIZE optimization) :

2.1M /aix_ppc64/jdk-dev-opt/images/jdk/lib/libsplashscreen.so (not split debuginfo file on AIX currently)

404K /macosaarch64/jdk-dev-opt/images/jdk/lib/libsplashscreen.dylib
1.5M /macosaarch64/jdk-dev-opt/images/jdk/lib/libsplashscreen.dylib.dSYM
316K /macosintel64/jdk-dev-opt/images/jdk/lib/libsplashscreen.dylib
1.4M /macosintel64/jdk-dev-opt/images/jdk/lib/libsplashscreen.dylib.dSYM

372K /linux_aarch64/jdk-dev-opt/images/jdk/lib/libsplashscreen.so
1.5M /linux_aarch64/jdk-dev-opt/images/jdk/lib/libsplashscreen.debuginfo
304K /linux_alpine_x86_64/jdk-dev-opt/images/jdk/lib/libsplashscreen.so
1.5M /linux_alpine_x86_64/jdk-dev-opt/images/jdk/lib/libsplashscreen.debuginfo
376K /linux_ppc64le/jdk-dev-opt/images/jdk/lib/libsplashscreen.so
1.4M /linux_ppc64le/jdk-dev-opt/images/jdk/lib/libsplashscreen.debuginfo
304K /linux_x86_64/jdk-dev-opt/images/jdk/lib/libsplashscreen.so
1.4M /linux_x86_64/jdk-dev-opt/images/jdk/lib/libsplashscreen.debuginfo

On Linux aarch64 only the debuginfo shrinks but the lib stays about the same in size. Maybe -Os does not work as well on this platform.
Other UNIX platforms have a reduction by ~ 10-20 % .
For Windows, LOW and SIZE optimization have currently the same O - flags so no reduction.

Build times are the same on Windows (because LOW and SIZE are currently 'optimize for size').
On Linux x86_64 it seems that the build times of java.desktop native libs get slightly better but only very little.

time make java.desktop-libs-only JOBS=1

LOW (current)
real 5m38.074s
user 4m16.709s
sys 0m26.335s

real 5m29.081s
user 4m17.838s
sys 0m26.749s

real 5m25.496s
user 4m17.906s
sys 0m25.506s

SIZE for libsplashscreen
real 5m18.468s
user 4m13.024s
sys 0m25.129s

real 5m31.944s
user 4m12.922s
sys 0m26.333s

real 5m12.874s
user 4m12.253s
sys 0m25.206s

real has a bit of variance but user time is 4m16 / 4m17 for LOW and 4m12/4m13 for SIZE for libsplashscreen.

---

Progress

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

Issue

• JDK-8349378: Build splashscreen lib with SIZE optimization (Enhancement - P4)

Reviewers

• Erik Joelsson (@erikj79 - Reviewer)
• Phil Race (@prrace - Reviewer)
• Harshitha Onkar (@honkar-jdk - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23493

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mbaesken! 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]

@MBaesken This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8349378: Build splashscreen lib with SIZE optimization

Reviewed-by: erikj, prr, honkar

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 69 new commits pushed to the master branch:

• 7a0832f: 8349934: Wrong file regex for copyright header format check in .jcheck/conf
• 2a5d1da: 8349571: Remove JavaThreadFactory interface from SA
• 74e458b: 8349684: Remove SA core file tests from problem list for macosx-x64
• ba28119: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition
• c5ac3c4: 8249831: Test sun/security/mscapi/nonUniqueAliases/NonUniqueAliases.java is marked with @ignore
• 336d0d8: 8349926: [BACKOUT] Support static JDK in libfontmanager/freetypeScaler.c
• 4b463ee: 8342103: C2 compiler support for Float16 type and associated scalar operations
• 332d87c: 8349859: Support static JDK in libfontmanager/freetypeScaler.c
• 73e1780: 8349836: G1: Improve group prediction log message
• ed17c55: 8349145: Make Class.getProtectionDomain() non-native
• ... and 59 more: https://git.openjdk.org/jdk/compare/dd8720e90dc5475afd4ccc7321bb5cd97282e101...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@MBaesken The following labels will be automatically applied to this pull request:

• build
• client

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

[mlbridge]

Webrevs

• 00: Full (51f4cab7)

[erikj79]

/reviewers 2

I think this looks good, but someone from client should probably also weigh in.

[openjdk]

@erikj79
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[MBaesken]

For the record, those are the lib sizes on Windows x86_64, VS2019 (LOW and SIZE uses currently the same opt flag, so same sizes)

204K /windows_x86_64/jdk-opt/images/jdk/bin/splashscreen.dll
1.4M /windows_x86_64/jdk-opt/images/jdk/bin/splashscreen.dll.pdb
352K /windows_x86_64/jdk-opt/images/jdk/bin/splashscreen.dll.stripped.pdb

[MBaesken]

Hi @erikj79 thanks for the review !

@magicus , @mrserb , @honkar-jdk any comments?

[magicus]

What tests have you run?

[MBaesken]

What tests have you run?

Had this in our internal test queue with jtreg / JCK plus some additional SAP internal tests. No issues seen.
However I think I have to run some more manual tests on client like setups.

Unfortunately my Linux terminal server does not like the awt jtreg tests (with and without my patch) , so I have to find something else.

[honkar-jdk]

@MBaesken

However I think I have to run some more manual tests on client like setups.

SplashScreen clientlib tests are located - test/jdk/java/awt/SplashScreen. Most of them are manual tests invoked by shell script and it is easier to run the whole test folder. I can run the tests on your behalf to check if the tests work fine with the fix.

What are we specifically looking for in the tests with this fix? No degradation in test startup and execution time?

[prrace]

I note that AIX seems to be the biggest beneficiary here. It doesn't seem to be a big deal for anything else.

I think what we are looking for in testing is no functional regression plus minimal perf impact on startup.
But I don't think any of our jtreg tests are set up to measure the perf so can only ensure the functional.
It would be good to check that as mentioned.
And I don't remember anyone ever testing perf of splash since the point of splash is to mask the much bigger hit of booting up the JVM .. so I doubt we'll be able to test it today.
But I'll make a note to self to ask the people who do perf testing to think about this, or perhaps tell me "gosh Phil, we've been testing that forever, didn't you know" ?

[MBaesken]

I note that AIX seems to be the biggest beneficiary here. It doesn't seem to be a big deal for anything else.

Please note that AIX has the debuginfo in the same binary (.so file), unlike e.g. Linux.
If you look at the size reduction of the debuginfo files on Linux, this goes down from 1.7M -> 1.4M on Linux x86_64 so it is also quite a good reduction.

[MBaesken]

@MBaesken

However I think I have to run some more manual tests on client like setups.

SplashScreen clientlib tests are located - test/jdk/java/awt/SplashScreen. Most of them are manual tests invoked by shell script and it is easier to run the whole test folder. I can run the tests on your behalf to check if the tests work fine with the fix.

What are we specifically looking for in the tests with this fix? No degradation in test startup and execution time?

If you find the time to run those, would be great. And yes a noticeable degradation would be important to know.

[prrace]

Looks OK to me. But Harshitha is going to do a bit of testing, so wait for her.

[honkar-jdk]

@MBaesken
SplashScreen tests work as expected on all platforms. No regressions observed.

On Linux aarch64 only the debuginfo shrinks but the lib stays about the same in size. Maybe -Os does not work as well on this platform.
Other UNIX platforms have a reduction by ~ 10-20 % .
For Windows, LOW and SIZE optimization have currently the same O - flags so no reduction.

With optimization=SIZE, libsplashscreen file sizes are less on linux and macOS as expected. On Windows, I expected it to be almost the same size (if not smaller) but the .dll files on Windows were slightly bigger with opt=SIZE than with opt=LOW for me (Win 11 x86-64).

Details below.

macOS
--------
408K Feb 10 14:59 libsplashscreen.dylib
96B Feb 10 15:00 libsplashscreen.dylib.dSYM

With opt=SIZE
344K Feb 11 10:38 libsplashscreen.dylib
96B Feb 11 10:39 libsplashscreen.dylib.dSYM

Linux
-----
1.7M Feb 10 16:39 libsplashscreen.debuginfo
364K Feb 10 16:39 libsplashscreen.so

With opt=SIZE
1.4M Feb 11 10:58 libsplashscreen.debuginfo
293K Feb 11 10:58 libsplashscreen.so

Windows
----------
201K Feb  7 15:37 splashscreen.dll
177K Feb  7 15:37 splashscreen.dll.map
1.6M Feb  7 15:37 splashscreen.dll.pdb

With opt=SIZE
547K Feb 11 11:31 splashscreen.dll
281K Feb 11 11:31 splashscreen.dll.map
1.7M Feb 11 11:31 splashscreen.dll.pdb

[MBaesken]

Thanks for the testing.

but the .dll files on Windows were slightly bigger with opt=SIZE than with opt=LOW for me

This is very strange.
It is for me (using vs2022-17.6.5)
208K splashscreen.dll and 1.6M for splashscreen.dll.pdb (for both SIZE and LOW)

And this is expected for an opt build because it is -O1 for both SIZE and NORM/LOW optimization , see

https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L342

Could you maybe check in your build log that it is O1 for both? Do you see any other differences in your Windows/VS build log ?

[MBaesken]

@honkar-jdk do you have the latest/newest jdk head, especially this change is important
8349214: Improve size optimization flags for MSVC builds
40603a5

Before the SIZE opt flags for MSVC were bad.

[honkar-jdk]

@honkar-jdk do you have the latest/newest jdk head, especially this change is important 8349214: Improve size optimization flags for MSVC builds 40603a5

Before the SIZE opt flags for MSVC were bad.

Looks like I didn't have this changeset in my repo.
With the size opt flag of -O1 , I see similar .dll file sizes as before. LGTM now.

201K Feb 12 11:29 splashscreen.dll
177K Feb 12 11:30 splashscreen.dll.map
1.6M Feb 12 11:30 splashscreen.dll.pdb

[MBaesken]

Thanks for looking into it !
Seems I need another re review ?

[MBaesken]

/integrate

[openjdk]

Going to push as commit c2fc947.
Since your change was applied there have been 78 commits pushed to the master branch:

• 5b75ff7: 8349907: jdk.tools.jlink.internal.plugins.ZipPlugin does not close the Deflater in exceptional cases
• add3cd1: 8348600: Update PipeWire to 1.3.81
• efc597b: 8349564: Clean warnings found in jpackage tests when building them with -Xlint:all
• 29202d1: 8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434
• 24b7f81: 8349783: g1RemSetSummary.cpp:344:68: runtime error: member call on null pointer of type 'struct G1HeapRegion'
• a637ccf: 8349851: RISC-V: Call VM leaf can use movptr2
• adc3f53: 8349787: java/lang/Thread/virtual/ThreadPollOnYield.java#default passes unexpectedly without libVThreadPinner.so
• 55097dd: 8344802: Crash in StubRoutines::verify_mxcsr with -XX:+EnableX86ECoreOpts and -Xcheck:jni
• b8576eb: 6562489: Font-Renderer should ignore invisible characters \u2062 and \u2063
• 7a0832f: 8349934: Wrong file regex for copyright header format check in .jcheck/conf
• ... and 68 more: https://git.openjdk.org/jdk/compare/dd8720e90dc5475afd4ccc7321bb5cd97282e101...master

Your commit was automatically rebased without conflicts.

[openjdk]

@MBaesken Pushed as commit c2fc947.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.