Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 16:45:12 GMT, Henry Jen wrote: >> src/java.base/unix/native/libjli/java_md.c line 666: >> >>> 664: return page_size * pages; >>> 665: } >>> 666: } >> >> Could probably be shortened to something like this: >> >> >> size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); >> return (stack_size + (pagesize - 1)) & ~(pagesize - 1); >> >> >> or, if you insist on checking for SIZE_MAX: >> >> >> size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); >> size_t max = SIZE_MAX - pagesize; >> return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) : >> max; >> >> >> (I see David requested this, so this is fine, though passing SIZE_MAX to >> this function will quite likely fail anyway :) > > While sysconf(_SC_PAGESIZE) is most likely(if not always) be power of 2, it's > not a constant we know for sure here and this is not critical path for > performance, thus I didn't take that approach. My concern was not performance but brevity, especially since you add the same function twice. And about using the same logic for aligning up as we do within hotspot (see align.hpp). You also mix up size_t and long (signed vs unsigned, and potentially different sizes) - there is nothing obvious wrong with that but I would at least consistently use size_t here. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8267630: Start of release updates for JDK 18 [v7]
> 8267630: Start of release updates for JDK 18 Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits: - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Update symbols for JDK 17 b25. - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - ... and 9 more: https://git.openjdk.java.net/jdk/compare/51e8201e...ee6bd4de - Changes: https://git.openjdk.java.net/jdk/pull/4175/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4175=06 Stats: 5334 lines in 63 files changed: 5292 ins; 0 del; 42 mod Patch: https://git.openjdk.java.net/jdk/pull/4175.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4175/head:pull/4175 PR: https://git.openjdk.java.net/jdk/pull/4175
Re: RFR: 8267630: Start of release updates for JDK 18 [v6]
On Tue, 8 Jun 2021 17:58:43 GMT, Joe Darcy wrote: >> 8267630: Start of release updates for JDK 18 > > 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 18 additional commits since > the last revision: > > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - Update symbols for JDK 17 b25. > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - Merge branch 'master' into 8267630 > - ... and 8 more: > https://git.openjdk.java.net/jdk/compare/edba5b0d...15479c92 Looks good. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4175
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
On Tue, 8 Jun 2021 16:53:38 GMT, Henry Jen wrote:
>> …d on macOS
>>
>> This patch simply round up the specified stack size to multiple of the
>> system page size.
>>
>> Test is trivial, simply run java with -Xss option against following code. On
>> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get
>> `7183` and `649` respectively. After fix, both would output `649`, while
>> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
>>
>> ```code:java
>> public class StackLeak {
>> public int depth = 0;
>> public void stackLeak() {
>> depth++;
>> stackLeak();
>> }
>>
>> public static void main(String[] args) {
>> var test = new StackLeak();
>> try {
>> test.stackLeak();
>> } catch (Throwable e) {
>> System.out.println(test.depth);
>> }
>> }
>> }
>
> Henry Jen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update help text
Marked as reviewed by dholmes (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On 8/06/2021 11:40 pm, Thomas Stuefe wrote:
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote:
…d on macOS
This patch simply round up the specified stack size to multiple of the system
page size.
Test is trivial, simply run java with -Xss option against following code. On
MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get `7183`
and `649` respectively. After fix, both would output `649`, while `-Xss161k`
would be same as `-Xss164k` and see 691 as the output.
```code:java
public class StackLeak {
public int depth = 0;
public void stackLeak() {
depth++;
stackLeak();
}
public static void main(String[] args) {
var test = new StackLeak();
try {
test.stackLeak();
} catch (Throwable e) {
System.out.println(test.depth);
}
}
}
Henry Jen 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 seven additional commits since the
last revision:
- Cast type
- Merge
- Change java -X output for -Xss
- Merge
- Only try to round-up when current value failed
- Avoid overflow on page size
- JDK-8236569: -Xss not multiple of 4K does not work for the main thread on
macOS
Please make sure the failing tests have nothing to do with your patch.
`gc/shenandoah/compiler/TestLinkToNativeRBP.java`
sounds at least suggestive.
warning: using incubating module(s): jdk.incubator.foreign
/home/runner/work/jdk/jdk/test/hotspot/jtreg/gc/shenandoah/compiler/TestLinkToNativeRBP.java:42:
error: cannot find symbol
import jdk.incubator.foreign.LibraryLookup;
Looks like shenandoah test was not updated for latest Foreign changes.
David
-
PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v3]
On Tue, 8 Jun 2021 00:30:38 GMT, Scott Gibbons
wrote:
>> Add the Base64 Decode intrinsic for x86 to utilize AVX-512 for acceleration.
>> Also allows for performance improvement for non-AVX-512 enabled platforms.
>> Due to the nature of MIME-encoded inputs, modify the intrinsic signature to
>> accept an additional parameter (isMIME) for fast-path MIME decoding.
>>
>> A change was made to the signature of DecodeBlock in Base64.java to provide
>> the intrinsic information as to whether MIME decoding was being done. This
>> allows for the intrinsic to bypass the expensive setup of zmm registers from
>> AVX tables, knowing there may be invalid Base64 characters every 76
>> characters or so. A change was also made here removing the restriction that
>> the intrinsic must return an even multiple of 3 bytes decoded. This
>> implementation handles the pad characters at the end of the string and will
>> return the actual number of characters decoded.
>>
>> The AVX portion of this code will decode in blocks of 256 bytes per loop
>> iteration, then in chunks of 64 bytes, followed by end fixup decoding. The
>> non-AVX code is an assembly-optimized version of the java DecodeBlock and
>> behaves identically.
>>
>> Running the Base64Decode benchmark, this change increases decode performance
>> by an average of 2.6x with a maximum 19.7x for buffers > ~20k. The numbers
>> are given in the table below.
>>
>> **Base Score** is without intrinsic support, **Optimized Score** is using
>> this intrinsic, and **Gain** is **Base** / **Optimized**.
>>
>>
>> Benchmark Name | Base Score | Optimized Score | Gain
>> -- | -- | -- | --
>> testBase64Decode size 1 | 15.36 | 15.32 | 1.00
>> testBase64Decode size 3 | 17.00 | 16.72 | 1.02
>> testBase64Decode size 7 | 20.60 | 18.82 | 1.09
>> testBase64Decode size 32 | 34.21 | 26.77 | 1.28
>> testBase64Decode size 64 | 54.43 | 38.35 | 1.42
>> testBase64Decode size 80 | 66.40 | 48.34 | 1.37
>> testBase64Decode size 96 | 73.16 | 52.90 | 1.38
>> testBase64Decode size 112 | 84.93 | 51.82 | 1.64
>> testBase64Decode size 512 | 288.81 | 32.04 | 9.01
>> testBase64Decode size 1000 | 560.48 | 40.79 | 13.74
>> testBase64Decode size 2 | 9530.28 | 483.37 | 19.72
>> testBase64Decode size 5 | 24552.24 | 1735.07 | 14.15
>> testBase64MIMEDecode size 1 | 22.87 | 21.36 | 1.07
>> testBase64MIMEDecode size 3 | 27.79 | 25.32 | 1.10
>> testBase64MIMEDecode size 7 | 44.74 | 43.81 | 1.02
>> testBase64MIMEDecode size 32 | 142.69 | 129.56 | 1.10
>> testBase64MIMEDecode size 64 | 256.90 | 243.80 | 1.05
>> testBase64MIMEDecode size 80 | 311.60 | 310.80 | 1.00
>> testBase64MIMEDecode size 96 | 364.00 | 346.66 | 1.05
>> testBase64MIMEDecode size 112 | 472.88 | 394.78 | 1.20
>> testBase64MIMEDecode size 512 | 1814.96 | 1671.28 | 1.09
>> testBase64MIMEDecode size 1000 | 3623.50 | 3227.61 | 1.12
>> testBase64MIMEDecode size 2 | 70484.09 | 64940.77 | 1.09
>> testBase64MIMEDecode size 5 | 191732.34 | 158158.95 | 1.21
>> testBase64WithErrorInputsDecode size 1 | 1531.02 | 1185.19 | 1.29
>> testBase64WithErrorInputsDecode size 3 | 1306.59 | 1170.99 | 1.12
>> testBase64WithErrorInputsDecode size 7 | 1238.11 | 1176.62 | 1.05
>> testBase64WithErrorInputsDecode size 32 | 1346.46 | 1138.47 | 1.18
>> testBase64WithErrorInputsDecode size 64 | 1195.28 | 1172.52 | 1.02
>> testBase64WithErrorInputsDecode size 80 | 1469.00 | 1180.94 | 1.24
>> testBase64WithErrorInputsDecode size 96 | 1434.48 | 1167.74 | 1.23
>> testBase64WithErrorInputsDecode size 112 | 1440.06 | 1162.56 | 1.24
>> testBase64WithErrorInputsDecode size 512 | 1362.79 | 1193.42 | 1.14
>> testBase64WithErrorInputsDecode size 1000 | 1426.07 | 1194.44 | 1.19
>> testBase64WithErrorInputsDecode size 2 | 1398.44 | 1138.17 | 1.23
>> testBase64WithErrorInputsDecode size 5 | 1409.41 | 1114.16 | 1.26
>
> Scott Gibbons has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixing review comments. Adding notes about isMIME parameter for other
> architectures; clarifying decodeBlock comments.
@asgibbons Thanks a lot for contributing this. The performance gain is
impressive. I have some minor comments. Please take a look.
src/hotspot/cpu/x86/assembler_x86.cpp line 4555:
> 4553: void Assembler::evpmaddubsw(XMMRegister dst, XMMRegister src1,
> XMMRegister src2, int vector_len) {
> 4554: assert(VM_Version::supports_avx512bw(), "");
> 4555: InstructionAttr attributes(vector_len, /* rex_w */ false, /*
> legacy_mode */ _legacy_mode_bw, /* no_mask_reg */ true, /* uses_vl */ true);
This instruction is also supported on AVX platforms. The assert check could be
as follows:
assert(vector_len == AVX_128bit? VM_Version::supports_avx() :
vector_len == AVX_256bit? VM_Version::supports_avx2() :
vector_len == AVX_512bit? VM_Version::supports_avx512bw() : 0, "");
Accordingly the instruction could be named as vpmaddubsw.
src/hotspot/cpu/x86/stubGenerator_x86_64.cpp
Re: RFR: 8265909 : build.tools.dtdbuilder.DTDBuilder.java failed detecting missing path of dtd_home [v2]
On Wed, 9 Jun 2021 00:42:35 GMT, ScientificWare
wrote:
>> This concerns [dtdbuilder
>> tools](https://github.com/openjdk/jdk/tree/master/make/jdk/src/classes/build/tools/dtdbuilder).
>>
>> In jshell, try `System.getProperty("html32") + ""` you'll get a `String`.
>>
>> So, in `DTDBuilder.java` when none `dtd_home` property is set, the
>> `dtd_home` string value is not `null`, causing an exception with the present
>> test.
>>
>> The expected value is `"Must set property 'dtd_home'"`
>>
>> And in this case, should'nt we have a `System.exit(1)` too rather than a
>> simple `return` ?
>
> ScientificWare has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Changes to keep home_dtd null check.
>
> As suggested in conversation moving `+ File.separator` is more elegant than
> changing the `null` check.
Submit suggested changes.
-
PR: https://git.openjdk.java.net/jdk/pull/3626
Re: RFR: 8265909 : build.tools.dtdbuilder.DTDBuilder.java failed detecting missing path of dtd_home [v2]
On Tue, 8 Jun 2021 12:37:27 GMT, Erik Joelsson wrote:
>> ScientificWare has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Changes to keep home_dtd null check.
>>
>> As suggested in conversation moving `+ File.separator` is more elegant
>> than changing the `null` check.
>
> make/jdk/src/classes/build/tools/dtdbuilder/DTDBuilder.java line 287:
>
>> 285:
>> 286: String dtd_home = System.getProperty("dtd_home") +
>> File.separator;
>> 287: if (dtd_home.equals("null" + File.separator)) {
>
> dtd_home is only used in one location, so instead of changing this null
> check, maybe just move the "+ File.separator" to line 295?
You are right. Thanks for this suggestion and reviewing.
-
PR: https://git.openjdk.java.net/jdk/pull/3626
Re: RFR: 8265909 : build.tools.dtdbuilder.DTDBuilder.java failed detecting missing path of dtd_home [v2]
> This concerns [dtdbuilder
> tools](https://github.com/openjdk/jdk/tree/master/make/jdk/src/classes/build/tools/dtdbuilder).
>
> In jshell, try `System.getProperty("html32") + ""` you'll get a `String`.
>
> So, in `DTDBuilder.java` when none `dtd_home` property is set, the `dtd_home`
> string value is not `null`, causing an exception with the present test.
>
> The expected value is `"Must set property 'dtd_home'"`
>
> And in this case, should'nt we have a `System.exit(1)` too rather than a
> simple `return` ?
ScientificWare has updated the pull request incrementally with one additional
commit since the last revision:
Changes to keep home_dtd null check.
As suggested in conversation moving `+ File.separator` is more elegant than
changing the `null` check.
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/3626/files
- new: https://git.openjdk.java.net/jdk/pull/3626/files/94c97f14..d1a6727e
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=3626=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=3626=00-01
Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
Patch: https://git.openjdk.java.net/jdk/pull/3626.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3626/head:pull/3626
PR: https://git.openjdk.java.net/jdk/pull/3626
Integrated: 8264766: ClassCastException during template compilation (Variable cannot be cast to Param)
On Mon, 7 Jun 2021 18:49:19 GMT, Joe Wang wrote: > Fixes the addVariable/addParam methods in the SymbolTable to check types > before casting. This pull request has now been integrated. Changeset: 1c3932f3 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/1c3932f3d5ec47678f55769cb6a9f657ace411c6 Stats: 70 lines in 2 files changed: 64 ins; 0 del; 6 mod 8264766: ClassCastException during template compilation (Variable cannot be cast to Param) Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/4398
RFR: 8268353: Test libsvml.so is and is not present in jdk image
Test that when the jdk.incubator.vector module is present that libsvml.so is present, and test the opposite case. - Commit messages: - Improve test. - Test for linux and windows - 8268353: Test libsvml.so is and is not present in jdk image Changes: https://git.openjdk.java.net/jdk/pull/4421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4421=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268353 Stats: 99 lines in 2 files changed: 96 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4421/head:pull/4421 PR: https://git.openjdk.java.net/jdk/pull/4421
Re: RFR: 8266835: Add a --validate option to the jar tool [v3]
On Tue, 8 Jun 2021 18:32:36 GMT, Jorn Vernee wrote: >> This patch adds a `--validate` option to the jar tool which can be used to >> validate a jar file that might be malformed. For instance, if a jar is a >> multi-release jar, it is malformed if different versions expose different >> APIs. >> >> The implementation is straight forward since there already exists validation >> logic that is run when creating or updating a jar. This patch just exposes >> that logic directly under a new command line flag. >> >> I've enhanced the existing ApiValidatorTest to also create malformed jars >> using the zip file APIs (the jar tool does not output malformed jars) and >> run them through `jar --validate`. >> >> Note that while the jdk's jar tool does not output malformed jars, >> third-party archiving tools might, or the jar could have been manually >> edited. >> >> Some prior discussion here: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html >> >> Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), >> manual testing. > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Improve help message. The changes look good - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3971
Integrated: 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages
On Tue, 8 Jun 2021 16:45:27 GMT, Alexey Semenyuk wrote: > …ages This pull request has now been integrated. Changeset: bcaa2cb1 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/bcaa2cb154ae5d23a067f6e38a19a21eef8fe8e8 Stats: 115 lines in 5 files changed: 114 ins; 0 del; 1 mod 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages Reviewed-by: herrick, almatvee - PR: https://git.openjdk.java.net/jdk/pull/4415
Re: RFR: 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages [v2]
On Tue, 8 Jun 2021 17:25:46 GMT, Alexey Semenyuk wrote: >> …ages > > Alexey Semenyuk 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 branch 'master' into JDK-8264144 > - 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4415
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v15]
On Tue, 8 Jun 2021 14:26:43 GMT, Roger Riggs wrote: >> JEP 415: Context-specific Deserialization Filters extends the >> deserialization filtering mechanisms with more flexible and customizable >> protections against malicious deserialization. See JEP 415: >> https://openjdk.java.net/jeps/415. >> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are >> extended with additional >> configuration mechanisms and filter utilities. >> >> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and >> `ObjectInputStream`: >> >> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify exceptions that occur if initializing the filter factory from > jdk.serialFilterFactory fails Marked as reviewed by bchristi (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3996
Integrated: JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed as SCR
On Wed, 2 Jun 2021 13:48:47 GMT, Andy Herrick wrote: > JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed > as SCR This pull request has now been integrated. Changeset: 51e8201e Author:Andy Herrick URL: https://git.openjdk.java.net/jdk/commit/51e8201eb5a66a8fbbff21194fd35389343baee1 Stats: 69 lines in 2 files changed: 67 ins; 0 del; 2 mod 8267764: jpackage cannot handle window screensaver files when EXE renamed as SCR Reviewed-by: asemenyuk, almatvee - PR: https://git.openjdk.java.net/jdk/pull/4306
Re: RFR: 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages [v2]
On Tue, 8 Jun 2021 17:25:46 GMT, Alexey Semenyuk wrote: >> …ages > > Alexey Semenyuk 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 branch 'master' into JDK-8264144 > - 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages Marked as reviewed by herrick (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4415
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v5]
On Tue, 8 Jun 2021 17:34:58 GMT, Jorn Vernee wrote: >> Hi, >> >> ~This is part 2 of a two part upstreaming process of the patch mentioned in >> the subject line. The patch was split into 2 in order to document 2 separate >> specification changes that arose from a change in the implementation, with 2 >> separate CSRs. The first patch can be found here: >> https://github.com/openjdk/jdk/pull/4395~ >> >> This patch adds uniform exception handling for exceptions thrown during FLA >> upcalls. Currently, these exceptions are either handled similarly to the VMs >> `CATCH` macro, or ignored after which control returns to unsuspecting native >> code, until control returns to Java, which will then handle the exception. >> The handling depends on the invocation mode. >> >> Both of these are bad. The former because a stack trace is not printed and >> instead the VM exits with a fatal error. The latter is bad because an upcall >> completing abruptly and returning to native code which has no idea an >> exception occurred is unsafe, in the sense that invariants about the state >> of the program that the native code depends on might no longer hold. >> >> This patch adds uniform exception handling that replaces both of these cases >> (see `SharedUtils::handleUncaughtException`), which prints the exception >> stack trace, and then unconditionally exits the VM, which is the only safe >> course of action at that point. >> >> Exceptions thrown by upcalls should instead be handled during the upcall >> itself, for instance by translating the exception into an error code that is >> then returned to the native caller, which can deal with it appropriately. >> >> See also the original review for panama-foreign: >> https://github.com/openjdk/panama-foreign/pull/543 >> >> Thanks, >> Jorn >> >> Testing: `jdk_foreign` test suite. >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Suggest try/catch Throwable in upcallStub javadoc I think this approach makes sense given the native code cannot react to the exception, possibly resulting in undefined behavior. We might be able to better check upcall handles if they declare they throw and reject them, but it's tricky to nail down the exact conditions, so better to defer and spend more time getting it right. (Perhaps one day we might be able to reflect over code and do more detailed analysis.) - Marked as reviewed by psandoz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v5]
On Tue, 8 Jun 2021 17:34:58 GMT, Jorn Vernee wrote: >> Hi, >> >> ~This is part 2 of a two part upstreaming process of the patch mentioned in >> the subject line. The patch was split into 2 in order to document 2 separate >> specification changes that arose from a change in the implementation, with 2 >> separate CSRs. The first patch can be found here: >> https://github.com/openjdk/jdk/pull/4395~ >> >> This patch adds uniform exception handling for exceptions thrown during FLA >> upcalls. Currently, these exceptions are either handled similarly to the VMs >> `CATCH` macro, or ignored after which control returns to unsuspecting native >> code, until control returns to Java, which will then handle the exception. >> The handling depends on the invocation mode. >> >> Both of these are bad. The former because a stack trace is not printed and >> instead the VM exits with a fatal error. The latter is bad because an upcall >> completing abruptly and returning to native code which has no idea an >> exception occurred is unsafe, in the sense that invariants about the state >> of the program that the native code depends on might no longer hold. >> >> This patch adds uniform exception handling that replaces both of these cases >> (see `SharedUtils::handleUncaughtException`), which prints the exception >> stack trace, and then unconditionally exits the VM, which is the only safe >> course of action at that point. >> >> Exceptions thrown by upcalls should instead be handled during the upcall >> itself, for instance by translating the exception into an error code that is >> then returned to the native caller, which can deal with it appropriately. >> >> See also the original review for panama-foreign: >> https://github.com/openjdk/panama-foreign/pull/543 >> >> Thanks, >> Jorn >> >> Testing: `jdk_foreign` test suite. >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Suggest try/catch Throwable in upcallStub javadoc test/jdk/java/foreign/TestUpcallException.java line 70: > 68: Process process = new ProcessBuilder() > 69: .command( > 70: Paths.get(Utils.TEST_JDK) You might find `jdk.test.lib.JDKToolLauncher` useful. - PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8266835: Add a --validate option to the jar tool [v2]
On Tue, 8 Jun 2021 17:28:21 GMT, Alan Bateman wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update error message > > This all looks reasonable, I just wonder if the --validate description in the > help output should provide a brief summary on what it does check. We can add > to it as some validation is added. Without it then people will make > assumptions on what it does. @AlanBateman Thanks for the suggestion. I think adding a description is good idea. I initially left the description vague, so that the behavior could be more easily amended. But, on second thought, such changes would seemingly require no less review, so I think it's better to include a description, as suggested. I've updated the description to say the following: \ --validate Validate a jar. This process will validate that the API\n\ \ exported by a multi-release jar is consistent across all\n\ \ different release versions. - PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8266835: Add a --validate option to the jar tool [v3]
> This patch adds a `--validate` option to the jar tool which can be used to > validate a jar file that might be malformed. For instance, if a jar is a > multi-release jar, it is malformed if different versions expose different > APIs. > > The implementation is straight forward since there already exists validation > logic that is run when creating or updating a jar. This patch just exposes > that logic directly under a new command line flag. > > I've enhanced the existing ApiValidatorTest to also create malformed jars > using the zip file APIs (the jar tool does not output malformed jars) and run > them through `jar --validate`. > > Note that while the jdk's jar tool does not output malformed jars, > third-party archiving tools might, or the jar could have been manually edited. > > Some prior discussion here: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html > > Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), > manual testing. Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Improve help message. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3971/files - new: https://git.openjdk.java.net/jdk/pull/3971/files/cbd6e81b..c4cfe847 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3971=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3971=01-02 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3971.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3971/head:pull/3971 PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 18:22:55 GMT, Weijun Wang wrote: >>> I thought about that but not sure of performance impact. Is the worst >>> problem that more than one warnings will be printed for a single caller? >>> It's not really harmless. >>> >>> As for the frame, if the warning message only contain the caller class name >>> and its code source, why is it worth using a key of multiple frames? The >>> message will look the same. >> >> WeakHashMap access needs synchronization. Whether we need to cache to avoid >> excessive warnings isn't clear. If the SM is enabled once and never >> disabled/re-enabled then caching isn't interesting. On the other hand if >> there are programs that are enabling/disabling to execute subsets of code >> then maybe it is. Maybe we should just drop this and see if there is any >> feedback on the repeated warning? > > Not sure what you meant by "WeakHashMap access synchronization", it's just a > noun without any other parts. Do you think synchronization is necessary? > > For the cache, I'm OK to drop it at the moment. I think it would be simpler to start out without the caller cache. Sorry the sentence got garbled, I was trying to repeat what I said above that WeakHashMap is not synchronized so you would need to add synchronization to use it. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 17:15:29 GMT, Alan Bateman wrote: >> I thought about that but not sure of performance impact. Is the worst >> problem that more than one warnings will be printed for a single caller? >> It's not really harmless. >> >> As for the frame, if the warning message only contain the caller class name >> and its code source, why is it worth using a key of multiple frames? The >> message will look the same. > >> I thought about that but not sure of performance impact. Is the worst >> problem that more than one warnings will be printed for a single caller? >> It's not really harmless. >> >> As for the frame, if the warning message only contain the caller class name >> and its code source, why is it worth using a key of multiple frames? The >> message will look the same. > > WeakHashMap access needs synchronization. Whether we need to cache to avoid > excessive warnings isn't clear. If the SM is enabled once and never > disabled/re-enabled then caching isn't interesting. On the other hand if > there are programs that are enabling/disabling to execute subsets of code > then maybe it is. Maybe we should just drop this and see if there is any > feedback on the repeated warning? Not sure what you meant by "WeakHashMap access synchronization", it's just a noun without any other parts. Do you think synchronization is necessary? For the cache, I'm OK to drop it at the moment. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8267630: Start of release updates for JDK 18 [v6]
> 8267630: Start of release updates for JDK 18 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 18 additional commits since the last revision: - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Update symbols for JDK 17 b25. - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - ... and 8 more: https://git.openjdk.java.net/jdk/compare/f12e9650...15479c92 - Changes: - all: https://git.openjdk.java.net/jdk/pull/4175/files - new: https://git.openjdk.java.net/jdk/pull/4175/files/34480e50..15479c92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4175=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4175=04-05 Stats: 5201 lines in 96 files changed: 2786 ins; 1026 del; 1389 mod Patch: https://git.openjdk.java.net/jdk/pull/4175.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4175/head:pull/4175 PR: https://git.openjdk.java.net/jdk/pull/4175
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v5]
> Hi, > > ~This is part 2 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. The first patch can be found here: > https://github.com/openjdk/jdk/pull/4395~ > > This patch adds uniform exception handling for exceptions thrown during FLA > upcalls. Currently, these exceptions are either handled similarly to the VMs > `CATCH` macro, or ignored after which control returns to unsuspecting native > code, until control returns to Java, which will then handle the exception. > The handling depends on the invocation mode. > > Both of these are bad. The former because a stack trace is not printed and > instead the VM exits with a fatal error. The latter is bad because an upcall > completing abruptly and returning to native code which has no idea an > exception occurred is unsafe, in the sense that invariants about the state of > the program that the native code depends on might no longer hold. > > This patch adds uniform exception handling that replaces both of these cases > (see `SharedUtils::handleUncaughtException`), which prints the exception > stack trace, and then unconditionally exits the VM, which is the only safe > course of action at that point. > > Exceptions thrown by upcalls should instead be handled during the upcall > itself, for instance by translating the exception into an error code that is > then returned to the native caller, which can deal with it appropriately. > > See also the original review for panama-foreign: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Suggest try/catch Throwable in upcallStub javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/4396/files - new: https://git.openjdk.java.net/jdk/pull/4396/files/a9e652e7..fd691628 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4396=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4396=03-04 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4396.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4396/head:pull/4396 PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8266835: Add a --validate option to the jar tool [v2]
On Wed, 19 May 2021 15:36:57 GMT, Jorn Vernee wrote: >> This patch adds a `--validate` option to the jar tool which can be used to >> validate a jar file that might be malformed. For instance, if a jar is a >> multi-release jar, it is malformed if different versions expose different >> APIs. >> >> The implementation is straight forward since there already exists validation >> logic that is run when creating or updating a jar. This patch just exposes >> that logic directly under a new command line flag. >> >> I've enhanced the existing ApiValidatorTest to also create malformed jars >> using the zip file APIs (the jar tool does not output malformed jars) and >> run them through `jar --validate`. >> >> Note that while the jdk's jar tool does not output malformed jars, >> third-party archiving tools might, or the jar could have been manually >> edited. >> >> Some prior discussion here: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html >> >> Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), >> manual testing. > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Update error message This all looks reasonable, I just wonder if the --validate description in the help output should provide a brief summary on what it does check. We can add to it as some validation is added. Without it then people will make assumptions on what it does. - PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages [v2]
> …ages Alexey Semenyuk 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 branch 'master' into JDK-8264144 - 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages - Changes: - all: https://git.openjdk.java.net/jdk/pull/4415/files - new: https://git.openjdk.java.net/jdk/pull/4415/files/74ece7db..0aba3dd9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4415=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4415=00-01 Stats: 29705 lines in 426 files changed: 25434 ins; 2162 del; 2109 mod Patch: https://git.openjdk.java.net/jdk/pull/4415.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4415/head:pull/4415 PR: https://git.openjdk.java.net/jdk/pull/4415
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v4]
> Hi, > > ~This is part 2 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. The first patch can be found here: > https://github.com/openjdk/jdk/pull/4395~ > > This patch adds uniform exception handling for exceptions thrown during FLA > upcalls. Currently, these exceptions are either handled similarly to the VMs > `CATCH` macro, or ignored after which control returns to unsuspecting native > code, until control returns to Java, which will then handle the exception. > The handling depends on the invocation mode. > > Both of these are bad. The former because a stack trace is not printed and > instead the VM exits with a fatal error. The latter is bad because an upcall > completing abruptly and returning to native code which has no idea an > exception occurred is unsafe, in the sense that invariants about the state of > the program that the native code depends on might no longer hold. > > This patch adds uniform exception handling that replaces both of these cases > (see `SharedUtils::handleUncaughtException`), which prints the exception > stack trace, and then unconditionally exits the VM, which is the only safe > course of action at that point. > > Exceptions thrown by upcalls should instead be handled during the upcall > itself, for instance by translating the exception into an error code that is > then returned to the native caller, which can deal with it appropriately. > > See also the original review for panama-foreign: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Remove another spurious import - Changes: - all: https://git.openjdk.java.net/jdk/pull/4396/files - new: https://git.openjdk.java.net/jdk/pull/4396/files/803427e7..a9e652e7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4396=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4396=02-03 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4396.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4396/head:pull/4396 PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 12:22:52 GMT, Weijun Wang wrote: > I thought about that but not sure of performance impact. Is the worst problem > that more than one warnings will be printed for a single caller? It's not > really harmless. > > As for the frame, if the warning message only contain the caller class name > and its code source, why is it worth using a key of multiple frames? The > message will look the same. WeakHashMap access synchronization. Whether we need to cache to avoid excessive warnings isn't clear. If the SM is enabled once and never disabled/re-enabled then caching isn't interesting. On the other hand if there are programs that are enabling/disabling to execute subsets of code then maybe it is. Maybe we should just drop this and see if there is any feedback on the repeated warning? - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v3]
On Tue, 8 Jun 2021 17:07:43 GMT, Jorn Vernee wrote: >> Hi, >> >> This is part 2 of a two part upstreaming process of the patch mentioned in >> the subject line. The patch was split into 2 in order to document 2 separate >> specification changes that arose from a change in the implementation, with 2 >> separate CSRs. The first patch can be found here: >> https://github.com/openjdk/jdk/pull/4395 >> >> This patch adds uniform exception handling for exceptions thrown during FLA >> upcalls. Currently, these exceptions are either handled similarly to the VMs >> `CATCH` macro, or ignored after which control returns to unsuspecting native >> code, until control returns to Java, which will then handle the exception. >> The handling depends on the invocation mode. >> >> Both of these are bad. The former because a stack trace is not printed and >> instead the VM exits with a fatal error. The latter is bad because an upcall >> completing abruptly and returning to native code which has no idea an >> exception occurred is unsafe, in the sense that invariants about the state >> of the program that the native code depends on might no longer hold. >> >> This patch adds uniform exception handling that replaces both of these cases >> (see `SharedUtils::handleUncaughtException`), which prints the exception >> stack trace, and then unconditionally exits the VM, which is the only safe >> course of action at that point. >> >> Exceptions thrown by upcalls should instead be handled during the upcall >> itself, for instance by translating the exception into an error code that is >> then returned to the native caller, which can deal with it appropriately. >> >> See also the original review for panama-foreign: >> https://github.com/openjdk/panama-foreign/pull/543 >> >> Thanks, >> Jorn >> >> Testing: `jdk_foreign` test suite. >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Remove spurious import > - Pt2 During some offline discussion of the CSR for the first part of this upstreaming, it was decided to focus on the second part only, and address the first part later. I've rebased this PR onto the master branch, and this has the effect that the `CLinker::upcallStub` method no longer does an eager check for exceptions, and I've updated the javadoc to reflect this. I've also removed the security manager usage, which tries to block calls to System.exit, from the test, since the new version of the jtreg agent also tries to call System.exit it seems, and the test fails because of the SecurityManager it installs. While it's theoretically possible to inspect the stack to figure out if the jtreg agent is calling System.exit, and then give permission, with the complexity that this would introduce in the test, together with the fact that the SecurityManager is deprecated, and it being seemingly unlikely that a SecurityManager check would be added to the alternative API currently being used to shut down the VM, it seems that programmatically testing that we can exit the VM with a SecurityManager installed is not worth the effort. We already know that the current code works from earlier testing. - PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v3]
> Hi, > > This is part 2 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. The first patch can be found here: > https://github.com/openjdk/jdk/pull/4395 > > This patch adds uniform exception handling for exceptions thrown during FLA > upcalls. Currently, these exceptions are either handled similarly to the VMs > `CATCH` macro, or ignored after which control returns to unsuspecting native > code, until control returns to Java, which will then handle the exception. > The handling depends on the invocation mode. > > Both of these are bad. The former because a stack trace is not printed and > instead the VM exits with a fatal error. The latter is bad because an upcall > completing abruptly and returning to native code which has no idea an > exception occurred is unsafe, in the sense that invariants about the state of > the program that the native code depends on might no longer hold. > > This patch adds uniform exception handling that replaces both of these cases > (see `SharedUtils::handleUncaughtException`), which prints the exception > stack trace, and then unconditionally exits the VM, which is the only safe > course of action at that point. > > Exceptions thrown by upcalls should instead be handled during the upcall > itself, for instance by translating the exception into an error code that is > then returned to the native caller, which can deal with it appropriately. > > See also the original review for panama-foreign: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. > > Thanks, > Jorn Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Remove spurious import - Pt2 - Changes: https://git.openjdk.java.net/jdk/pull/4396/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4396=02 Stats: 227 lines in 8 files changed: 222 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4396.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4396/head:pull/4396 PR: https://git.openjdk.java.net/jdk/pull/4396
RFR: 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages
…ages - Commit messages: - 8264144: Add handling of "--about-url" CLI parameter for RPM/DEB packages Changes: https://git.openjdk.java.net/jdk/pull/4415/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4415=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264144 Stats: 115 lines in 5 files changed: 114 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4415.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4415/head:pull/4415 PR: https://git.openjdk.java.net/jdk/pull/4415
RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling
Please review this PR which is just syncing the implementation of DirectMethodHandleDesc.Kind.valueOf(int) with its spec. My reading of the method's spec is that if the value of the `refKind` parameter is: MethodHandleInfo.REF_invokeInterface, then DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a value of `true` for its second argument, currently it is invoked with `false` regardless of the value of the `refKind` parameter TIA - Commit messages: - 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling Changes: https://git.openjdk.java.net/jdk/pull/4416/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4416=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267421 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4416.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4416/head:pull/4416 PR: https://git.openjdk.java.net/jdk/pull/4416
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
> …d on macOS
>
> This patch simply round up the specified stack size to multiple of the system
> page size.
>
> Test is trivial, simply run java with -Xss option against following code. On
> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get
> `7183` and `649` respectively. After fix, both would output `649`, while
> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
>
> ```code:java
> public class StackLeak {
> public int depth = 0;
> public void stackLeak() {
> depth++;
> stackLeak();
> }
>
> public static void main(String[] args) {
> var test = new StackLeak();
> try {
> test.stackLeak();
> } catch (Throwable e) {
> System.out.println(test.depth);
> }
> }
> }
Henry Jen has updated the pull request incrementally with one additional commit
since the last revision:
Update help text
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4256/files
- new: https://git.openjdk.java.net/jdk/pull/4256/files/5a8d4a10..a3966612
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4256=05
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4256=04-05
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/4256.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256
PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 08:17:54 GMT, Thomas Stuefe wrote: >> Henry Jen 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 seven additional commits >> since the last revision: >> >> - Cast type >> - Merge >> - Change java -X output for -Xss >> - Merge >> - Only try to round-up when current value failed >> - Avoid overflow on page size >> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on >> macOS > > src/java.base/unix/native/libjli/java_md.c line 666: > >> 664: return page_size * pages; >> 665: } >> 666: } > > Could probably be shortened to something like this: > > > size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); > return (stack_size + (pagesize - 1)) & ~(pagesize - 1); > > > or, if you insist on checking for SIZE_MAX: > > > size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); > size_t max = SIZE_MAX - pagesize; > return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) : > max; > > > (I see David requested this, so this is fine, though passing SIZE_MAX to this > function will quite likely fail anyway :) While sysconf(_SC_PAGESIZE) is most likely(if not always) be power of 2, it's not a constant we know for sure here and this is not critical path for performance, thus I didn't take that approach. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 02:36:26 GMT, David Holmes wrote: >> Henry Jen 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 seven additional commits >> since the last revision: >> >> - Cast type >> - Merge >> - Change java -X output for -Xss >> - Merge >> - Only try to round-up when current value failed >> - Avoid overflow on page size >> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on >> macOS > > src/java.base/share/classes/sun/launcher/resources/launcher.properties line > 175: > >> 173: \ configuration and continue\n\ >> 174: \-Xssset java thread stack size\n\ >> 175: \ The actual size may be round up to multiple of >> system\n\ > > See updated help text in the CSR request. Updated, thanks - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2)
On Mon, 7 Jun 2021 16:38:01 GMT, Jorn Vernee wrote: > Hi, > > This is part 2 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. The first patch can be found here: > https://github.com/openjdk/jdk/pull/4395 > > This patch adds uniform exception handling for exceptions thrown during FLA > upcalls. Currently, these exceptions are either handled similarly to the VMs > `CATCH` macro, or ignored after which control returns to unsuspecting native > code, until control returns to Java, which will then handle the exception. > The handling depends on the invocation mode. > > Both of these are bad. The former because a stack trace is not printed and > instead the VM exits with a fatal error. The latter is bad because an upcall > completing abruptly and returning to native code which has no idea an > exception occurred is unsafe, in the sense that invariants about the state of > the program that the native code depends on might no longer hold. > > This patch adds uniform exception handling that replaces both of these cases > (see `SharedUtils::handleUncaughtException`), which prints the exception > stack trace, and then unconditionally exits the VM, which is the only safe > course of action at that point. > > Exceptions thrown by upcalls should instead be handled during the upcall > itself, for instance by translating the exception into an error code that is > then returned to the native caller, which can deal with it appropriately. > > See also the original review for panama-foreign: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. > > Thanks, > Jorn The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout Upstream_Excp_Hndl git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v2]
> Hi, > > This is part 2 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. The first patch can be found here: > https://github.com/openjdk/jdk/pull/4395 > > This patch adds uniform exception handling for exceptions thrown during FLA > upcalls. Currently, these exceptions are either handled similarly to the VMs > `CATCH` macro, or ignored after which control returns to unsuspecting native > code, until control returns to Java, which will then handle the exception. > The handling depends on the invocation mode. > > Both of these are bad. The former because a stack trace is not printed and > instead the VM exits with a fatal error. The latter is bad because an upcall > completing abruptly and returning to native code which has no idea an > exception occurred is unsafe, in the sense that invariants about the state of > the program that the native code depends on might no longer hold. > > This patch adds uniform exception handling that replaces both of these cases > (see `SharedUtils::handleUncaughtException`), which prints the exception > stack trace, and then unconditionally exits the VM, which is the only safe > course of action at that point. > > Exceptions thrown by upcalls should instead be handled during the upcall > itself, for instance by translating the exception into an error code that is > then returned to the native caller, which can deal with it appropriately. > > See also the original review for panama-foreign: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. > > Thanks, > Jorn Jorn Vernee 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. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4396/files - new: https://git.openjdk.java.net/jdk/pull/4396/files/b495aae3..b495aae3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4396=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4396=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4396.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4396/head:pull/4396 PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v3]
On Tue, 8 Jun 2021 14:13:53 GMT, Jatin Bhateja wrote: >> I must be missing something. How is the brute force loop aligned if not by >> this directive? I don't see an alignment anywhere else that could force it. >> After the entry(), there are pushes and length comparisons followed by the >> conditional on VBMI. The only thing I can guess would be that the jmp >> aligns, but I see no indication that that occurs. >> >> Perhaps what you missed was that L_forceLoop is aligned (line 6288). This >> is not the same label as L_bruteForce, which is a jump target from within >> the VBMI conditional (which should be aligned)? Otherwise, I don't see how >> L_bruteForce could possibly already be aligned. > > Yes, I meant force loop already has alignment so earlier one can be removed. Sorry - still confused. These are two different labels, bound to two different locations. I believe the alignments for both are justified. - PR: https://git.openjdk.java.net/jdk/pull/4368
Withdrawn: 8268328: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 1)
On Mon, 7 Jun 2021 15:14:39 GMT, Jorn Vernee wrote: > Hi, > > This is part 1 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. > > This patch changes the handling of method handles that might throw checked > exceptions, by the var handle combinators found in > `jdk.incubator.foreign.MemoryHandles`. Particularly, it makes the check more > lenient towards `BoundMethodHandle`s that have fields that are method handles > that might themselves throw exceptions, since it is not known whether the > enclosing method handle catches such exceptions. For instance, if a target > method handle is wrapped using the `catchException` combinator, which handles > all exceptions thrown by that target, the current code will still reject such > method handles, even though no checked exceptions can be thrown. > > The patch fixes this by instead wrapping method handles for which it is not > possible to eagerly determine whether they throw exception using the > `catchException` combinator, with an exception handler that catches checked > exceptions and instead throws an `IllegalStateException` with the original > exception as the cause. (See also the CSR). > > The main motivation for doing this is having the ability to share the > implementation with the `CLinker::upcallStub` API, which also wants to > eagerly reject method handles that are determined to throw exceptions. > > See also the original review for the panama-foreign repo: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/4395
Re: RFR: 8268328: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 1)
On Mon, 7 Jun 2021 15:14:39 GMT, Jorn Vernee wrote: > Hi, > > This is part 1 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. > > This patch changes the handling of method handles that might throw checked > exceptions, by the var handle combinators found in > `jdk.incubator.foreign.MemoryHandles`. Particularly, it makes the check more > lenient towards `BoundMethodHandle`s that have fields that are method handles > that might themselves throw exceptions, since it is not known whether the > enclosing method handle catches such exceptions. For instance, if a target > method handle is wrapped using the `catchException` combinator, which handles > all exceptions thrown by that target, the current code will still reject such > method handles, even though no checked exceptions can be thrown. > > The patch fixes this by instead wrapping method handles for which it is not > possible to eagerly determine whether they throw exception using the > `catchException` combinator, with an exception handler that catches checked > exceptions and instead throws an `IllegalStateException` with the original > exception as the cause. (See also the CSR). > > The main motivation for doing this is having the ability to share the > implementation with the `CLinker::upcallStub` API, which also wants to > eagerly reject method handles that are determined to throw exceptions. > > See also the original review for the panama-foreign repo: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. Deferred until a better solution is available. - PR: https://git.openjdk.java.net/jdk/pull/4395
Re: RFR: JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed as SCR [v2]
On Tue, 8 Jun 2021 14:06:50 GMT, Andy Herrick wrote: >> JDK-8267764: jpackage cannot handle window screensaver files when EXE >> renamed as SCR > > Andy Herrick 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 122 additional > commits since the last revision: > > - JDK-8267764: jpackage cannot handle window screensaver files when EXE > renamed as SCR > - Merge branch 'master' into JDK-8267764 > - Merge branch 'master' into JDK-8267764 > - 8191441: (Process) add Readers and Writer access to java.lang.Process > streams > >Reviewed-by: naoto, alanb > - 8261549: Adjust memory size in MTLTexurePool.m > >Reviewed-by: prr > - 8268299: jvms tag produces incorrect URL > >Reviewed-by: iris, erikj, jjg > - 8254129: IR Test Framework to support regex-based matching on the IR in > JTreg compiler tests > >Co-authored-by: Christian Hagedorn >Co-authored-by: Tobias Hartmann >Reviewed-by: iignatyev > - 8268331: Fix crash in humongous object eager reclaim logging > >Reviewed-by: sjohanss > - 8267875: Shenandoah: Duplicated code in > ShenandoahBarrierSetC2::ideal_node() > >Reviewed-by: rkennke, roland > - 8268267: Remove -Djavatest.security.noSecurityManager=true from jtreg runs > >Reviewed-by: lancea, jjg, erikj > - ... and 112 more: > https://git.openjdk.java.net/jdk/compare/95c2968d...7ad71791 Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4306
Re: RFR: 8268192: LambdaMetafactory with invokespecial causes VerificationError [v2]
On Tue, 8 Jun 2021 02:23:09 GMT, liach
wrote:
>> Dan Smith has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Clean up validation of implKind REF_invokeSpecial
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
> line 191:
>
>> 189: useImplMethodHandle =
>> (Modifier.isProtected(implInfo.getModifiers()) &&
>> 190:!VerifyAccess.isSamePackage(implClass,
>> implInfo.getDeclaringClass())) ||
>> 191:implKind == H_INVOKESPECIAL;
>
> Won't this make regular private instance method calls use condy as well, as
> they are invokespecial?
See this code from the `AbstractValidatingLambdaMetafactory` constructor:
case REF_invokeSpecial:
// JDK-8172817: should use referenced class here, but we don't
know what it was
this.implClass = implInfo.getDeclaringClass();
this.implIsInstanceMethod = true;
// Classes compiled prior to dynamic nestmate support invokes a
private instance
// method with REF_invokeSpecial.
//
// invokespecial should only be used to invoke private nestmate
constructors.
// The lambda proxy class will be defined as a nestmate of
targetClass.
// If the method to be invoked is an instance method of
targetClass, then
// convert to use invokevirtual or invokeinterface.
if (targetClass == implClass &&
!implInfo.getName().equals("")) {
this.implKind = implClass.isInterface() ?
REF_invokeInterface : REF_invokeVirtual;
} else {
this.implKind = REF_invokeSpecial;
}
break;
We turn all same-class invocations into `invokevirtual`. (And all ``
invocations have kind `newInvokeSpecial`, mentioning them here is actually
useless.)
-
PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8268192: LambdaMetafactory with invokespecial causes VerificationError [v2]
On Tue, 8 Jun 2021 15:17:44 GMT, Dan Smith wrote:
>> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
>> line 191:
>>
>>> 189: useImplMethodHandle =
>>> (Modifier.isProtected(implInfo.getModifiers()) &&
>>> 190:!VerifyAccess.isSamePackage(implClass,
>>> implInfo.getDeclaringClass())) ||
>>> 191:implKind == H_INVOKESPECIAL;
>>
>> Won't this make regular private instance method calls use condy as well, as
>> they are invokespecial?
>
> See this code from the `AbstractValidatingLambdaMetafactory` constructor:
>
>
> case REF_invokeSpecial:
> // JDK-8172817: should use referenced class here, but we
> don't know what it was
> this.implClass = implInfo.getDeclaringClass();
> this.implIsInstanceMethod = true;
>
> // Classes compiled prior to dynamic nestmate support invokes
> a private instance
> // method with REF_invokeSpecial.
> //
> // invokespecial should only be used to invoke private
> nestmate constructors.
> // The lambda proxy class will be defined as a nestmate of
> targetClass.
> // If the method to be invoked is an instance method of
> targetClass, then
> // convert to use invokevirtual or invokeinterface.
> if (targetClass == implClass &&
> !implInfo.getName().equals("")) {
> this.implKind = implClass.isInterface() ?
> REF_invokeInterface : REF_invokeVirtual;
> } else {
> this.implKind = REF_invokeSpecial;
> }
> break;
>
>
> We turn all same-class invocations into `invokevirtual`. (And all ``
> invocations have kind `newInvokeSpecial`, mentioning them here is actually
> useless.)
Actually, I think I'll fix up this code and the comment...
-
PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8268192: LambdaMetafactory with invokespecial causes VerificationError [v2]
On Tue, 8 Jun 2021 15:27:04 GMT, Dan Smith wrote:
>> Small bug fix.
>
> Dan Smith has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clean up validation of implKind REF_invokeSpecial
src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
line 160:
> 158: // method with REF_invokeSpecial. Newer classes use
> REF_invokeVirtual or
> 159: // REF_invokeInterface, and we can use that instruction
> in the lambda class.
> 160: if (targetClass == implClass) {
The name `` can't appear here. It's only used by `newInvokeSpecial`.
src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
line 189:
> 187: // to invoke directly. (javac prefers to avoid this situation by
> 188: // generating bridges in the target class)
> 189: useImplMethodHandle =
> (Modifier.isProtected(implInfo.getModifiers()) &&
Switched from `!Modifier.isPublic` to `Modifier.isProtected`. Access errors
from the caller Lookup should already be caught by validation when the
`implementation` is cracked. Protected and `super` calls are the only cases
where the access from the nestmate is not equivalent.
-
PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8268192: LambdaMetafactory with invokespecial causes VerificationError [v2]
> Small bug fix. Dan Smith has updated the pull request incrementally with one additional commit since the last revision: Clean up validation of implKind REF_invokeSpecial - Changes: - all: https://git.openjdk.java.net/jdk/pull/4403/files - new: https://git.openjdk.java.net/jdk/pull/4403/files/74c346a1..149fb55b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4403=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4403=00-01 Stats: 8 lines in 1 file changed: 0 ins; 4 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4403.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4403/head:pull/4403 PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8240349: jlink should not leave partial image output directory on failure [v2]
On Tue, 8 Jun 2021 14:29:38 GMT, Athijegannathan Sundararajan wrote: >> jlink should clean up output directory on any failure. should not leave >> partially filled output. > > Athijegannathan Sundararajan has updated the pull request incrementally with > one additional commit since the last revision: > > Fixed resouce closing issue for lib/moduls file. Pre-existing output > directory should not be deleted when exiting. Marked as reviewed by jlaskey (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4386
Integrated: 8268083: JDK-8267706 breaks bin/idea.sh on a Mac
On Fri, 4 Jun 2021 21:23:27 GMT, Nikita Gubarkov wrote: > I got rid of `realpath` usage as discussed in > https://github.com/openjdk/jdk/pull/4190 and used `RelativePath` macro > instead, however there were quite a few problems with this macro, here's the > example: > > $(call RelativePath,/foo/bar,/foo/bar/baz) -> "..//foo/bar" > $(call RelativePath,/foo/bar/baz/,/foo/bar/baz) -> SEGFAULT > $(call RelativePath,/foo/bar/baz/banan,/foo/bar/) -> "./baz/banan" > $(call RelativePath,/foo/bar/baz,/foo/bar/banan) -> "../baz" > > As you can see, 1st case is just plain wrong, 2nd crashes make because of > infinite loop, 3rd can be simplified and all of them have leading whitespaces > First commit in this PR fixes all these issues and adds corresponding test > cases and second commit replaces usage of `realpath` in idea.sh with > `RelativePath` macro in idea.gmk and fixes problems, when paths are > incorrectly treated by IDEA This pull request has now been integrated. Changeset: 159cb6fa Author:Nikita Gubarkov Committer: Alexey Ushakov URL: https://git.openjdk.java.net/jdk/commit/159cb6facc668acc30552665e46b18edf58c3a91 Stats: 219 lines in 11 files changed: 107 ins; 47 del; 65 mod 8268083: JDK-8267706 breaks bin/idea.sh on a Mac Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/4369
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v15]
On Tue, 8 Jun 2021 14:26:43 GMT, Roger Riggs wrote: >> JEP 415: Context-specific Deserialization Filters extends the >> deserialization filtering mechanisms with more flexible and customizable >> protections against malicious deserialization. See JEP 415: >> https://openjdk.java.net/jeps/415. >> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are >> extended with additional >> configuration mechanisms and filter utilities. >> >> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and >> `ObjectInputStream`: >> >> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify exceptions that occur if initializing the filter factory from > jdk.serialFilterFactory fails Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v15]
On Tue, 8 Jun 2021 14:26:43 GMT, Roger Riggs wrote: >> JEP 415: Context-specific Deserialization Filters extends the >> deserialization filtering mechanisms with more flexible and customizable >> protections against malicious deserialization. See JEP 415: >> https://openjdk.java.net/jeps/415. >> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are >> extended with additional >> configuration mechanisms and filter utilities. >> >> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and >> `ObjectInputStream`: >> >> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify exceptions that occur if initializing the filter factory from > jdk.serialFilterFactory fails Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8240349: jlink should not leave partial image output directory on failure [v2]
> jlink should clean up output directory on any failure. should not leave > partially filled output. Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision: Fixed resouce closing issue for lib/moduls file. Pre-existing output directory should not be deleted when exiting. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4386/files - new: https://git.openjdk.java.net/jdk/pull/4386/files/f12e6a29..df036a3a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4386=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4386=00-01 Stats: 48 lines in 5 files changed: 22 ins; 12 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/4386.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4386/head:pull/4386 PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v14]
On Tue, 8 Jun 2021 11:41:28 GMT, Daniel Fuchs wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Clarified javadoc for rejectUndecidedClass.
>> Added javadoc to describe throwing of ExceptionInInitializerError if the
>> class
>> named by system property jdk.serialFilterFactory is not valid.
>> Added description of jdk.serialFilterFactory to java.security file.
>
> src/java.base/share/classes/java/io/ObjectInputFilter.java line 550:
>
>> 548: * be accessible via the {@linkplain
>> ClassLoader#getSystemClassLoader() application class loader}.
>> 549: * If the filter factory constructor is not invoked successfully,
>> an {@link ExceptionInInitializerError}
>> 550: * is thrown.
>
> Should you also say that later attempts to create an `ObjectInputStream` or
> to call `ObjectInputStream::setObjectInputFilter` will result in an
> `IllegalStateException`?
Yes, and setObjectInputFilter should throw ISE if the initialization from the
system property has failed.
```
* If the filter factory constructor is not invoked successfully, an {@link
ExceptionInInitializerError}
* is thrown and subsequent use of the filter factory for deserialization
fails with
* {@link IllegalStateException}.
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v15]
> JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Clarify exceptions that occur if initializing the filter factory from jdk.serialFilterFactory fails - Changes: - all: https://git.openjdk.java.net/jdk/pull/3996/files - new: https://git.openjdk.java.net/jdk/pull/3996/files/755d8f20..d96d4e3e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3996=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3996=13-14 Stats: 7 lines in 1 file changed: 5 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996 PR: https://git.openjdk.java.net/jdk/pull/3996
Integrated: 8268327: Upstream: 8268169: The system lookup can not find stdio functions such as printf on Windows 10
On Mon, 7 Jun 2021 13:23:46 GMT, Jorn Vernee wrote: > Hi, > > The documentation of `CLinker::systemLookup` [1] says this: > > > * Obtains a system lookup which is suitable to find symbols in the standard C > libraries. > > > However, currently it is not possible to look up common stdio.h symbols, such > as `printf`, using the system lookup on Windows 10. This is because the > backing library that is loaded, namely `ucrtbase.dll`, does not contain these > symbols, as they are implemented in the standard library as inline functions. > > This patch changes the system lookup implementation on Windows 10 to make > these symbols findable as well, by using a fallback library that forces the > generation of the code for the inline functions, and exposes the missing > symbols as a table of function pointers. > > See also the prior review of this patch for the panama-foreign repo: > https://github.com/openjdk/panama-foreign/pull/547 > > Since the exact set of symbols findable by the system lookup is unspecified, > and since the API in question (`CLinker::systemLookup`) was added only last > week [2], I've elected to not create a CSR for this patch, but please let me > know if one is needed). > > Thanks, > Jorn > > [1] : > https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java#L134-L151 > [2] : > https://github.com/openjdk/jdk/commit/a223189b069a7cfe49511d49b5b09e7107cb3cab This pull request has now been integrated. Changeset: 8158b822 Author:Jorn Vernee URL: https://git.openjdk.java.net/jdk/commit/8158b82269513a60c13bb10a6edfa82f806e8efc Stats: 273 lines in 5 files changed: 203 ins; 58 del; 12 mod 8268327: Upstream: 8268169: The system lookup can not find stdio functions such as printf on Windows 10 Reviewed-by: erikj, sundar - PR: https://git.openjdk.java.net/jdk/pull/4390
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v3]
On Tue, 8 Jun 2021 13:25:00 GMT, Scott Gibbons wrote: >> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 6239: >> >>> 6237: >>> 6238: __ align(32); >>> 6239: __ BIND(L_bruteForce); >> >> Is this alignment needed ? Given that brute force loop is already aligned. > > I must be missing something. How is the brute force loop aligned if not by > this directive? I don't see an alignment anywhere else that could force it. > After the entry(), there are pushes and length comparisons followed by the > conditional on VBMI. The only thing I can guess would be that the jmp > aligns, but I see no indication that that occurs. > > Perhaps what you missed was that L_forceLoop is aligned (line 6288). This is > not the same label as L_bruteForce, which is a jump target from within the > VBMI conditional (which should be aligned)? Otherwise, I don't see how > L_bruteForce could possibly already be aligned. Yes, I meant force loop already has alignment so earlier one can be removed. - PR: https://git.openjdk.java.net/jdk/pull/4368
Re: RFR: JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed as SCR [v2]
> JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed > as SCR Andy Herrick 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 122 additional commits since the last revision: - JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed as SCR - Merge branch 'master' into JDK-8267764 - Merge branch 'master' into JDK-8267764 - 8191441: (Process) add Readers and Writer access to java.lang.Process streams Reviewed-by: naoto, alanb - 8261549: Adjust memory size in MTLTexurePool.m Reviewed-by: prr - 8268299: jvms tag produces incorrect URL Reviewed-by: iris, erikj, jjg - 8254129: IR Test Framework to support regex-based matching on the IR in JTreg compiler tests Co-authored-by: Christian Hagedorn Co-authored-by: Tobias Hartmann Reviewed-by: iignatyev - 8268331: Fix crash in humongous object eager reclaim logging Reviewed-by: sjohanss - 8267875: Shenandoah: Duplicated code in ShenandoahBarrierSetC2::ideal_node() Reviewed-by: rkennke, roland - 8268267: Remove -Djavatest.security.noSecurityManager=true from jtreg runs Reviewed-by: lancea, jjg, erikj - ... and 112 more: https://git.openjdk.java.net/jdk/compare/583e2df7...7ad71791 - Changes: - all: https://git.openjdk.java.net/jdk/pull/4306/files - new: https://git.openjdk.java.net/jdk/pull/4306/files/cfd8ce4d..7ad71791 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4306=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4306=00-01 Stats: 484800 lines in 1905 files changed: 468469 ins; 10878 del; 5453 mod Patch: https://git.openjdk.java.net/jdk/pull/4306.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4306/head:pull/4306 PR: https://git.openjdk.java.net/jdk/pull/4306
Re: RFR: JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed as SCR
On Wed, 2 Jun 2021 13:48:47 GMT, Andy Herrick wrote: > JDK-8267764: jpackage cannot handle window screensaver files when EXE renamed > as SCR as suggested added a simple test to verify an app on windows can continue to run after ".exe" is renamed to any other extension - PR: https://git.openjdk.java.net/jdk/pull/4306
Integrated: 8268227: java/foreign/TestUpcall.java still times out
On Fri, 4 Jun 2021 10:53:42 GMT, Maurizio Cimadamore wrote: > Turns out that adding more timeout is a lost cause here. The root cause of > the slowdown when running the test in debug build is: > > https://bugs.openjdk.java.net/browse/JDK-8266074 > > Which has also caused related test issues: > > https://bugs.openjdk.java.net/browse/JDK-8268095 > > So, the fix (at least temporarily) is to run method handle-heavy tests with > the -XX:-VerifyDependency options. > > On my machine, execution time of these tests on debug goes from 10 minutes > down to less than 1. > > Since `-XX:-VerifyDependencies` cannot be specified on non-debug build, the > `-XX:+IgnoreUnrecognizedVMOptions` is also passed (thanks Vlad for the tip!). This pull request has now been integrated. Changeset: 6843576c Author:Maurizio Cimadamore URL: https://git.openjdk.java.net/jdk/commit/6843576c95a70bffad95df278d5f5be29371bca4 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod 8268227: java/foreign/TestUpcall.java still times out Reviewed-by: dcubed - PR: https://git.openjdk.java.net/jdk/pull/4355
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote:
>> …d on macOS
>>
>> This patch simply round up the specified stack size to multiple of the
>> system page size.
>>
>> Test is trivial, simply run java with -Xss option against following code. On
>> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get
>> `7183` and `649` respectively. After fix, both would output `649`, while
>> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
>>
>> ```code:java
>> public class StackLeak {
>> public int depth = 0;
>> public void stackLeak() {
>> depth++;
>> stackLeak();
>> }
>>
>> public static void main(String[] args) {
>> var test = new StackLeak();
>> try {
>> test.stackLeak();
>> } catch (Throwable e) {
>> System.out.println(test.depth);
>> }
>> }
>> }
>
> Henry Jen 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 seven additional commits since
> the last revision:
>
> - Cast type
> - Merge
> - Change java -X output for -Xss
> - Merge
> - Only try to round-up when current value failed
> - Avoid overflow on page size
> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on
> macOS
Please make sure the failing tests have nothing to do with your patch.
`gc/shenandoah/compiler/TestLinkToNativeRBP.java`
sounds at least suggestive.
-
PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8268276: Base64 Decoding optimization for x86 using AVX-512 [v3]
On Tue, 8 Jun 2021 01:56:42 GMT, Jatin Bhateja wrote: >> Scott Gibbons has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixing review comments. Adding notes about isMIME parameter for other >> architectures; clarifying decodeBlock comments. > > src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 6239: > >> 6237: >> 6238: __ align(32); >> 6239: __ BIND(L_bruteForce); > > Is this alignment needed ? Given that brute force loop is already aligned. I must be missing something. How is the brute force loop aligned if not by this directive? I don't see an alignment anywhere else that could force it. After the entry(), there are pushes and length comparisons followed by the conditional on VBMI. The only thing I can guess would be that the jmp aligns, but I see no indication that that occurs. - PR: https://git.openjdk.java.net/jdk/pull/4368
Re: RFR: 8240349: jlink --vm with not present VM does not fail fast
On Tue, 8 Jun 2021 11:32:29 GMT, Alan Bateman wrote:
>> jlink should clean up output directory on any failure. should not leave
>> partially filled output.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 295:
>
>> 293: cleanupOutput(outputPath);
>> 294: return EXIT_ERROR;
>> 295: } catch (BadArgs e) {
>
> Can you confirm that this change is benign for the case that the output
> directory exists? I believe it's a BadArgs case so it won't attempt to delete
> a file or tree if it exists but I'd like confirmation.
Will check that.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 490:
>
>> 488:
>> 489: private static void deleteDirectory(Path dir) throws IOException {
>> 490: if (dir != null && Files.isDirectory(dir)) {
>
> I don't think this method should be checking dir, that should be up to the
> caller to ensure that it is not called when output is null.
okay. will move that check to caller.
-
PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8240349: jlink --vm with not present VM does not fail fast
On Tue, 8 Jun 2021 12:20:32 GMT, Jorn Vernee wrote:
> WRT the test failure on Windows discussed offline: when the directory is
> deleted as a result of a `PluginException` being thrown, there is still an
> open file handle on the `lib/modules` file in the image directory, which
> prevents the directory from being deleted.
>
> Bisecting this, it seems that the file handle is being created in
> `ImageFileCreater::writeImage` with a call to
> `plugins.getJImageFileOutputStream`
> (https://github.com/openjdk/jdk/blob/master/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java#L162).
> This creates an output stream that is never closed.
>
> Following patch fixes:
>
> ```
> diff --git
> a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> index 749025bea9d..8beddc5a037 100644
> ---
> a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> +++
> b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> @@ -158,8 +158,10 @@ public final class ImageFileCreator {
> BasicImageWriter writer = new BasicImageWriter(byteOrder);
> ResourcePoolManager allContent = createPoolManager(archives,
> entriesForModule, byteOrder, writer);
> -ResourcePool result = generateJImage(allContent,
> - writer, plugins, plugins.getJImageFileOutputStream());
> +ResourcePool result;
> +try (DataOutputStream out = plugins.getJImageFileOutputStream()) {
> +result = generateJImage(allContent, writer, plugins, out);
> +}
>
> //Handle files.
> try {
> ```
Thanks @JornVernee
-
PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8265909 : build.tools.dtdbuilder.DTDBuilder.java failed detecting missing path of dtd_home
On Thu, 22 Apr 2021 13:47:03 GMT, ScientificWare
wrote:
> This concerns [dtdbuilder
> tools](https://github.com/openjdk/jdk/tree/master/make/jdk/src/classes/build/tools/dtdbuilder).
>
> In jshell, try `System.getProperty("html32") + ""` you'll get a `String`.
>
> So, in `DTDBuilder.java` when none `dtd_home` property is set, the `dtd_home`
> string value is not `null`, causing an exception with the present test.
>
> The expected value is `"Must set property 'dtd_home'"`
>
> And in this case, should'nt we have a `System.exit(1)` too rather than a
> simple `return` ?
make/jdk/src/classes/build/tools/dtdbuilder/DTDBuilder.java line 287:
> 285:
> 286: String dtd_home = System.getProperty("dtd_home") +
> File.separator;
> 287: if (dtd_home.equals("null" + File.separator)) {
dtd_home is only used in one location, so instead of changing this null check,
maybe just move the "+ File.separator" to line 295?
-
PR: https://git.openjdk.java.net/jdk/pull/3626
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 06:11:17 GMT, Alan Bateman wrote: >> More loudly and precise warning messages when a security manager is either >> enabled at startup or installed at runtime. > > src/java.base/share/classes/java/lang/System.java line 331: > >> 329: >> 330: // Remember original System.err. setSecurityManager() warning goes >> here >> 331: private static PrintStream oldErrStream = null; > > I assume this should needs to be volatile and @Stable. I think we need a > better name for it too. Will add the modifiers. How about "originalErr"? > src/java.base/share/classes/java/lang/System.java line 336: > >> 334: // Remember callers of setSecurityManager() here so that warning >> 335: // is only printed once for each different caller >> 336: final static Map callersOfSSM = new >> WeakHashMap<>(); > > You can't use a WeakHashMap without synchronization but a big question here > is whether a single caller frame is sufficient. If I were doing this then I > think I would capture the hash of a number of stack frames to create a better > filter. I thought about that but not sure of performance impact. Is the worst problem that more than one warnings will be printed for a single caller? It's not really harmless. As for the frame, if the warning message only contain the caller class name and its code source, why is it worth using a key of multiple frames? The message will look the same. > src/java.base/share/classes/java/lang/System.java line 2219: > >> 2217: WARNING: java.lang.SecurityManager is >> deprecated and will be removed in a future release >> 2218: WARNING: -Djava.security.manager=%s >> will have no effect when java.lang.SecurityManager is removed >> 2219: """, smProp); > > Raw strings may be useful here but means the lines length are inconsistent > and makes it too hard to look at side by side diffs now. I understand what you mean when I switch to Split View. While I can extract the lines to a method, I somehow think it's not worth doing because for each type of warning the method is only called once. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8240349: jlink --vm with not present VM does not fail fast
On Mon, 7 Jun 2021 11:00:00 GMT, Athijegannathan Sundararajan
wrote:
> jlink should clean up output directory on any failure. should not leave
> partially filled output.
WRT the test failure on Windows discussed offline: when the directory is
deleted as a result of a `PluginException` being thrown, there is still an open
file handle on the `lib/modules` file in the image directory, which prevents
the directory from being deleted.
Bisecting this, it seems that the file handle is being created in
`ImageFileCreater::writeImage` with a call to
`plugins.getJImageFileOutputStream`
(https://github.com/openjdk/jdk/blob/master/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java#L162).
This creates an output stream that is never closed.
Following patch fixes:
diff --git
a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
index 749025bea9d..8beddc5a037 100644
--- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
+++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
@@ -158,8 +158,10 @@ public final class ImageFileCreator {
BasicImageWriter writer = new BasicImageWriter(byteOrder);
ResourcePoolManager allContent = createPoolManager(archives,
entriesForModule, byteOrder, writer);
-ResourcePool result = generateJImage(allContent,
- writer, plugins, plugins.getJImageFileOutputStream());
+ResourcePool result;
+try (DataOutputStream out = plugins.getJImageFileOutputStream()) {
+result = generateJImage(allContent, writer, plugins, out);
+}
//Handle files.
try {
-
PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 06:41:11 GMT, Alan Bateman wrote: > > You might want to make your "WARNING" consistent with the VM's "Warning" so > > that OutputAnalyzer's logic to ignore warnings will automatically ignore > > these too. > > The uppercase "WARNING" is intentional here, it was the same with illegal > reflective access warnings. I'm sure Max has or will run all tests to see if > there are any issues. Will definitely run all from tier1-tier9. I ran them multiple times while implementing JEP 411. I've seen warnings with "VM" word in the prefix and test methods that filter them out, but feel the warnings here are not related to VM. The new warnings do have impacts on some tests and I'll be very carefully not break them. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v14]
On Tue, 8 Jun 2021 10:32:49 GMT, Roger Riggs wrote:
>> JEP 415: Context-specific Deserialization Filters extends the
>> deserialization filtering mechanisms with more flexible and customizable
>> protections against malicious deserialization. See JEP 415:
>> https://openjdk.java.net/jeps/415.
>> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are
>> extended with additional
>> configuration mechanisms and filter utilities.
>>
>> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and
>> `ObjectInputStream`:
>>
>> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clarified javadoc for rejectUndecidedClass.
> Added javadoc to describe throwing of ExceptionInInitializerError if the
> class
> named by system property jdk.serialFilterFactory is not valid.
> Added description of jdk.serialFilterFactory to java.security file.
src/java.base/share/classes/java/io/ObjectInputFilter.java line 392:
> 390: * Returns a filter that invokes a given filter and maps {@code
> UNDECIDED} to {@code REJECTED}
> 391: * for classes, with some special cases, and otherwise returns the
> status.
> 392: * If the class is not a primitive class and not an array, the
> status returned is REJECTED.
{@code REJECTED}
src/java.base/share/classes/java/io/ObjectInputFilter.java line 550:
> 548: * be accessible via the {@linkplain
> ClassLoader#getSystemClassLoader() application class loader}.
> 549: * If the filter factory constructor is not invoked successfully, an
> {@link ExceptionInInitializerError}
> 550: * is thrown.
Should you also say that later attempts to create an `ObjectInputStream` or to
call `ObjectInputStream::setObjectInputFilter` will result in an
`IllegalStateException`?
-
PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8240349: jlink --vm with not present VM does not fail fast
On Mon, 7 Jun 2021 11:00:00 GMT, Athijegannathan Sundararajan
wrote:
> jlink should clean up output directory on any failure. should not leave
> partially filled output.
I think we need to rename the JBS issue as this is not specific to the -vm
option. Instead, this is about cleanup when jlink fails with some intermediate
files in the output directory.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 295:
> 293: cleanupOutput(outputPath);
> 294: return EXIT_ERROR;
> 295: } catch (BadArgs e) {
Can you confirm that this change is benign for the case that the output
directory exists? I believe it's a BadArgs case so it won't attempt to delete a
file or tree if it exists but I'd like confirmation.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 490:
> 488:
> 489: private static void deleteDirectory(Path dir) throws IOException {
> 490: if (dir != null && Files.isDirectory(dir)) {
I don't think this method should be checking dir, that should be up to the
caller to ensure that it is not called when output is null.
-
PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Mon, 7 Jun 2021 20:42:53 GMT, Weijun Wang wrote: > More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. Changes to LoggerFinderLoaderTest look reasonable to me. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v14]
> JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Clarified javadoc for rejectUndecidedClass. Added javadoc to describe throwing of ExceptionInInitializerError if the class named by system property jdk.serialFilterFactory is not valid. Added description of jdk.serialFilterFactory to java.security file. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3996/files - new: https://git.openjdk.java.net/jdk/pull/3996/files/6d07298f..755d8f20 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3996=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3996=12-13 Stats: 19 lines in 2 files changed: 9 ins; 1 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996 PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8266054: VectorAPI rotate operation optimization [v8]
> Current VectorAPI Java side implementation expresses rotateLeft and > rotateRight operation using following operations:- > > vec1 = lanewise(VectorOperators.LSHL, n) > vec2 = lanewise(VectorOperators.LSHR, n) > res = lanewise(VectorOperations.OR, vec1 , vec2) > > This patch moves above handling from Java side to C2 compiler which > facilitates dismantling the rotate operation if target ISA does not support a > direct rotate instruction. > > AVX512 added vector rotate instructions vpro[rl][v][dq] which operate over > long and integer type vectors. For other cases (i.e. sub-word type vectors or > for targets which do not support direct rotate operations ) instruction > sequence comprising of vector SHIFT (LEFT/RIGHT) and vector OR is emitted. > > Please find below the performance data for included JMH benchmark. > Machine: Cascade Lake Server (Intel(R) Xeon(R) Platinum 8280 CPU @ 2.70GHz) > > > Benchmark | (TESTSIZE) | Shift | Baseline AVX3 (ops/ms) | Withopt AVX3 > (ops/ms) | Gain % | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain % > -- | -- | -- | -- | -- | -- | -- | -- | -- > | | | | | | | | > RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 17223.35 | 17094.69 | -0.75 > | 17008.32 | 17488.06 | 2.82 > RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 8944.98 | 8811.34 | -1.49 | > 8878.17 | 9218.68 | 3.84 > RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 17195.75 | 17137.32 | > -0.34 | 16789.01 | 17780.34 | 5.90 > RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 9052.67 | 8838.60 | -2.36 > | 8814.62 | 9206.01 | 4.44 > RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 17100.19 | 16950.64 | > -0.87 | 16827.73 | 17720.37 | 5.30 > RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 9079.95 | 8471.26 | -6.70 > | .44 | 9167.68 | 3.14 > RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 21231.33 | 21513.08 | 1.33 > | 21824.51 | 21479.48 | -1.58 > RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 11103.62 | 11180.16 | 0.69 > | 11173.67 | 11529.22 | 3.18 > RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 21119.14 | 21552.04 | 2.05 > | 21693.05 | 21915.37 | 1.02 > RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 11048.68 | 11094.20 | 0.41 > | 11049.90 | 11439.07 | 3.52 > RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 21506.31 | 21391.41 | > -0.53 | 21263.18 | 21986.29 | 3.40 > RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 11056.12 | 11232.78 | 1.60 > | 10941.59 | 11397.09 | 4.16 > RotateBenchmark.testRotateLeftB | 512.00 | 7.00 | 17976.56 | 18180.85 | 1.14 > | 1212.26 | 2533.34 | 108.98 > RotateBenchmark.testRotateLeftB | 512.00 | 15.00 | 17553.70 | 18219.07 | 3.79 > | 1256.73 | 2537.41 | 101.91 > RotateBenchmark.testRotateLeftB | 512.00 | 31.00 | 17618.03 | 17738.15 | 0.68 > | 1214.69 | 2533.83 | 108.60 > RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 7258.87 | 7468.88 | 2.89 | > 7115.12 | 7117.26 | 0.03 > RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 3586.65 | 3950.85 | 10.15 | > 3532.17 | 3595.80 | 1.80 > RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 1835.07 | 1999.68 | 8.97 | > 1789.90 | 1819.93 | 1.68 > RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 7273.36 | 7410.91 | 1.89 | > 7198.60 | 6994.79 | -2.83 > RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 3674.98 | 3926.27 | 6.84 | > 3549.90 | 3755.09 | 5.78 > RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 1840.94 | 1882.25 | 2.24 | > 1801.56 | 1872.89 | 3.96 > RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 7457.11 | 7361.48 | -1.28 > | 6975.33 | 7385.94 | 5.89 > RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 3570.74 | 3929.30 | 10.04 > | 3635.37 | 3736.67 | 2.79 > RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 1902.32 | 1960.46 | 3.06 | > 1812.32 | 1813.88 | 0.09 > RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 11174.24 | 12044.52 | 7.79 > | 11509.87 | 11273.44 | -2.05 > RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 5981.47 | 6073.70 | 1.54 | > 5593.66 | 5661.93 | 1.22 > RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 2932.49 | 3069.54 | 4.67 | > 2950.86 | 2892.42 | -1.98 > RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 11764.11 | 12098.63 | 2.84 > | 11069.52 | 11476.93 | 3.68 > RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 5855.20 | 6080.40 | 3.85 | > 5919.11 | 5607.04 | -5.27 > RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 2989.05 | 3048.56 | 1.99 | > 2902.63 | 2821.83 | -2.78 > RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 11652.84 | 11965.40 | 2.68 > | 11525.62 | 11459.83 | -0.57 > RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 5851.82 | 6164.94 | 5.35 | > 5882.60 | 5842.30 | -0.69 > RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 3015.99 | 3043.79 | 0.92 | > 2963.71 | 2947.97 | -0.53 > RotateBenchmark.testRotateLeftI | 512.00 | 7.00 | 16029.15 | 16189.79 | 1.00 > | 860.43 | 2339.32 | 171.88 > RotateBenchmark.testRotateLeftI | 512.00 | 7.00 | 8078.25 | 8081.84 | 0.04 | > 427.39
Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]
On Mon, 7 Jun 2021 18:46:11 GMT, Naoto Sato wrote: >> @mkartashev thank you for the detailed explanation. >> >> It is not clear to me that the JDK's conformance to being a Unicode >> application has significantly changed since the evaluation of JDK-8017274 - >> @naotoj can you comment on that and related discussion from the CCC for >> JDK-4958170 ? In particular I'm not sure that using the platform encoding is >> wrong, nor how we can have a path that cannot be represented by the platform >> encoding? >> >> Not being an expert in this area I cannot evaluate the affects of these >> shared code changes on other platforms, and so am reluctant to introduce any >> change that affects any non-Windows platforms. Also the JVM and JNI work >> with modified-UTF8 so I do not think we should diverge from that. >> I would hate to see windows specific code introduced into the JDK or JVM's >> shared code for these APIs, but that may be the only choice to avoid >> potential disruption to other platforms. Though perhaps we could push the >> initial conversion down into the JVM? > > @dholmes-ora Sorry, I don't think anything has changed as to the encoding as > of JDK-8017274. For some reason, I had the impression that JVM_LoadLibrary() > accepts UTF-8 (either modified or standard), but that was not correct. It is > using the platform encoded string for the pathname. > > @mkartashev As you mentioned in another comment, the only way to fix this > issue is to pass UTF-8 down to JVM_LoadLibray, but I don't think it is > feasible. One reason is the effort is too great, and the other is that all VM > implementations would need to be modified. @naotoj Then I guess this bug will have to wait until Windows evolves to the point when its platform encoding is UTF-8. In the mean time, I'm closing this PR. Thank you all so much for your time! - PR: https://git.openjdk.java.net/jdk/pull/4169
Withdrawn: 8195129: System.load() fails to load from unicode paths
On Mon, 24 May 2021 16:43:09 GMT, Maxim Kartashev wrote: > Character strings within JVM are produced and consumed in several formats. > Strings come from/to Java in the UTF8 format and POSIX APIs (like fprintf() > or dlopen()) consume strings also in UTF8. On Windows, however, the situation > is far less simple: some new(er) APIs expect UTF16 (wide-character strings), > some older APIs can only work with strings in a "platform" format, where not > all UTF8 characters can be represented; which ones can depends on the current > "code page". > > This commit switches the Windows version of native library loading code to > using the new UTF16 API `LoadLibraryW()` and attempts to streamline the use > of various string formats in the surrounding code. > > Namely, exception messages are made to consume strings explicitly in the UTF8 > format, while logging functions (that end up using legacy Windows API) are > made to consume "platform" strings in most cases. One exception is > `JVM_LoadLibrary()` logging where the UTF8 name of the library is logged, > which can, of course, be fixed, but was considered not worth the additional > code (NB: this isn't a new bug). > > The test runs in a separate JVM in order to make NIO happy about non-ASCII > characters in the file name; tests are executed with LC_ALL=C and that > doesn't let NIO work with non-ASCII file names even on Linux or MacOS. > > Tested by running `test/hotspot/jtreg:tier1` on Linux and > `jtreg:test/hotspot/jtreg/runtime` on Windows 10. The new test (` > jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`) was explicitly ran > on those platforms as well. > > Results from Linux: > > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/hotspot/jtreg:tier1 1784 1784 0 0 > > == > TEST SUCCESS > > > Building target 'run-test-only' in configuration 'linux-x86_64-server-release' > Test selection 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', > will run: > * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode > > Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' > Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java > Test results: passed: 1 > > > Results from Windows 10: > > Test summary > == >TEST TOTAL PASS FAIL ERROR >jtreg:test/hotspot/jtreg/runtime746 746 0 0 > == > TEST SUCCESS > Finished building target 'run-test-only' in configuration > 'windows-x86_64-server-fastdebug' > > > Building target 'run-test-only' in configuration > 'windows-x86_64-server-fastdebug' > Test selection 'test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', will run: > * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode > > Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode' > Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java > Test results: passed: 1 This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/4169
Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException
On Sun, 9 May 2021 21:59:29 GMT, Rafael Winterhalter wrote: > During annotation parsing, the parser assumes that a declared property is of > an array type if the parsed annotation property is defined as an array. In > this case, the component type is `null`, and a `NullPointerException` is > thrown. This change discovers the mismatch and throws an > `AnnotationTypeMismatchException`. Commenting on the issue to avoid closing as it is still under review. - PR: https://git.openjdk.java.net/jdk/pull/3940
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote:
>> …d on macOS
>>
>> This patch simply round up the specified stack size to multiple of the
>> system page size.
>>
>> Test is trivial, simply run java with -Xss option against following code. On
>> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get
>> `7183` and `649` respectively. After fix, both would output `649`, while
>> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
>>
>> ```code:java
>> public class StackLeak {
>> public int depth = 0;
>> public void stackLeak() {
>> depth++;
>> stackLeak();
>> }
>>
>> public static void main(String[] args) {
>> var test = new StackLeak();
>> try {
>> test.stackLeak();
>> } catch (Throwable e) {
>> System.out.println(test.depth);
>> }
>> }
>> }
>
> Henry Jen 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 seven additional commits since
> the last revision:
>
> - Cast type
> - Merge
> - Change java -X output for -Xss
> - Merge
> - Only try to round-up when current value failed
> - Avoid overflow on page size
> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on
> macOS
Hi,
proposed shorter form. Otherwise this looks fine.
Cheers, Thomas
src/java.base/unix/native/libjli/java_md.c line 666:
> 664: return page_size * pages;
> 665: }
> 666: }
Could probably be shortened to something like this:
size_t pagesize = (size_t)sysconf(_SC_PAGESIZE);
return (stack_size + (pagesize - 1)) & ~(pagesize - 1);
or, if you insist on checking for SIZE_MAX:
size_t pagesize = (size_t)sysconf(_SC_PAGESIZE);
size_t max = SIZE_MAX - pagesize;
return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) :
max;
(I see David requested this, so this is fine, though passing SIZE_MAX to this
function will quite likely fail anyway :)
-
PR: https://git.openjdk.java.net/jdk/pull/4256
Integrated: JDK-8266918: merge_stack in check_code.c add NULL check
On Tue, 11 May 2021 14:49:42 GMT, Matthias Baesken wrote: > Hello, please review this small Sonar finding. > Sonar reports a potential NULL pointer dereference here : > https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8CPLBBG2CXpcnh_z=false=MAJOR=BUG > "Access to field 'item' results in a dereference of a null pointer (loaded > from variable 'new')" > It would be better to add a check . This pull request has now been integrated. Changeset: 00c88f79 Author:Matthias Baesken URL: https://git.openjdk.java.net/jdk/commit/00c88f79b30d7867be4a66317b90b9ba7e947f4f Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8266918: merge_stack in check_code.c add NULL check Reviewed-by: rschmelter, clanger - PR: https://git.openjdk.java.net/jdk/pull/3979
Re: RFR: 8268113: Re-use Long.hashCode() where possible [v5]
> There is a few JDK classes duplicating the contents of Long.hashCode() for > hash code calculation. They should explicitly delegate to Long.hashCode(). Сергей Цыпанов 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 five additional commits since the last revision: - Merge branch 'master' into 8268113 - Merge branch 'master' into 8268113 - 8268113: Inline local vars where reasonable - 8268113: Delegate to Double.hashCode() - 8268113: Re-use Long.hashCode() where possible - Changes: - all: https://git.openjdk.java.net/jdk/pull/4309/files - new: https://git.openjdk.java.net/jdk/pull/4309/files/7dc5020e..76af35c6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4309=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4309=03-04 Stats: 30371 lines in 444 files changed: 25591 ins; 2648 del; 2132 mod Patch: https://git.openjdk.java.net/jdk/pull/4309.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4309/head:pull/4309 PR: https://git.openjdk.java.net/jdk/pull/4309
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Tue, 8 Jun 2021 06:30:00 GMT, David Holmes wrote: > You might want to make your "WARNING" consistent with the VM's "Warning" so > that OutputAnalyzer's logic to ignore warnings will automatically ignore > these too. The uppercase "WARNING" is intentional here, it was the same with illegal reflective access warnings. I'm sure Max has or will run all tests to see if there are any issues. - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Mon, 7 Jun 2021 20:42:53 GMT, Weijun Wang wrote: > More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. There are a number of hotspot tests that will trigger this warning, so please ensure they work correctly with the extra output. You might want to make your "WARNING" consistent with the VM's "Warning" so that OutputAnalyzer's logic to ignore warnings will automatically ignore these too. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/4400
Re: RFR: 8268227: java/foreign/TestUpcall.java still times out
On 8/06/2021 5:23 am, Daniel D.Daugherty wrote: On Fri, 4 Jun 2021 10:53:42 GMT, Maurizio Cimadamore wrote: Turns out that adding more timeout is a lost cause here. The root cause of the slowdown when running the test in debug build is: https://bugs.openjdk.java.net/browse/JDK-8266074 Which has also caused related test issues: https://bugs.openjdk.java.net/browse/JDK-8268095 So, the fix (at least temporarily) is to run method handle-heavy tests with the -XX:-VerifyDependency options. On my machine, execution time of these tests on debug goes from 10 minutes down to less than 1. Since `-XX:-VerifyDependencies` cannot be specified on non-debug build, the `-XX:+IgnoreUnrecognizedVMOptions` is also passed (thanks Vlad for the tip!). @mcimadamore - Can you please integrate this fix? I see this test failing on Windows in GHA for tier1. David - - PR: https://git.openjdk.java.net/jdk/pull/4355
Re: RFR: 8268349: Provide more detail in JEP 411 warning messages
On Mon, 7 Jun 2021 20:42:53 GMT, Weijun Wang wrote: > More loudly and precise warning messages when a security manager is either > enabled at startup or installed at runtime. Changes requested by alanb (Reviewer). src/java.base/share/classes/java/lang/System.java line 331: > 329: > 330: // Remember original System.err. setSecurityManager() warning goes > here > 331: private static PrintStream oldErrStream = null; I assume this should needs to be volatile and @Stable. I think we need a better name for it too. src/java.base/share/classes/java/lang/System.java line 336: > 334: // Remember callers of setSecurityManager() here so that warning > 335: // is only printed once for each different caller > 336: final static Map callersOfSSM = new > WeakHashMap<>(); You can't use a WeakHashMap without synchronization but a big question here is whether a single caller frame is sufficient. If I were doing this then I think I would capture the hash of a number of stack frames to create a better filter. src/java.base/share/classes/java/lang/System.java line 2219: > 2217: WARNING: java.lang.SecurityManager is > deprecated and will be removed in a future release > 2218: WARNING: -Djava.security.manager=%s > will have no effect when java.lang.SecurityManager is removed > 2219: """, smProp); Raw strings may be useful here but means the lines length are inconsistent and makes it too hard to look at side by side diffs now. - PR: https://git.openjdk.java.net/jdk/pull/4400
