Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default
On Thu, 9 May 2024 17:05:40 GMT, Ioi Lam wrote: >> On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, >> and (when run on Mac M1 hardware) 16K. >> >> Since forgetting to specify `--enable-compatible-cds-alignment` is a common >> error that is only noticed when running the produced JVM on hardware with >> different page size, we propose to enable that option by default on Linux >> aarch64. The cost is a moderate increase in CDS archive size (about 300K). >> >> I tested this patch on: >> - x64 Linux >> - x64 Linux, crossbuilding to aarch64 >> - building natively on aarch64 Linux > > Looks good to me. I will close my JBS issue as a duplicate of this issue. > > Thanks Okay, thanks @iklam - PR Comment: https://git.openjdk.org/jdk/pull/19142#issuecomment-2103841320
Integrated: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default
On Wed, 8 May 2024 15:14:16 GMT, Thomas Stuefe wrote: > On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, > and (when run on Mac M1 hardware) 16K. > > Since forgetting to specify `--enable-compatible-cds-alignment` is a common > error that is only noticed when running the produced JVM on hardware with > different page size, we propose to enable that option by default on Linux > aarch64. The cost is a moderate increase in CDS archive size (about 300K). > > I tested this patch on: > - x64 Linux > - x64 Linux, crossbuilding to aarch64 > - building natively on aarch64 Linux This pull request has now been integrated. Changeset: d2d37c91 Author:Thomas Stuefe URL: https://git.openjdk.org/jdk/commit/d2d37c913e5b55f7aec2c7a6b5a2328348ded223 Stats: 12 lines in 1 file changed: 11 ins; 0 del; 1 mod 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default Reviewed-by: aph, iklam - PR: https://git.openjdk.org/jdk/pull/19142
Re: RFR: 8331952: --enable-compatible-cds-alignment should be enabled by default
On Thu, 9 May 2024 01:07:26 GMT, xiaotaonan wrote: > --enable-compatible-cds-alignment should be enabled by default @lgxbslgx - PR Comment: https://git.openjdk.org/jdk/pull/19150#issuecomment-2103672340
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. @magicus? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2103667760
Re: RFR: 8330182: Start of release updates for JDK 24 [v4]
> Get JDK 24 underway. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - Adjust test for deprecated options. - Merge branch 'master' into JDK-8330188 - Update deprecated options test. - Merge branch 'master' into JDK-8330188 - Merge branch 'master' into JDK-8330188 - Merge branch 'master' into JDK-8330188 - Correct release date as observed in review feedback. - Improve javadoc of class file update. - JDK-8330182: Start of release updates for JDK 24 JDK-8330183: Add SourceVersion.RELEASE_24 JDK-8330184: Add source 24 and target 24 to javac - Changes: https://git.openjdk.org/jdk/pull/18787/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18787=03 Stats: 107 lines in 37 files changed: 46 ins; 7 del; 54 mod Patch: https://git.openjdk.org/jdk/pull/18787.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18787/head:pull/18787 PR: https://git.openjdk.org/jdk/pull/18787
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v9]
> Performance. Before: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt ScoreError Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6443.934 ± 6.491 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6152.979 ± 4.954 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1895.410 ± 36.979 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1878.955 ± 45.487 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1357.810 ± 26.584 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1352.119 ± 23.547 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± > 10.970 ops/s > > Performance, no intrinsic: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6529.839 ± 42.420 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6199.747 ± 133.566 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1973.676 ± 54.071 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1932.127 ± 35.920 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1355.788 ± 29.858 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1346.523 ± 28.722 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± > 10.591 ops/s > > Performance, **with intrinsics*... Volodymyr Paprotski has updated the pull request incrementally with one additional commit since the last revision: whitespace - Changes: - all: https://git.openjdk.org/jdk/pull/18583/files - new: https://git.openjdk.org/jdk/pull/18583/files/83b21310..8cd095dd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18583=08 - incr: https://webrevs.openjdk.org/?repo=jdk=18583=07-08 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18583.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18583/head:pull/18583 PR: https://git.openjdk.org/jdk/pull/18583
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v7]
On Thu, 9 May 2024 23:36:03 GMT, Anthony Scarpino wrote:
>> Volodymyr Paprotski has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> whitespace
>
> src/java.base/share/classes/sun/security/ec/ECOperations.java line 701:
>
>> 699: if (!m.equals(v)) {
>> 700: java.util.HexFormat hex =
>> java.util.HexFormat.of();
>> 701: throw new RuntimeException();
>
> I think your cleanup went to far. You should have some message saying they
> are not equal and if you don't want to print hex, remove getting an instance.
I put the message back.. I removed it 'half'-intentionally; Was comparing
against the original version and it didn't have any details, thought maybe
should follow suit. But I did find this message helpful, so its back.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1596116606
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v8]
> Performance. Before: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt ScoreError Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6443.934 ± 6.491 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6152.979 ± 4.954 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1895.410 ± 36.979 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1878.955 ± 45.487 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1357.810 ± 26.584 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1352.119 ± 23.547 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± > 10.970 ops/s > > Performance, no intrinsic: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6529.839 ± 42.420 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6199.747 ± 133.566 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1973.676 ± 54.071 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1932.127 ± 35.920 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1355.788 ± 29.858 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1346.523 ± 28.722 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± > 10.591 ops/s > > Performance, **with intrinsics*... Volodymyr Paprotski has updated the pull request incrementally with one additional commit since the last revision: add message back - Changes: - all: https://git.openjdk.org/jdk/pull/18583/files - new: https://git.openjdk.org/jdk/pull/18583/files/1ecfdc44..83b21310 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18583=07 - incr: https://webrevs.openjdk.org/?repo=jdk=18583=06-07 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18583.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18583/head:pull/18583 PR: https://git.openjdk.org/jdk/pull/18583
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v7]
On Thu, 9 May 2024 22:23:02 GMT, Volodymyr Paprotski wrote:
>> Performance. Before:
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt ScoreError Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt3 6443.934 ± 6.491 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt3 6152.979 ± 4.954 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt3 1895.410 ± 36.979 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt3 1878.955 ± 45.487 ops/s
>> Benchmark(algorithm)
>> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units
>> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1357.810 ± 26.584 ops/s
>> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1352.119 ± 23.547 ops/s
>> Benchmark (isMontBench) Mode Cnt Score
>> Error Units
>> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ±
>> 10.970 ops/s
>>
>> Performance, no intrinsic:
>>
>> Benchmark(algorithm) (dataSize) (keyLength)
>> (provider) Mode Cnt Score Error Units
>> SignatureBench.ECDSA.signSHA256withECDSA1024 256
>> thrpt3 6529.839 ± 42.420 ops/s
>> SignatureBench.ECDSA.signSHA256withECDSA 16384 256
>> thrpt3 6199.747 ± 133.566 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA1024 256
>> thrpt3 1973.676 ± 54.071 ops/s
>> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
>> thrpt3 1932.127 ± 35.920 ops/s
>> Benchmark(algorithm)
>> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units
>> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1355.788 ± 29.858 ops/s
>> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH
>> 256 EC thrpt3 1346.523 ± 28.722 ops/s
>> Benchmark (isMontBench) Mode Cnt Score
>> Error Units
>> PolynomialP256Bench.benchMultiply true thrpt3 1919.57...
>
> Volodymyr Paprotski has updated the pull request incrementally with one
> additional commit since the last revision:
>
> whitespace
src/java.base/share/classes/sun/security/ec/ECOperations.java line 701:
> 699: if (!m.equals(v)) {
> 700: java.util.HexFormat hex =
> java.util.HexFormat.of();
> 701: throw new RuntimeException();
I think your cleanup went to far. You should have some message saying they are
not equal and if you don't want to print hex, remove getting an instance.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1596099182
Integrated: 8332008: Enable issuestitle check
On Thu, 9 May 2024 20:53:59 GMT, Zhao Song wrote: > As requested in [SKARA-2170](https://bugs.openjdk.org/browse/SKARA-2170) and > [SKARA-2248](https://bugs.openjdk.org/browse/SKARA-2248), now Skara bot is > able to warn on trailing periods or leading lowercase letter in issue titles. > I am going to update the jcheck configuration to configure issuestitle check > as a warning. Given that it's just a warning, it wouldn't block integration. This pull request has now been integrated. Changeset: d47a4e9f Author:Zhao Song Committer: Erik Joelsson URL: https://git.openjdk.org/jdk/commit/d47a4e9f63a9414b90e09514bc26f6f7142ad49f Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8332008: Enable issuestitle check Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/19161
Re: RFR: 8332008: Enable issuestitle check
On Thu, 9 May 2024 20:53:59 GMT, Zhao Song wrote: > As requested in [SKARA-2170](https://bugs.openjdk.org/browse/SKARA-2170) and > [SKARA-2248](https://bugs.openjdk.org/browse/SKARA-2248), now Skara bot is > able to warn on trailing periods or leading lowercase letter in issue titles. > I am going to update the jcheck configuration to configure issuestitle check > as a warning. Given that it's just a warning, it wouldn't block integration. Thank you for the review! - PR Comment: https://git.openjdk.org/jdk/pull/19161#issuecomment-2103553031
Re: RFR: 8332008: Enable issuestitle check
On Thu, 9 May 2024 20:53:59 GMT, Zhao Song wrote: > As requested in [SKARA-2170](https://bugs.openjdk.org/browse/SKARA-2170) and > [SKARA-2248](https://bugs.openjdk.org/browse/SKARA-2248), now Skara bot is > able to warn on trailing periods or leading lowercase letter in issue titles. > I am going to update the jcheck configuration to configure issuestitle check > as a warning. Given that it's just a warning, it wouldn't block integration. Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19161#pullrequestreview-2048931426
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v7]
> Performance. Before: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt ScoreError Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6443.934 ± 6.491 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6152.979 ± 4.954 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1895.410 ± 36.979 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1878.955 ± 45.487 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1357.810 ± 26.584 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1352.119 ± 23.547 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± > 10.970 ops/s > > Performance, no intrinsic: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6529.839 ± 42.420 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6199.747 ± 133.566 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1973.676 ± 54.071 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1932.127 ± 35.920 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1355.788 ± 29.858 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1346.523 ± 28.722 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± > 10.591 ops/s > > Performance, **with intrinsics*... Volodymyr Paprotski has updated the pull request incrementally with one additional commit since the last revision: whitespace - Changes: - all: https://git.openjdk.org/jdk/pull/18583/files - new: https://git.openjdk.org/jdk/pull/18583/files/8ff243a2..1ecfdc44 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18583=06 - incr: https://webrevs.openjdk.org/?repo=jdk=18583=05-06 Stats: 753 lines in 9 files changed: 303 ins; 101 del; 349 mod Patch: https://git.openjdk.org/jdk/pull/18583.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18583/head:pull/18583 PR: https://git.openjdk.org/jdk/pull/18583
RFR: 8332008: Enable issuestitle check
As requested in [SKARA-2170](https://bugs.openjdk.org/browse/SKARA-2170) and [SKARA-2248](https://bugs.openjdk.org/browse/SKARA-2248), now Skara bot is able to warn on trailing periods or leading lowercase letter in issue titles. I am going to update the jcheck configuration to configure issuestitle check as a warning. Given that it's just a warning, it wouldn't block integration. - Commit messages: - JDK-8332008 Changes: https://git.openjdk.org/jdk/pull/19161/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19161=00 Issue: https://bugs.openjdk.org/browse/JDK-8332008 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19161.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19161/head:pull/19161 PR: https://git.openjdk.org/jdk/pull/19161
Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default
On Wed, 8 May 2024 15:14:16 GMT, Thomas Stuefe wrote: > On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, > and (when run on Mac M1 hardware) 16K. > > Since forgetting to specify `--enable-compatible-cds-alignment` is a common > error that is only noticed when running the produced JVM on hardware with > different page size, we propose to enable that option by default on Linux > aarch64. The cost is a moderate increase in CDS archive size (about 300K). > > I tested this patch on: > - x64 Linux > - x64 Linux, crossbuilding to aarch64 > - building natively on aarch64 Linux Looks good to me. I will close my JBS issue as a duplicate of this issue. Thanks - Marked as reviewed by iklam (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19142#pullrequestreview-2048421270
Integrated: 8327476: Upgrade JLine to 3.26.1
On Wed, 6 Mar 2024 21:20:50 GMT, Jan Lahoda wrote: > This is a patch that: > a) upgrades the JLine inside the JDK to 3.25.1 > b) since the new version of JLine has a FFM backend, our custom native > backends are removed, and replaced with the FFM backend > > Some changes had to be made to the original JLine in order to fit into the > JDK. Most of them were already done for the previous version (repackaging, > avoiding non-ASCII characters, commenting out logging, adding ability to > modify to wrap the InputStream used by the terminal), and have only been > transferred to the new one. The main two new changes are: > - fixes to the FFM backend, so that it works on Linux and JDK 22. These have > been proposed to JLine itself: https://github.com/jline/jline3/pull/945 > - disabling the `NativeFileDescriptorCreator`, as I believe we don't need it, > and cannot make it work easily > > There's a full patch between the > `src/jdk.internal.le/share/classes/jdk/internal/org` and the merged content > of the corresponding sources of these original JLine sub-projects: > https://github.com/jline/jline3/tree/jline-parent-3.25.1/reader > https://github.com/jline/jline3/tree/jline-parent-3.25.1/terminal-ffm > https://github.com/jline/jline3/tree/jline-parent-3.25.1/terminal > the patch is here: > https://cr.openjdk.org/~jlahoda/8327476/jline-3.25.1-upgrade.diff > > I've also cleaned the patch a little removing most of the changes for the > rename. The result is here: > https://cr.openjdk.org/~jlahoda/8327476/jline-3.25.1-upgrade-significant.diff This pull request has now been integrated. Changeset: aaa90b30 Author:Jan Lahoda URL: https://git.openjdk.org/jdk/commit/aaa90b3005c85852971203ce6feb88e7091e167b Stats: 13181 lines in 144 files changed: 5784 ins; 5349 del; 2048 mod 8327476: Upgrade JLine to 3.26.1 Reviewed-by: ihse, vromero - PR: https://git.openjdk.org/jdk/pull/18142
Re: RFR: 8331952: --enable-compatible-cds-alignment should be enabled by default
On Thu, 9 May 2024 01:07:26 GMT, xiaotaonan wrote: > --enable-compatible-cds-alignment should be enabled by default Build change looks ok. I think this should be approved by someone from hotspot as well. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19150#pullrequestreview-2047945852
Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default
On Thu, 9 May 2024 05:04:47 GMT, Thomas Stuefe wrote: >> On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, >> and (when run on Mac M1 hardware) 16K. >> >> Since forgetting to specify `--enable-compatible-cds-alignment` is a common >> error that is only noticed when running the produced JVM on hardware with >> different page size, we propose to enable that option by default on Linux >> aarch64. The cost is a moderate increase in CDS archive size (about 300K). >> >> I tested this patch on: >> - x64 Linux >> - x64 Linux, crossbuilding to aarch64 >> - building natively on aarch64 Linux > > Does anyone else have an opinion on this matter? The question is: > > "Should we enable cds-compatible alignment in the configure options by > default generally, not only for Linux aarch64" ? > > This would, in addition to the proposed change affecting Linux on aarch64, > also affect MacOS x64. JVMs built there would be able to run, by default, in > Rosetta. At the price of sligthly increased archive size of a dozen KB or so. > > If nobody objects, I will do that since I like the simplicity of it. > @tstuefe We already have an issue to reset the default to true (raised by > @iklam) > > https://bugs.openjdk.org/browse/JDK-8331952 Okay. I see it was created yesterday evening. @iklam how do you want to handle this? Do you want to proceed with 8331952 and I close this as duplicate? Or vice versa? - PR Comment: https://git.openjdk.org/jdk/pull/19142#issuecomment-2102339405
Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default
On Thu, 9 May 2024 05:04:47 GMT, Thomas Stuefe wrote: >> On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, >> and (when run on Mac M1 hardware) 16K. >> >> Since forgetting to specify `--enable-compatible-cds-alignment` is a common >> error that is only noticed when running the produced JVM on hardware with >> different page size, we propose to enable that option by default on Linux >> aarch64. The cost is a moderate increase in CDS archive size (about 300K). >> >> I tested this patch on: >> - x64 Linux >> - x64 Linux, crossbuilding to aarch64 >> - building natively on aarch64 Linux > > Does anyone else have an opinion on this matter? The question is: > > "Should we enable cds-compatible alignment in the configure options by > default generally, not only for Linux aarch64" ? > > This would, in addition to the proposed change affecting Linux on aarch64, > also affect MacOS x64. JVMs built there would be able to run, by default, in > Rosetta. At the price of sligthly increased archive size of a dozen KB or so. > > If nobody objects, I will do that since I like the simplicity of it. @tstuefe We already have an issue to reset the default to true (raised by @iklam) https://bugs.openjdk.org/browse/JDK-8331952 - PR Comment: https://git.openjdk.org/jdk/pull/19142#issuecomment-2102202184
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. Also @magicus, what is the typical path passed to --with-binutils like on Windows? Something like --with-binutils=/c/Users/vertig0/Downloads/binutils-2.42 doesn't work correctly, since the include path to dis-asm.h would then become `#include "/c/Users/vertig0/Downloads/binutils-2.42/include/dis-asm.h"` Which causes a configure check to fail on the compile stage since gcc cannot recognise the MINGW-esque /c/ as a drive, and then causes configure to erroneously report binutils as using the Old API when it's in fact using the New API. --with-binutils=C:/Users/vertig0/Downloads/binutils-2.42 on the other hand works as expected. Should there be a fixup for the path there so gcc can recognise it properly? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2102146604
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
> WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Add check for compiler in configure - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis - Changes: https://git.openjdk.org/jdk/pull/18915/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18915=01 Stats: 334 lines in 19 files changed: 154 ins; 25 del; 155 mod Patch: https://git.openjdk.org/jdk/pull/18915.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18915/head:pull/18915 PR: https://git.openjdk.org/jdk/pull/18915
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. @magicus I think I might need some help here. Currently all the Cygwin stuff is gated behind ifeq ($(TOOLCHAIN_TYPE), microsoft) checks... should they be behind isBuildOsEnv cygwin checks instead? I don't know how to add support for Cygwin gcc for most of hsdis, since it is quite different from the more modern gcc distributions that are found in places like MSYS2 and MINGW64. But most of the existing logic seems to accomodate more for the Microsoft compiler than it is concerned about the OS environment being used, and for this reason I can't tell which of the 2 checks to use for the existing hack that switches from microsoft to gcc. Also, gcc doesn't require FIXPATH, but Microsoft does, but I don't want to check for microsoft inside TOOLCHAIN_FIND_COMPILER, what should I do instead to ensure gcc doesn't get FIXPATH while Microsoft does? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2097196121
