Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable
On Mon, 21 Feb 2022 12:16:53 GMT, Andrey Turbanov wrote: > Method `isAssignableFrom` is opposite: it brings unnecessary complexity in > the code. And it's easy to confuse orders of parameters. Even JBS confirms > that: Maybe we should add `Class::isSubclassOf(Class that)` that performs `that.isAssignableFrom(this)`: - PR: https://git.openjdk.java.net/jdk/pull/7061
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
On Tue, 22 Mar 2022 03:31:16 GMT, Fei Yang wrote:
>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the
>> https://openjdk.java.net/projects/riscv-port branch.
>>
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are
>> also carried out regularly. So it should be good enough to run most Java
>> programs.
>>
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains two additional commits since
> the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into JDK-8276799
> - 8276799: Implementation of JEP 422: Linux/RISC-V Port
Hi,
I've looked at everything that is not a RISC-V specific file, except for the C1
changes as the compiler folk will need to approve those.
Some copyrights will need updating to 2022 on the Oracle copyright line please.
I have flagged one issue in regard to C++ atomics - see below.
Thanks,
David
make/autoconf/libraries.m4 line 152:
> 150: fi
> 151:
> 152: # Programs which use C11 or C++11 atomics, like #include ,
Use of C++ atomics is not allowed in hotspot code base. See the style guide:
https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md
That said, I don't see any actual use of C++ atomics. ??
-
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy wrote: > Small refactoring to use sealed classes in the MethodHandle implementation > hierarchy. > > DelegatingMethodHandle is non-sealed rather than sealed since it has two > subclasses, one defined as a nested class and only a separate type in the > same package. The permits clause does not allow listed those two kinds of > subclasses. > > Please also review the corresponding CSR > https://bugs.openjdk.java.net/browse/JDK-8283434 **javac** should be changed to allow fully qualified access to private inner classes in the `permits` clause of an enclosing class. - PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: 8283411: InflaterInputStream holds on to a temporary byte array of 512 bytes [v2]
On Sun, 20 Mar 2022 14:01:50 GMT, Jaikiran Pai wrote: >> src/java.base/share/classes/java/util/zip/InflaterInputStream.java line 206: >> >>> 204: int max = (int)Math.min(n, Integer.MAX_VALUE); >>> 205: int total = 0; >>> 206: byte[] b = new byte[512]; >> >> n may be less than 512 so maybe the temporary array can be of length >> Math.min(max, 512). > > That's a good idea. I just pushed a update to this PR with this suggested > change. Plus updated the copyright year too. I ran tier1, tier2 and tier3 tests with this change and they completed successfully. Alan, does the current state of the PR look fine to you? Should I go ahead with the merge? - PR: https://git.openjdk.java.net/jdk/pull/7875
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Mon, 21 Mar 2022 18:17:06 GMT, Mandy Chung wrote:
> > $ javac com/acme/*.java
> > is fine. Am I missing something?
>
> If you make P.Q private, it will fail the compilation.
>
Yes, I got that part. I did make a comment earlier ("Two of those subclasses
are nested "private" in another class and hence cannot be referred from
DelegatingMethodHandle's permits clause, right?"). My example was in response
to "I did try to name the nested type in the permits clause, and it was not
accepted by javac." comment by @jddarcy. I wondered if there is any other issue
with permits beyond private access of those 2 nested classes.
> To make `DelegatingMethodHandle` a sealed class, we could make
> `AsVarargsCollector` and `WrappedMember` package-private classes.
right.
-
PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]
> This PR implements JEP 422: Linux/RISC-V Port [1].
> The PR starts as a squashed merge of the
> https://openjdk.java.net/projects/riscv-port branch.
>
> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also
> carried out regularly. So it should be good enough to run most Java programs.
>
> [1] https://openjdk.java.net/jeps/422
Fei Yang has updated the pull request with a new target base due to a merge or
a rebase. The incremental webrev excludes the unrelated changes brought in by
the merge/rebase. The pull request contains two additional commits since the
last revision:
- Merge remote-tracking branch 'upstream/master' into JDK-8276799
- 8276799: Implementation of JEP 422: Linux/RISC-V Port
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/6294/files
- new: https://git.openjdk.java.net/jdk/pull/6294/files/33402035..a144cd20
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=6294=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=6294=00-01
Stats: 2517 lines in 698 files changed: 1371 ins; 865 del; 281 mod
Patch: https://git.openjdk.java.net/jdk/pull/6294.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8279508: Auto-vectorize Math.round API [v15]
On Tue, 22 Mar 2022 02:52:07 GMT, Jatin Bhateja wrote: >>> A read from constant table will incur minimum of L1I access penalty to >>> access code blob or at worst even more if data is not present in first >>> level cache >> >> But your approach comes at a cost of frontend bandwidth and port contention, >> which imo are more important than latency in this case since a constant load >> does not prolong dependency chains. A load has very good throughput so it is >> often performant unless the load depends on its input (the memory location >> or the registers used for address calculation). Thanks > > Thanks for going into details, multicycle memory load will also defer > dispatch of dependent instructions to execution port, port congestion becomes > bottleneck when multiple ready instructions cannot be issued due to lack of > execution resource or throughput constraints imposed by instruction, but a > single cycle dependency chain may still win over latency due to pending > memory operations. I think I get it now, thanks a lot for your detailed explanation. - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: 8279508: Auto-vectorize Math.round API [v15]
On Tue, 22 Mar 2022 01:55:38 GMT, Quan Anh Mai wrote: >> A read from constant table will incur minimum of L1I access penalty to >> access code blob or at worst even more if data is not present in first level >> cache. Change was done for replace vpbroadcastd with vbroadcastss because of >> two reasons. >> 1) vbroadcastss works at AVX=1 level where as vpbroadcastd need AVX2 >> feature. >> 2) We can avoid extra cycle penalty due to two domain switchovers (FP -> INT >> and then from INT-> FP). > >> A read from constant table will incur minimum of L1I access penalty to >> access code blob or at worst even more if data is not present in first level >> cache > > But your approach comes at a cost of frontend bandwidth and port contention, > which imo are more important than latency in this case since a constant load > does not prolong dependency chains. A load has very good throughput so it is > often performant unless the load depends on its input (the memory location or > the registers used for address calculation). Thanks Thanks for going into details, multicycle memory load will also defer dispatch of dependent instructions to execution port, port congestion becomes bottleneck when multiple ready instructions cannot be issued due to lack of execution resource or throughput constraints imposed by instruction, but a single cycle dependency chain may still win over latency due to pending memory operations. - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: 8282429: StringBuilder/StringBuffer.toString() skip compressing for UTF16 strings [v3]
On Wed, 9 Mar 2022 08:33:36 GMT, Xin Liu wrote: >> If AbstractStringBuilder only grow, the inflated value which has been >> encoded in UTF16 can't be compressed. >> toString() can skip compression in this case. This can save an >> ArrayAllocation in StringUTF16::compress(). >> >> java.io.BufferedRead::readLine() is a case that StringBuilder grows only. >> >> In microbench, we expect to see that allocation/op reduces 20%. The initial >> capacity of StringBuilder is S in bytes. When it encounters the 1st >> character that can't be encoded in LATIN1, it inflates and allocate a new >> array of 2*S. `toString()` will try to compress that value so it need to >> allocate S bytes. The last step allocates 2*S bytes because it has to copy >> the string. so it requires to allocate 5 * S bytes in total. By skipping >> the failed compression, it only allocates 4 * S bytes. that's 20%. In real >> execution, we observe 16% allocation reduction, tracked by JMH GC profiler >> `gc.alloc.rate.norm `. I think it's because HotSpot can't track all >> allocations. >> >> Not only allocation drops, the runtime performance(ns/op) also increases >> from 3.34% to 18.91%. >> >> Before: >> >> $$make test >> TEST="micro:org.openjdk.bench.java.lang.StringBuilders.toStringWithMixedChars" >> MICRO="OPTIONS=-prof gc -gc true -o before.log -jvm >> $HOME/Devel/jdk_baseline/bin/java" >> >> Benchmark >> (MIXED_SIZE) Mode Cnt Score Error Units >> StringBuilders.toStringWithMixedChars >> 128 avgt 15 649.846 ± 76.291 ns/op >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate >> 128 avgt 15 872.855 ± 128.259 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >> 128 avgt 15 880.121 ± 0.050B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >> 128 avgt 15 707.730 ± 194.421 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >> 128 avgt 15 706.602 ± 94.504B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >> 128 avgt 15 0.001 ± 0.002 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >> 128 avgt 15 0.001 ± 0.001B/op >> StringBuilders.toStringWithMixedChars:·gc.count >> 128 avgt 15 113.000counts >> StringBuilders.toStringWithMixedChars:·gc.time >> 128 avgt 1585.000ms >> StringBuilders.toStringWithMixedChars >> 256 avgt 15 1316.652 ± 112.771 ns/op >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate >> 256 avgt 15 800.864 ± 76.869 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >> 256 avgt 15 1648.288 ± 0.162B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >> 256 avgt 15 599.736 ± 174.001 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >> 256 avgt 15 1229.669 ± 318.518B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >> 256 avgt 15 0.001 ± 0.001 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >> 256 avgt 15 0.001 ± 0.002B/op >> StringBuilders.toStringWithMixedChars:·gc.count >> 256 avgt 15 133.000counts >> StringBuilders.toStringWithMixedChars:·gc.time >> 256 avgt 1592.000ms >> StringBuilders.toStringWithMixedChars >>1024 avgt 15 5204.303 ± 418.115 ns/op >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate >>1024 avgt 15 768.730 ± 72.945 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >>1024 avgt 15 6256.844 ± 0.358B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >>1024 avgt 15 655.852 ± 121.602 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >>1024 avgt 15 5315.265 ± 578.878B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >>1024 avgt 15 0.002 ± 0.002 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >>1024 avgt 15 0.014 ± 0.011B/op >> StringBuilders.toStringWithMixedChars:·gc.count >>1024 avgt 1596.000counts >> StringBuilders.toStringWithMixedChars:·gc.time
Re: RFR: 8279508: Auto-vectorize Math.round API [v15]
On Mon, 21 Mar 2022 18:25:36 GMT, Jatin Bhateja wrote: > A read from constant table will incur minimum of L1I access penalty to access > code blob or at worst even more if data is not present in first level cache But your approach comes at a cost of frontend bandwidth and port contention, which imo are more important than latency in this case since a constant load does not prolong dependency chains. A load has very good throughput so it is often performant unless the load depends on its input (the memory location or the registers used for address calculation). Thanks - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: JDK-8283480: Make AbstractStringBuilder sealed
On Tue, 22 Mar 2022 00:01:59 GMT, Joe Darcy wrote: > As part of updating the core libraries to be sealed, the package-access > AbstractStringBuilder, implementation superclass of StringBuilder and > StringBuffer, can be marked as sealed with those two subclasses on its > permits list. LGTM - Marked as reviewed by jlaskey (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7898
Re: RFR: JDK-8283480: Make AbstractStringBuilder sealed
On Tue, 22 Mar 2022 00:01:59 GMT, Joe Darcy wrote: > As part of updating the core libraries to be sealed, the package-access > AbstractStringBuilder, implementation superclass of StringBuilder and > StringBuffer, can be marked as sealed with those two subclasses on its > permits list. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7898
Re: RFR: JDK-8283480: Make AbstractStringBuilder sealed
On Tue, 22 Mar 2022 00:01:59 GMT, Joe Darcy wrote: > As part of updating the core libraries to be sealed, the package-access > AbstractStringBuilder, implementation superclass of StringBuilder and > StringBuffer, can be marked as sealed with those two subclasses on its > permits list. Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7898
RFR: JDK-8283480: Make AbstractStringBuilder sealed
As part of updating the core libraries to be sealed, the package-access AbstractStringBuilder, implementation superclass of StringBuilder and StringBuffer, can be marked as sealed with those two subclasses on its permits list. - Commit messages: - Merge branch 'master' into JDK-8283480 - Fix whitespace. - JDK-8283480: Make AbstractStringBuilder sealed Changes: https://git.openjdk.java.net/jdk/pull/7898/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7898=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283480 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7898.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7898/head:pull/7898 PR: https://git.openjdk.java.net/jdk/pull/7898
Re: Unused paramter 'boolean newln' in java.lang.VersionProps#print(boolean err, boolean newln)
VersionProps::print(boolean err, boolean newln) is called by VersionProps::println(boolean) and VersionProps::print(boolean) which is called from the launcher java -version and -showversion option [1]. Mandy [1] https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjli/java.c#L1804 On 3/19/22 11:20 AM, Andrey Turbanov wrote: Hello. I found a suspicious method java.lang.VersionProps#print with unused parameter 'boolean newln'. This class is generated from template - https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/VersionProps.java.template#L203 It's unused since integration of 'JDK-8169069 Module system implementation refresh (11/2016)' - https://github.com/openjdk/jdk/commit/fbe85300bfcc69cb4dd56e4df33ceea632366283 Andrey Turbanov
Re: RFR: JDK-8282798 java.lang.runtime.Carrier [v11]
On 21 Mar 2022, at 11:05, ExE Boss wrote: > Actually, this lookup object should probably be kept cached. +1. Good “cache”.
Integrated: 8257733: Move module-specific data from make to respective module
On Thu, 3 Dec 2020 23:44:20 GMT, Magnus Ihse Bursie wrote: > A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se This pull request has now been integrated. Changeset: f8878cb0 Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/f8878cb0cc436993ef1222bc13b00b923d91aad1 Stats: 140 lines in 619 files changed: 14 ins; 6 del; 120 mod 8257733: Move module-specific data from make to respective module Reviewed-by: jjg, weijun, naoto, erikj, prr, alanb, mchung - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v14]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Add missing newline at EOF - Changes: - all: https://git.openjdk.java.net/jdk/pull/1611/files - new: https://git.openjdk.java.net/jdk/pull/1611/files/1c47d82d..8faeff36 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1611=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1611=12-13 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
On Mon, 21 Mar 2022 14:17:21 GMT, Jorn Vernee wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/CallArranger.java > line 53: > >> (failed to retrieve contents of file, check the PR for context) > Same here, I think keeping the static import for this would make things more > readable. Good catch. I think the IDE did that when I moved files across; I've fixed few of these, but there's more it seems. - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8257733: Move module-specific data from make to respective module [v13]
On Mon, 21 Mar 2022 16:29:25 GMT, Magnus Ihse Bursie wrote: >> A lot (but not all) of the data in make/data is tied to a specific module. >> For instance, the publicsuffixlist is used by java.base, and fontconfig by >> java.desktop. (A few directories, like mainmanifest, is *actually* used by >> make for the whole build.) >> >> These data files should move to the module they belong to. The are, after >> all, "source code" for that module that is "compiler" into resulting >> deliverables, for that module. (But the "source code" language is not Java >> or C, but typically a highly domain specific language or data format, and >> the "compilation" is, often, a specialized transformation.) >> >> This misplacement of the data directory is most visible at code review time. >> When such data is changed, most of the time build-dev (or the new build >> label) is involved, even though this has nothing to do with the build. While >> this is annoying, a worse problem is if the actual team that needs to review >> the patch (i.e., the team owning the module) is missed in the review. >> >> ### Modules reviewed >> >> - [x] java.base >> - [x] java.desktop >> - [x] jdk.compiler >> - [x] java.se > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains 20 commits: > > - Merge branch 'master' into shuffle-data > - Correct name for bfc files > - Merge tag 'jdk-19+14' into shuffle-data > >Added tag jdk-19+14 for changeset 08cadb47 > - Move x11wrappergen from share/data to unix/data > - Fix fontconfig according to review request > - Fix typos > - Restore charsetmapping and cldr to make/data > - Update copyright year > - Merge branch 'master' into shuffle-data-reborn > - Fix merge > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/19d34bdf...1c47d82d I reviewed java.base and java.se. I agree that java.se is a better home for jdwp.spec. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
On Mon, 21 Mar 2022 10:45:27 GMT, Maurizio Cimadamore
wrote:
> This PR contains the API and implementation changes for JEP-424 [1]. A more
> detailed description of such changes, to avoid repetitions during the review
> process, is included as a separate comment.
>
> [1] - https://openjdk.java.net/jeps/424
src/java.base/share/classes/java/lang/foreign/CLinker.java line 176:
> 174: * @param symbol the address of the target function.
> 175: * @param function the function descriptor of the target function.
> 176: * @return a new downcall method handle. The method handle type is
> inferred
Maybe drop the "new" from this. Since we want to do caching in the future.
Suggestion:
* @return a downcall method handle. The method handle type is inferred
src/java.base/share/classes/java/lang/foreign/CLinker.java line 199:
> 197: *
> 198: * @param function the function descriptor of the target function.
> 199: * @return a new downcall method handle. The method handle type is
> inferred
Suggestion:
* @return a downcall method handle. The method handle type is inferred
src/java.base/share/classes/java/lang/foreign/MemoryAddress.java line 159:
> 157: * Creates a memory address from the given long value.
> 158: * @param value the long value representing a raw address.
> 159: * @return a new memory address instance.
Similar here. A new address is not always returned.
Suggestion:
* @return a memory address instance.
src/java.base/share/classes/java/lang/foreign/package-info.java line 230:
> 228: * where {@code M1}, {@code M2}, {@code ... Mn} are module names (for
> the unnamed module, the special value {@code ALL-UNNAMED}
> 229: * can be used). If this option is specified, access to restricted
> methods is only granted to the modules listed by that
> 230: * option. If this option is not specified, access to restricted method
> is enabled for all modules, but
Suggestion:
* option. If this option is not specified, access to restricted methods is
enabled for all modules, but
src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java
line 53:
> (failed to retrieve contents of file, check the PR for context)
Keeping this static import would seem more readable here, instead of prefixing
everything with `AArch64Architecture.`. (especially in the ABI definition below)
src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/CallArranger.java
line 53:
> (failed to retrieve contents of file, check the PR for context)
Same here, I think keeping the static import for this would make things more
readable.
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFC: JDK-8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..)
Hi Volker, I have read through what you have provided/pointed to, thank you, and on the surface what you are suggesting sounds reasonable. That being said given that this API dates back to 1997ish, I think we have to be careful not introduce any regressions with existing applications with the proposal you suggest(even though it is just relaxes the spec), as you mentioned their is a jtreg test that seems to fail. Have you run the JCK with your ZLIB implementation? I only skimmed the tests but looks like there might be a couple of tests which validate the array’s contents. We don’t have a lot of test coverage so if we were to consider moving forward with your proposal, I believe additional test coverage would be warranted. Best Lance On Mar 4, 2022, at 5:04 AM, Volker Simonis mailto:volker.simo...@gmail.com>> wrote: `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater functionality. `Inflater::inflate(byte[] output, int off, int len)` currently calls zlib's native `inflate(..)` function and passes the address of `output[off]` and `len` to it via JNI. The specification of zlib's `inflate(..)` function (i.e. the [API documentation in the original zlib implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) doesn't give any guarantees with regard to usage of the output buffer. It only states that upon completion the function will return the number of bytes that have been written (i.e. "inflated") into the output buffer. The original zlib implementation only wrote as many bytes into the output buffer as it inflated. However, this is not a hard requirement and newer, more performant implementations of the zlib library like [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes of the output buffer than they actually inflate as a scratch buffer. See https://github.com/simonis/zlib-chromium for a more detailed description of their approach and its performance benefit. These new zlib versions can still be used transparently from Java (e.g. by putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because they still fully comply to specification of `Inflater::inflate(..)`. However, we might run into problems when using the `Inflater` functionality from the `InflaterInputStream` class. `InflaterInputStream` is derived from from `InputStream` and as such, its `read(byte[] b, int off, int len)` method is quite constrained. It specifically specifies that if *k* bytes have been read, then "these bytes will be stored in elements `b[off]` through `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] b, int off, int len)` (which is constrained by `InputStream::read(..)`'s specification) calls `Inflater::inflate(byte[] b, int off, int len)` and directly passes its output buffer down to the native zlib `inflate(..)` method which is free to change the bytes beyond `b[off+`*k*`]` (where *k* is the number of inflated bytes). From a practical point of view, I don't see this as a big problem, because callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never know how many bytes will be written into the output buffer `b` (and in fact its content can always be completely overwritten). It therefore makes no sense to depend on any data there being untouched after the call. Also, having used zlib-cloudflare productively for about two years, we haven't seen real-world issues because of this behavior yet. However, from a specification point of view it is easy to artificially construct a program which violates `InflaterInputStream::read(..)`'s postcondition if using one of the alterantive zlib implementations. A recently integrated JTreg test (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails with zlib-chromium but could be easily fixed to run with alternative implementations as well. What should/can be done to solve this problem? I think we have three options: 1. Relax the specification of `InflaterInputStream::read(..)` and specifically note in the API documentation that a call to `InflaterInputStream::read(byte[] b, int off, int len)` may write more than *k* characters into `b` where *k* is the returned number of inflated bytes. Notice that there's a precedence for this approach in `java.io.ByteArrayInputStream::read(..)` which "*unlike the overridden method of InputStream, returns -1 instead of zero if the end of the stream has been reached and len==0*". 2. Tighten the specification of `Inflater::read(byte[] b, int off, int len)` to explicitly forbid any writes into the output array `b` beyond the inflated bytes. 3. Change the implementation of `InflaterInputStream::read(..)` to call `Inflater::read(..)` with a temporary buffer and only copy the nu,ber of inflated bytes into `InflaterInputStream::read(..)`'s
Re: RFC: JDK-8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..)
On 04/03/2022 10:04, Volker Simonis wrote: : 1. Relax the specification of `InflaterInputStream::read(..)` and specifically note in the API documentation that a call to `InflaterInputStream::read(byte[] b, int off, int len)` may write more than *k* characters into `b` where *k* is the returned number of inflated bytes. Notice that there's a precedence for this approach in `java.io.ByteArrayInputStream::read(..)` which "*unlike the overridden method of InputStream, returns -1 instead of zero if the end of the stream has been reached and len==0*". On the surface this is probably okay. It wouldn't really be relaxing the specification, rather than it would say for a return value n, the bytes in b[n] to b[len-1] are undefined as the read operate may have clobbered their original values. The risk is that it becomes a performance "trick" to provide a larger buffer. That said, I think the main thing we need to satisfy ourselves on is security. One part of that is whether anything can be gleaned by reading from the byte array during or after an inflate. The other part is how the implementation behaves when there is a tampering of the array contents during an inflate. -Alan
Re: RFR: 8257733: Move module-specific data from make to respective module [v13]
On Mon, 21 Mar 2022 16:29:25 GMT, Magnus Ihse Bursie wrote: >> A lot (but not all) of the data in make/data is tied to a specific module. >> For instance, the publicsuffixlist is used by java.base, and fontconfig by >> java.desktop. (A few directories, like mainmanifest, is *actually* used by >> make for the whole build.) >> >> These data files should move to the module they belong to. The are, after >> all, "source code" for that module that is "compiler" into resulting >> deliverables, for that module. (But the "source code" language is not Java >> or C, but typically a highly domain specific language or data format, and >> the "compilation" is, often, a specialized transformation.) >> >> This misplacement of the data directory is most visible at code review time. >> When such data is changed, most of the time build-dev (or the new build >> label) is involved, even though this has nothing to do with the build. While >> this is annoying, a worse problem is if the actual team that needs to review >> the patch (i.e., the team owning the module) is missed in the review. >> >> ### Modules reviewed >> >> - [x] java.base >> - [x] java.desktop >> - [x] jdk.compiler >> - [x] java.se > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains 20 commits: > > - Merge branch 'master' into shuffle-data > - Correct name for bfc files > - Merge tag 'jdk-19+14' into shuffle-data > >Added tag jdk-19+14 for changeset 08cadb47 > - Move x11wrappergen from share/data to unix/data > - Fix fontconfig according to review request > - Fix typos > - Restore charsetmapping and cldr to make/data > - Update copyright year > - Merge branch 'master' into shuffle-data-reborn > - Fix merge > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/19d34bdf...1c47d82d As before, the changes for `jdk.compiler` and `idk.javadoc` look OK, and if there is general consensus on the overall proposed move, then I think that overall it is a good idea. I did not review the `jdk.compiler` in low-level detail, but scanning the webrev index file, the changes seemed OK, and a pure move (no lines changed.) It might have been better to have handled this on a per-module basis, rather than all-at-once, but whatever ... - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8279508: Auto-vectorize Math.round API [v15]
On Mon, 21 Mar 2022 17:56:22 GMT, Quan Anh Mai wrote: >> constant and register to register moves are never issued to execution ports, >> rematerializing value rather than reading from memory will give better >> performance. > > I have come across this a little bit. While `movl r, i` may not consume > execution ports, `movq x, r` and `vbroadcastss x, x` surely do. This leads to > 3 retired and 2 executed uops. Furthermore, both `movq x, r` and > `vbroadcastss x, x` can only run on port 5, limit the throughput of the > operation. On the contrary, a `vbroadcastss x, m` only results in 1 retired > and 1 executed uop, reducing pressure on the decoder and the backend. A > `vbroadcastss x, m` can run on both port 2 and port 3, offering a much better > throughput. Latency is not much of a concern in this circumstance since the > operation does not have any input dependency. > >> register to register moves are never issued to execution ports > > I believe you misremembered this part, a register to register move is only > elided when the registers are of the same kind, `vmovq x, r` would result in > 1 uop being executed on port 5. > > What do you think? Thank you very much. A read from constant table will incur minimum of L1I access penalty to access code blob or at worst even more if data is not present in first level cache. Change was done for replace vpbroadcastd with vbroadcastss because of two reasons. 1) vbroadcastss works at AVX=1 level where as vpbroadcastd need AVX2 feature. 2) We can avoid extra cycle penalty due to two domain switchovers (FP -> INT and then from INT-> FP). - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Mon, 21 Mar 2022 06:08:32 GMT, Athijegannathan Sundararajan wrote: > $ javac com/acme/*.java > > is fine. Am I missing something? If you make P.Q private, it will fail the compilation. To make `DelegatingMethodHandle` a sealed class, we could make `AsVarargsCollector` and `WrappedMember` package-private classes. - PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: 8279508: Auto-vectorize Math.round API [v15]
On Sun, 13 Mar 2022 04:27:44 GMT, Jatin Bhateja wrote: >> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4178: >> >>> 4176: movl(scratch, 1056964608); >>> 4177: movq(xtmp1, scratch); >>> 4178: vbroadcastss(xtmp1, xtmp1, vec_enc); >> >> You could put the constant in the constant table and use `vbroadcastss` here >> also. >> >> Thank you very much. > > constant and register to register moves are never issued to execution ports, > rematerializing value rather than reading from memory will give better > performance. I have come across this a little bit. While `movl r, i` may not consume execution ports, `movq x, r` and `vbroadcastss x, x` surely do. This leads to 3 retired and 2 executed uops. Furthermore, both `movq x, r` and `vbroadcastss x, x` can only run on port 5, limit the throughput of the operation. On the contrary, a `vbroadcastss x, m` only results in 1 retired and 1 executed uop, reducing pressure on the decoder and the backend. A `vbroadcastss x, m` can run on both port 2 and port 3, offering a much better throughput. Latency is not much of a concern in this circumstance since the operation does not have any input dependency. > register to register moves are never issued to execution ports I believe you misremembered this part, a register to register move is only elided when the registers are of the same kind, `vmovq x, r` would result in 1 uop being executed on port 5. What do you think? Thank you very much. - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: JDK-8282798 java.lang.runtime.Carrier [v11]
On Mon, 21 Mar 2022 14:24:30 GMT, Jim Laskey wrote:
>> We propose to provide a runtime anonymous carrier class object generator;
>> java.lang.runtime.Carrier. This generator class is designed to share
>> anonymous classes when shapes are similar. For example, if several clients
>> require objects containing two integer fields, then Carrier will ensure that
>> each client generates carrier objects using the same underlying anonymous
>> class.
>>
>> See JBS for details.
>
> Jim Laskey has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains 18 additional commits since
> the last revision:
>
> - Remove LOOKUP static final
> - Merge branch 'master' into 8282798
> - Typos
> - Update Carrier.java
> - Redo API to use list, bring Carrier.component back
> - Clean up API
> - Remove redundant MethodHandle component(MethodType methodType, int i) API
> - Revert to {@return} style comments.
> - Clarify primitive store in array carriers.
> - Use long array for primitives
> - ... and 8 more:
> https://git.openjdk.java.net/jdk/compare/7feac45e...a8657bbe
src/java.base/share/classes/java/lang/runtime/Carrier.java line 574:
> 572: try {
> 573: Lookup lookup = MethodHandles.lookup();
> 574: return lookup.defineHiddenClass(bytes, false,
> ClassOption.STRONG);
Actually, this lookup object should probably be kept cached.
-
PR: https://git.openjdk.java.net/jdk/pull/7744
Re: RFR: 8257733: Move module-specific data from make to respective module [v9]
On Fri, 18 Mar 2022 21:24:43 GMT, Phil Race wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix typos > > This doesn't seem right to me to put x11wrappergen into share. > > This is X11 specific. It should not be in share. > > Same for all of the fontconfig files. In make/data it did not seem too weird > but it is very weird to put them all in share. If you were to go back and > look how it used to be before someone moved them to make I am sure you'd find > them in platform specific source directories. > @prrace I have now moved the fontconfig files to `$OS/data`. (I also took the > opportunity to clean up the GenData file, which had a quite hairy logic for a > trivial task.) I have moved the x11wrappergen files to `unix/data`. > > (Strictly speaking, "unix" and "x11" do not have the same "sense" (in the > Fregean meaning), even if it has the same "referent". But we've used that > convention before so I'm sticking to it.) Indeed they do not but it is a better approximation than "shared". - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
On Mon, 21 Mar 2022 17:16:49 GMT, Erik Joelsson wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > make/modules/java.base/Lib.gmk line 217: > >> 215: CXXFLAGS := $(CXXFLAGS_JDKLIB), \ >> 216: LDFLAGS := $(LDFLAGS_JDKLIB) -Wl$(COMMA)--no-as-needed, \ >> 217: LIBS := $(LIBCXX) -lc -lm -ldl, \ > > Instead of repeating this whole macro call for both Linux and non Linux, you > can use parameters of the form LDFLAGS_linux and LIBS_linux to add the Linux > specific flags. Something like this: > > > LDFLAGS := $(LDFLAGS_JDKLIB), \ > LDFLAGS_linux := -Wl$(COMMA)--no-as-needed, \ > > > For the NAME field, there is no such support, so the way we usually do that > is through a variable and conditionals before the macro call. What's the > reason to have a different lib name on Windows? If they were the same, and > the source file in windows/native/... had the same name, it would just > automatically override in the build. > > I realize now that this is just moved code from jdk.incubator.foreign, and > this patch is probably big enough as it is so no need to refactor the build > logic at the same time. Good points - there is really no need AFAIK for the lib name to be different. I'll do few experiments. - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
On Mon, 21 Mar 2022 10:45:27 GMT, Maurizio Cimadamore wrote: > This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Build changes look ok. make/modules/java.base/Lib.gmk line 217: > 215: CXXFLAGS := $(CXXFLAGS_JDKLIB), \ > 216: LDFLAGS := $(LDFLAGS_JDKLIB) -Wl$(COMMA)--no-as-needed, \ > 217: LIBS := $(LIBCXX) -lc -lm -ldl, \ Instead of repeating this whole macro call for both Linux and non Linux, you can use parameters of the form LDFLAGS_linux and LIBS_linux to add the Linux specific flags. Something like this: LDFLAGS := $(LDFLAGS_JDKLIB), \ LDFLAGS_linux := -Wl$(COMMA)--no-as-needed, \ For the NAME field, there is no such support, so the way we usually do that is through a variable and conditionals before the macro call. What's the reason to have a different lib name on Windows? If they were the same, and the source file in windows/native/... had the same name, it would just automatically override in the build. I realize now that this is just moved code from jdk.incubator.foreign, and this patch is probably big enough as it is so no need to refactor the build logic at the same time. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8257733: Move module-specific data from make to respective module [v13]
On Mon, 21 Mar 2022 16:29:25 GMT, Magnus Ihse Bursie wrote: >> A lot (but not all) of the data in make/data is tied to a specific module. >> For instance, the publicsuffixlist is used by java.base, and fontconfig by >> java.desktop. (A few directories, like mainmanifest, is *actually* used by >> make for the whole build.) >> >> These data files should move to the module they belong to. The are, after >> all, "source code" for that module that is "compiler" into resulting >> deliverables, for that module. (But the "source code" language is not Java >> or C, but typically a highly domain specific language or data format, and >> the "compilation" is, often, a specialized transformation.) >> >> This misplacement of the data directory is most visible at code review time. >> When such data is changed, most of the time build-dev (or the new build >> label) is involved, even though this has nothing to do with the build. While >> this is annoying, a worse problem is if the actual team that needs to review >> the patch (i.e., the team owning the module) is missed in the review. >> >> ### Modules reviewed >> >> - [x] java.base >> - [x] java.desktop >> - [x] jdk.compiler >> - [x] java.se > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains 20 commits: > > - Merge branch 'master' into shuffle-data > - Correct name for bfc files > - Merge tag 'jdk-19+14' into shuffle-data > >Added tag jdk-19+14 for changeset 08cadb47 > - Move x11wrappergen from share/data to unix/data > - Fix fontconfig according to review request > - Fix typos > - Restore charsetmapping and cldr to make/data > - Update copyright year > - Merge branch 'master' into shuffle-data-reborn > - Fix merge > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/19d34bdf...1c47d82d OS specific X11 and fontconfig looks good. make/modules/java.desktop/gendata/GendataFontConfig.gmk line 57: > 55: TARGETS += $(FONTCONFIG_OUT_BIN_FILE) > 56: > 57: endif It says no newline at end of file, maybe add that if it's true. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1611
Integrated: JDK-8283415: Update java.lang.ref to use sealed classes
On Sat, 19 Mar 2022 22:30:13 GMT, Joe Darcy wrote: > Another refactoring to use sealed classes where possible, this time in the > java.lang.ref package. > > Copyright dates will be updated, if needed, before a push. This pull request has now been integrated. Changeset: 14b9e80b Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/14b9e80b8adcc0ab0634357f5a7c25f24fd6808c Stats: 11 lines in 5 files changed: 1 ins; 0 del; 10 mod 8283415: Update java.lang.ref to use sealed classes Reviewed-by: kbarrett, alanb - PR: https://git.openjdk.java.net/jdk/pull/7874
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
On Mon, 21 Mar 2022 10:45:27 GMT, Maurizio Cimadamore
wrote:
> This PR contains the API and implementation changes for JEP-424 [1]. A more
> detailed description of such changes, to avoid repetitions during the review
> process, is included as a separate comment.
>
> [1] - https://openjdk.java.net/jeps/424
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 600:
> 598: * @param elementLayout the source element layout. If the byte order
> associated with the layout is
> 599: * different from the native order, a byte swap operation will be
> performed on each array element.
> 600: * @return a fresh short array copy of this memory segment.
Maybe use "new" instead of "fresh" here and in the other MemorySegment::toArray
methods?
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 600:
> 598: * @param elementLayout the source element layout. If the byte order
> associated with the layout is
> 599: * different from the native order, a byte swap operation will be
> performed on each array element.
> 600: * @return a fresh short array copy of this memory segment.
Maybe use "new" instead of "fresh" here and in the other MemorySegment::toArray
methods?
src/java.base/share/classes/java/lang/foreign/package-info.java line 149:
> 147: * provided:
> 148: *
> 149: * {@snippet lang=java :
missing leading space in comment on line 150 and 162
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 7986:
> 7984: *
> 7985: * When calling e.g. {@link VarHandle#get(Object...)} on the
> resulting var handle, the incoming coordinate values
> 7986: * starting at position {@code pos} (of type {@code C1, C2 ... Cn},
> where {@code C1, C2 ... Cn} are the return type
... are the return *types* ...
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 7986:
> 7984: *
> 7985: * When calling e.g. {@link VarHandle#get(Object...)} on the
> resulting var handle, the incoming coordinate values
> 7986: * starting at position {@code pos} (of type {@code C1, C2 ... Cn},
> where {@code C1, C2 ... Cn} are the return type
... are the return *types* ...
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 8035:
> 8033: * @param pos the position of the first coordinate to be inserted
> 8034: * @param values the series of bound coordinates to insert
> 8035: * @return an adapter var handle which inserts an additional
> coordinates,
... which inserts additional coordinates, ...
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 8035:
> 8033: * @param pos the position of the first coordinate to be inserted
> 8034: * @param values the series of bound coordinates to insert
> 8035: * @return an adapter var handle which inserts an additional
> coordinates,
... which inserts additional coordinates, ...
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 8151:
> 8149: *
> 8150: * @param target the var handle to invoke after the dummy
> coordinates are dropped
> 8151: * @param pos position of first coordinate to drop (zero for the
> leftmost)
... of *the* first coordinate to drop ...
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 8151:
> 8149: *
> 8150: * @param target the var handle to invoke after the dummy
> coordinates are dropped
> 8151: * @param pos position of first coordinate to drop (zero for the
> leftmost)
... of *the* first coordinate to drop ...
-
PR: https://git.openjdk.java.net/jdk/pull/7888
RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
This PR contains the API and implementation changes for JEP-424 [1]. A more detailed description of such changes, to avoid repetitions during the review process, is included as a separate comment. [1] - https://openjdk.java.net/jeps/424 - Commit messages: - Fix TestLayouts on 32-bits - Update copyright - Drop unused imports in Reflection.java - Fix writeOversized for booleans - Revert changes to scopedMemoryAccess.cpp - Add TestUpcallStack to problem list - Revert changes to problem list - Revert to previous handshake logic - Initial push Changes: https://git.openjdk.java.net/jdk/pull/7888/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7888=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282191 Stats: 66428 lines in 364 files changed: 44012 ins; 19911 del; 2505 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview)
On Mon, 21 Mar 2022 10:45:27 GMT, Maurizio Cimadamore wrote: > This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Here is a list of the main changes in this iteration. java.lang.foreign The API is now a **preview** API in `java.lang.foreign`. As such to be able to use the API, users must pass the `--enable-preview` flag accordingly, to `javac` and `java`. Since the API now lives in `java.base`, we dropped the `MemoryHandles` class and moved all its var handle combinator methods under `MethodHandles`. We have also dropped the `MemorySegment::map` factory and replaced it with a new overload in `FileChannel`, which plays much better with custom file systems. ResourceScope The `ResourceScope` abstraction has been renamed to `MemorySession`. Aside from the naming difference (which also is reflected in some of the factories associated with `MemorySession`, another difference are that `MemorySession` now implements `SegmentAllocator` and can be used straight away to allocate segments. Finally, the fact that some sessions are not closeable is now reflected in the API (see `MemorySession::isCloseable`), and a method has been added to create a non-closeable *view* of the memory session. Restricted methods Addressing the feedback we have received during incubation, the mechanism to control access to restricted methods is now more permissive. Users can still use the `--enable-native-access` flag, to get a strict, opt-in behavior, in case they want to control which modules can access restricted methods in the foreign API. But if no flag is specified, access is allowed, with a runtime warning. Supporting this required some changes in `ModuleBootstrap` so that we could record the fact that we have seen an `--enable-native-access` flag (so that all checks in `Reflection.java` becomes constant). Deterministic library loading/unloading We have enhanced the `SymbolLookup` abstraction to provide a new symbol lookup, called *library lookup*. A library lookup is a symbol lookup built around a specific native library which is loaded/unloaded using dlopen/LoadLibrary. Library lookups are associated with memory sessions, so the library can be unloaded deterministically when the session is closed. Memory layouts All memory layout constants feature the expected alignment constraints. For instance, `JAVA_CHAR` is 2 byte aligned, `JAVA_INT` is 4 byte aligned, and so on. Removed functionalities As we moved the API in `java.base` we have dropped a number of API points which did not seem to add much value, based on the feedback received: * `SequenceLayout`s now always have a bounded size. As a result, `MemoryLayout::byteSize` no longer returns an optional. A zero-length sequence can be used instead; * `NativeSymbol` has been dropped. At the end of the day, its role is undistinguishable from that of a memory segment with zero-length, so API (and users) should use zero-length segments instead; * `MemorySession::keepAlive` - has been removed; in its place we have a simple, high-order method which executes an action (a `Runnable`) while keeping the session alive (`MemorySession::whileAlive`); * `MemoryLayout::map` only provides limited capabilities when rewriting layouts (e.g. it can only replace one layout at a time); as such we removed this API, and we might add something better at a later point. Hotspot changes While the Panama foreign repo contains several Hotspot changes which simplify and regularize the foreign function support, these changes are not included here, as we have discovered some intermittent instability in macosx-aarch64. We might attempt to integrate the hotspot changes at a later date. Javadoc: http://cr.openjdk.java.net/~mcimadamore/8282191/v1/javadoc/java.base/module-summary.html Specdiff: http://cr.openjdk.java.net/~mcimadamore/8282191/v1/specdiff_out/overview-summary.html - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8257733: Move module-specific data from make to respective module [v13]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits: - Merge branch 'master' into shuffle-data - Correct name for bfc files - Merge tag 'jdk-19+14' into shuffle-data Added tag jdk-19+14 for changeset 08cadb47 - Move x11wrappergen from share/data to unix/data - Fix fontconfig according to review request - Fix typos - Restore charsetmapping and cldr to make/data - Update copyright year - Merge branch 'master' into shuffle-data-reborn - Fix merge - ... and 10 more: https://git.openjdk.java.net/jdk/compare/19d34bdf...1c47d82d - Changes: https://git.openjdk.java.net/jdk/pull/1611/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1611=12 Stats: 140 lines in 619 files changed: 14 ins; 6 del; 120 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8283415: Update java.lang.ref to use sealed classes [v2]
> Another refactoring to use sealed classes where possible, this time in the > java.lang.ref package. > > Copyright dates will be updated, if needed, before a push. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Update copyrights. - Merge branch 'master' into JDK-8283415 - JDK-8283415: Update java.lang.ref to use sealed classes - Changes: - all: https://git.openjdk.java.net/jdk/pull/7874/files - new: https://git.openjdk.java.net/jdk/pull/7874/files/8dc83e5f..6382c607 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7874=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7874=00-01 Stats: 1302 lines in 71 files changed: 785 ins; 314 del; 203 mod Patch: https://git.openjdk.java.net/jdk/pull/7874.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7874/head:pull/7874 PR: https://git.openjdk.java.net/jdk/pull/7874
Re: RFR: 8257733: Move module-specific data from make to respective module [v12]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Correct name for bfc files - Changes: - all: https://git.openjdk.java.net/jdk/pull/1611/files - new: https://git.openjdk.java.net/jdk/pull/1611/files/ea17b206..f66afd1a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1611=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1611=10-11 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Integrated: 8283277: ISO 4217 Amendment 171 Update
On Thu, 17 Mar 2022 18:10:17 GMT, Naoto Sato wrote: > This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE > redenomination (removing 3 zeros). Its effective date is 4/1, but I went > ahead as JDK19 won't be released by 4/1. This pull request has now been integrated. Changeset: c4dc58e1 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/c4dc58e12e197562dce90c0027aa74c29047cea6 Stats: 17 lines in 6 files changed: 3 ins; 0 del; 14 mod 8283277: ISO 4217 Amendment 171 Update Reviewed-by: iris, joehw - PR: https://git.openjdk.java.net/jdk/pull/7857
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port
On Mon, 8 Nov 2021 11:17:47 GMT, Fei Yang wrote:
> This PR implements JEP 422: Linux/RISC-V Port [1].
> The PR starts as a squashed merge of the
> https://openjdk.java.net/projects/riscv-port branch.
>
> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also
> carried out regularly. So it should be good enough to run most Java programs.
>
> [1] https://openjdk.java.net/jeps/422
Build changes look good. I can't say anything about the rest of the code.
-
Marked as reviewed by ihse (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: 8257733: Move module-specific data from make to respective module [v9]
On Fri, 18 Mar 2022 21:24:43 GMT, Phil Race wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix typos > > This doesn't seem right to me to put x11wrappergen into share. > > This is X11 specific. It should not be in share. > > Same for all of the fontconfig files. In make/data it did not seem too weird > but it is very weird to put them all in share. If you were to go back and > look how it used to be before someone moved them to make I am sure you'd find > them in platform specific source directories. @prrace I have now moved the fontconfig files to `$OS/data`. (I also took the opportunity to clean up the GenData file, which had a quite hairy logic for a trivial task.) I have moved the x11wrappergen files to `unix/data`. (Strictly speaking, "unix" and "x11" do not have the same "sense" (in the Fregean meaning), even if it has the same "referent". But we've used that convention before so I'm sticking to it.) - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: JDK-8282798 java.lang.runtime.Carrier [v11]
> We propose to provide a runtime anonymous carrier class object generator;
> java.lang.runtime.Carrier. This generator class is designed to share
> anonymous classes when shapes are similar. For example, if several clients
> require objects containing two integer fields, then Carrier will ensure that
> each client generates carrier objects using the same underlying anonymous
> class.
>
> See JBS for details.
Jim Laskey has updated the pull request with a new target base due to a merge
or a rebase. The incremental webrev excludes the unrelated changes brought in
by the merge/rebase. The pull request contains 18 additional commits since the
last revision:
- Remove LOOKUP static final
- Merge branch 'master' into 8282798
- Typos
- Update Carrier.java
- Redo API to use list, bring Carrier.component back
- Clean up API
- Remove redundant MethodHandle component(MethodType methodType, int i) API
- Revert to {@return} style comments.
- Clarify primitive store in array carriers.
- Use long array for primitives
- ... and 8 more: https://git.openjdk.java.net/jdk/compare/ffc8a484...a8657bbe
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7744/files
- new: https://git.openjdk.java.net/jdk/pull/7744/files/a82bfd64..a8657bbe
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=7744=10
- incr: https://webrevs.openjdk.java.net/?repo=jdk=7744=09-10
Stats: 9833 lines in 495 files changed: 4777 ins; 2492 del; 2564 mod
Patch: https://git.openjdk.java.net/jdk/pull/7744.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7744/head:pull/7744
PR: https://git.openjdk.java.net/jdk/pull/7744
Re: RFR: 8283279: [Testbug] Improve TestGetSwapSpaceSize
On Thu, 17 Mar 2022 13:40:53 GMT, Severin Gehwolf wrote: > Please review this container test improvement. The test in question only > makes sense iff the total swap space size as reported by the container aware > OperatingSystemMXBean is `0`. If that's not the case, then something else > might be amiss, e.g. OperatingSystemMXBean reporting the host swap limits. In > such a case a passing test tells us nothing. Certainly not if the > fix from [JDK-8242480](https://bugs.openjdk.java.net/browse/JDK-8242480) is > present or not. > > Testing: > - [x] Manual with and without the code fix of JDK-8242480. Still passes with > the fix, and fails without. Tested the test on cgroups v1 and cgroups v2. @DamonFool Do you think you can have a look at this? Thanks! - PR: https://git.openjdk.java.net/jdk/pull/7854
Re: RFR: 8257733: Move module-specific data from make to respective module [v11]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits: - Merge tag 'jdk-19+14' into shuffle-data Added tag jdk-19+14 for changeset 08cadb47 - Move x11wrappergen from share/data to unix/data - Fix fontconfig according to review request - Fix typos - Restore charsetmapping and cldr to make/data - Update copyright year - Merge branch 'master' into shuffle-data-reborn - Fix merge - Merge tag 'jdk-19+13' into shuffle-data-reborn Added tag jdk-19+13 for changeset 5df2a057 - Move characterdata templates to share/classes/java/lang. - ... and 8 more: https://git.openjdk.java.net/jdk/compare/08cadb47...ea17b206 - Changes: https://git.openjdk.java.net/jdk/pull/1611/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1611=10 Stats: 140 lines in 619 files changed: 14 ins; 6 del; 120 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8257733: Move module-specific data from make to respective module [v10]
> A lot (but not all) of the data in make/data is tied to a specific module. > For instance, the publicsuffixlist is used by java.base, and fontconfig by > java.desktop. (A few directories, like mainmanifest, is *actually* used by > make for the whole build.) > > These data files should move to the module they belong to. The are, after > all, "source code" for that module that is "compiler" into resulting > deliverables, for that module. (But the "source code" language is not Java or > C, but typically a highly domain specific language or data format, and the > "compilation" is, often, a specialized transformation.) > > This misplacement of the data directory is most visible at code review time. > When such data is changed, most of the time build-dev (or the new build > label) is involved, even though this has nothing to do with the build. While > this is annoying, a worse problem is if the actual team that needs to review > the patch (i.e., the team owning the module) is missed in the review. > > ### Modules reviewed > > - [x] java.base > - [x] java.desktop > - [x] jdk.compiler > - [x] java.se Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix fontconfig according to review request - Changes: - all: https://git.openjdk.java.net/jdk/pull/1611/files - new: https://git.openjdk.java.net/jdk/pull/1611/files/b1d1e4d8..5a75e7d1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1611=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1611=08-09 Stats: 26 lines in 5 files changed: 10 ins; 5 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/1611.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1611/head:pull/1611 PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8283426: Fix 'exeption' typo [v2]
On Mon, 21 Mar 2022 09:02:17 GMT, Andrey Turbanov wrote: >> Fix repeated typo `exeption` > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8283426: Fix 'exeption' typo > > Apply suggestion > > Co-authored-by: Alexey Ivanov > <70774172+aivanov-...@users.noreply.github.com> Marked as reviewed by aivanov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7879
Re: RFR: JDK-8282798 java.lang.runtime.Carrier [v10]
On Mon, 21 Mar 2022 07:29:21 GMT, ExE Boss wrote: >> The package is java.lang.runtime not java.lang.invoke so both fields are not >> accessible. > > Well, you could use `SharedSecrets.getJavaLangInvokeAccess().findStatic(…)` > and `SharedSecrets.getJavaLangInvokeAccess().findVirtual(…)` in place of > `LOOKUP.findStatic(…)` and `LOOKUP.findVirtual(…)`. Trusted access is not needed, but I can get rid of the LOOKUP static final. The UNSAFE static final is trickier since it might impact the performance of the array getInt. - PR: https://git.openjdk.java.net/jdk/pull/7744
Re: RFC: JDK-8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..)
Ping... On Thu, Mar 10, 2022 at 8:26 PM Lance Andersen wrote: > Hi Volker, > > Thank you for the reminder > > This is on my radar as well but have not had a chance spend any time on > this as yet. > > > > On Mar 9, 2022, at 2:33 PM, Volker Simonis > wrote: > > @Alan, @Lance, > > Sorry for my obtrusiveness, but what's your opinion on this issue? > > Thank you and best regards, > Volker > > On Fri, Mar 4, 2022 at 11:04 AM Volker Simonis > wrote: > > > `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater > functionality. `Inflater::inflate(byte[] output, int off, int len)` > currently calls zlib's native `inflate(..)` function and passes the > address of `output[off]` and `len` to it via JNI. > > The specification of zlib's `inflate(..)` function (i.e. the [API > documentation in the original zlib > implementation]( > https://urldefense.com/v3/__https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h*L400__;Iw!!ACWV5N9M2RV99hQ!fpVcYO7aobaYs-KfF-KOOiQtV17pdX5BNzn8ZkU0UX9B2Lj2Pcs1uh4mh0w3u0Q7HA$ > )) > doesn't give any guarantees with regard to usage of the output buffer. > It only states that upon completion the function will return the > number of bytes that have been written (i.e. "inflated") into the > output buffer. > > The original zlib implementation only wrote as many bytes into the > output buffer as it inflated. However, this is not a hard requirement > and newer, more performant implementations of the zlib library like > [zlib-chromium]( > https://urldefense.com/v3/__https://chromium.googlesource.com/chromium/src/third_party/zlib/__;!!ACWV5N9M2RV99hQ!fpVcYO7aobaYs-KfF-KOOiQtV17pdX5BNzn8ZkU0UX9B2Lj2Pcs1uh4mh0yNMBtuew$ > ) > or [zlib-cloudflare]( > https://urldefense.com/v3/__https://github.com/cloudflare/zlib__;!!ACWV5N9M2RV99hQ!fpVcYO7aobaYs-KfF-KOOiQtV17pdX5BNzn8ZkU0UX9B2Lj2Pcs1uh4mh0y7Uhi8nA$ > ) can use more > bytes of the output buffer than they actually inflate as a scratch > buffer. See > https://urldefense.com/v3/__https://github.com/simonis/zlib-chromium__;!!ACWV5N9M2RV99hQ!fpVcYO7aobaYs-KfF-KOOiQtV17pdX5BNzn8ZkU0UX9B2Lj2Pcs1uh4mh0z1qVlYPg$ > for a more > detailed description of their approach and its performance benefit. > > These new zlib versions can still be used transparently from Java > (e.g. by putting them into the `LD_LIBRARY_PATH` or by using > `LD_PRELOAD`), because they still fully comply to specification of > `Inflater::inflate(..)`. However, we might run into problems when > using the `Inflater` functionality from the `InflaterInputStream` > class. `InflaterInputStream` is derived from from `InputStream` and as > such, its `read(byte[] b, int off, int len)` method is quite > constrained. It specifically specifies that if *k* bytes have been > read, then "these bytes will be stored in elements `b[off]` through > `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through > `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] > b, int off, int len)` (which is constrained by > `InputStream::read(..)`'s specification) calls > `Inflater::inflate(byte[] b, int off, int len)` and directly passes > its output buffer down to the native zlib `inflate(..)` method which > is free to change the bytes beyond `b[off+`*k*`]` (where *k* is the > number of inflated bytes). > > From a practical point of view, I don't see this as a big problem, > because callers of `InflaterInputStream::read(byte[] b, int off, int > len)` can never know how many bytes will be written into the output > buffer `b` (and in fact its content can always be completely > overwritten). It therefore makes no sense to depend on any data there > being untouched after the call. Also, having used zlib-cloudflare > productively for about two years, we haven't seen real-world issues > because of this behavior yet. However, from a specification point of > view it is easy to artificially construct a program which violates > `InflaterInputStream::read(..)`'s postcondition if using one of the > alterantive zlib implementations. A recently integrated JTreg test > (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" > fails with zlib-chromium but could be easily fixed to run with > alternative implementations as well. > > What should/can be done to solve this problem? I think we have three > options: > > 1. Relax the specification of `InflaterInputStream::read(..)` and > specifically note in the API documentation that a call to > `InflaterInputStream::read(byte[] b, int off, int len)` may write more > than *k* characters into `b` where *k* is the returned number of > inflated bytes. Notice that there's a precedence for this approach in > `java.io.ByteArrayInputStream::read(..)` which "*unlike the overridden > method of InputStream, returns -1 instead of zero if the end of the > stream has been reached and len==0*". > > 2. Tighten the specification of `Inflater::read(byte[] b, int off, int > len)` to explicitly forbid any writes into the output array `b`
Re: RFR: 8283426: Fix 'exeption' typo [v2]
> Fix repeated typo `exeption` Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8283426: Fix 'exeption' typo Apply suggestion Co-authored-by: Alexey Ivanov <70774172+aivanov-...@users.noreply.github.com> - Changes: - all: https://git.openjdk.java.net/jdk/pull/7879/files - new: https://git.openjdk.java.net/jdk/pull/7879/files/d93dde25..4c1e68ed Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7879=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7879=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7879.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7879/head:pull/7879 PR: https://git.openjdk.java.net/jdk/pull/7879
Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port
On Mon, 8 Nov 2021 11:17:47 GMT, Fei Yang wrote:
> This PR implements JEP 422: Linux/RISC-V Port [1].
> The PR starts as a squashed merge of the
> https://openjdk.java.net/projects/riscv-port branch.
>
> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive
> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also
> carried out regularly. So it should be good enough to run most Java programs.
>
> [1] https://openjdk.java.net/jeps/422
Rebased to master
keep alive
-
PR: https://git.openjdk.java.net/jdk/pull/6294
RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port
This PR implements JEP 422: Linux/RISC-V Port [1].
The PR starts as a squashed merge of the
https://openjdk.java.net/projects/riscv-port branch.
This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive Unmatched
board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also carried out
regularly. So it should be good enough to run most Java programs.
[1] https://openjdk.java.net/jeps/422
-
Commit messages:
- 8276799: Implementation of JEP 422: Linux/RISC-V Port
Changes: https://git.openjdk.java.net/jdk/pull/6294/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6294=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8276799
Stats: 59153 lines in 188 files changed: 59002 ins; 54 del; 97 mod
Patch: https://git.openjdk.java.net/jdk/pull/6294.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294
PR: https://git.openjdk.java.net/jdk/pull/6294
Re: RFR: JDK-8282798 java.lang.runtime.Carrier [v10]
On Mon, 21 Mar 2022 06:29:53 GMT, Rémi Forax wrote:
>> src/java.base/share/classes/java/lang/runtime/Carrier.java line 309:
>>
>>> 307: static {
>>> 308: LOOKUP = MethodHandles.lookup();
>>> 309: UNSAFE = Unsafe.getUnsafe();
>>
>> It might be better to use `java.lang.invoke.MethodHandleStatics.UNSAFE`, and
>> probably also `java.lang.invoke.MethodHandles.Lookup.IMPL_LOOKUP`.
>
> The package is java.lang.runtime not java.lang.invoke so both fields are not
> accessible.
Well, you could use `SharedSecrets.getJavaLangInvokeAccess().findStatic(…)` and
`SharedSecrets.getJavaLangInvokeAccess().findVirtual(…)` in place of
`LOOKUP.findStatic(…)` and `LOOKUP.findVirtual(…)`.
-
PR: https://git.openjdk.java.net/jdk/pull/7744
Re: RFR: 8283426: Fix 'exeption' typo
On Sun, 20 Mar 2022 13:30:01 GMT, Andrey Turbanov wrote: > Fix repeated typo `exeption` Marked as reviewed by aivanov (Reviewer). src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformEmbeddedFrame.java line 201: > 199: /* > 200: * The method could not be implemented due to CALayer restrictions. > 201: * The exception enforce clients not to use it. Suggestion: * The exception enforces clients not to use it. - PR: https://git.openjdk.java.net/jdk/pull/7879
Re: RFR: JDK-8282798 java.lang.runtime.Carrier [v10]
On Mon, 21 Mar 2022 05:17:31 GMT, ExE Boss wrote:
>> Jim Laskey has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - Typos
>> - Update Carrier.java
>> - Redo API to use list, bring Carrier.component back
>
> src/java.base/share/classes/java/lang/runtime/Carrier.java line 309:
>
>> 307: static {
>> 308: LOOKUP = MethodHandles.lookup();
>> 309: UNSAFE = Unsafe.getUnsafe();
>
> It might be better to use `java.lang.invoke.MethodHandleStatics.UNSAFE`, and
> probably also `java.lang.invoke.MethodHandles.Lookup.IMPL_LOOKUP`.
The package is java.lang.runtime not java.lang.invoke so both fields are not
accessible.
-
PR: https://git.openjdk.java.net/jdk/pull/7744
Re: RFR: 8283411: InflaterInputStream holds on to a temporary byte array of 512 bytes [v2]
On Sun, 20 Mar 2022 14:05:58 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which handles >> https://bugs.openjdk.java.net/browse/JDK-8283411? >> >> The commit here moves the temporary byte array from being a member of the >> class to a local variable within the `skip` method which is the only place >> where it is used as a temporary buffer. >> >> tier1, tier2, tier3 tests have been run successfully with this change. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Use Alan's suggestion and allocate less than 512 bytes if possible. Plus > copyright year fix. Looks ok. - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/7875
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy wrote: > Small refactoring to use sealed classes in the MethodHandle implementation > hierarchy. > > DelegatingMethodHandle is non-sealed rather than sealed since it has two > subclasses, one defined as a nested class and only a separate type in the > same package. The permits clause does not allow listed those two kinds of > subclasses. > > Please also review the corresponding CSR > https://bugs.openjdk.java.net/browse/JDK-8283434 LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy wrote:
> Small refactoring to use sealed classes in the MethodHandle implementation
> hierarchy.
>
> DelegatingMethodHandle is non-sealed rather than sealed since it has two
> subclasses, one defined as a nested class and only a separate type in the
> same package. The permits clause does not allow listed those two kinds of
> subclasses.
>
> Please also review the corresponding CSR
> https://bugs.openjdk.java.net/browse/JDK-8283434
Hmm.. I tried the following example:
$ cat com/acme/X.java
package com.acme;
// We can omit permits clause if all the subclasses are in the same compilation
unit.
// But that's applicable here as two subclasses "Z", "P.Q" are outside the
compilation unit.
// So we use explicit permits clause that lists all the subclasses.
public sealed class X
permits X.Y, W, Z, P.Q {
final class Y extends X {}
}
final class W extends X {}
$ cat com/acme/Z.java
package com.acme;
final class Z extends X {
}
$ cat com/acme/P.java
package com.acme;
class P {
final class Q extends X {}
}
$ javac com/acme/*.java
is fine. Am I missing something?
-
PR: https://git.openjdk.java.net/jdk/pull/7881
