Re: RFR: 8273314: Add tier4 test groups
On Fri, 3 Sep 2021 18:40:14 GMT, Aleksey Shipilev wrote: > > > <...> I have excluded `vmTestbase` and `hotspot:tier4`<...> I have also > > > excluded `applications` from `hotspot:tier4` <...> > > > > > > assuming the goal of tier4 is to catch the rest of the tests, I don't think > > we should exclude `vmTestbase`, `applications` or any other tests from > > tier4. unless you also want to create tier5 for them. > > Apart from practicality of using `tier4`, I think `vmTestbase` and > `applications` are separate test suites in their own right. `tier4` is > catching all the assorted Hotspot tests that are not part of larger suites. > Makes sense? to some extent. I agree that `applications` tests can/should be seen as a separate test suite, yet I differ on `vmTestbase` as the end goal for `vmTestbase` tests is (and always was) to become just another test within hotspot jtreg test suite, hence I don't think we should treat them any different than other jtreg tests. there is also a plan (which I need to formalize and share w/ a broader audience) to rearrange `vmTestbase` tests so they will be placed within the corresponding component subdirectories, which would bring us closer to the end goal and at the same time make it slightly harder to select all `vmTestbase` tests. -- Igor - PR: https://git.openjdk.java.net/jdk/pull/5357
Re: RFR: 8273314: Add tier4 test groups
On Fri, 3 Sep 2021 09:10:20 GMT, Aleksey Shipilev wrote:
> During the review of JDK-8272914 that added hotspot:tier{2,3} groups,
> @iignatev suggested to create tier4 groups that capture all tests not in
> tiers{1,2,3}. I have excluded `vmTestbase` and `hotspot:tier4,` because they
> take 10+ hours on my highly parallel machine. I have also excluded
> `applications` from `hotspot:tier4`, because they require test dependencies
> (e.g. jcstress).
>
> Sample run:
>
>
> ==
> Test summary
> ==
>TEST TOTAL PASS FAIL ERROR
>
>>> jtreg:test/hotspot/jtreg:tier4 426 425 1 0 <<
>>> jtreg:test/jdk:tier4 2891 2885 4 2 <<
>jtreg:test/langtools:tier40 0 0 0
>
>jtreg:test/jaxp:tier4 0 0 0 0
>
> ==
>
> real 64m13.994s
> user 1462m1.213s
> sys 39m38.032s
>
>
> There are interesting test failures on my machine, which I would address
> separately.
it looks like the results above do not include the headful tests did you filter
them out?
>> jtreg:test/jdk:tier4 2891 2885 4 2 <<
-
PR: https://git.openjdk.java.net/jdk/pull/5357
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Fri, 3 Sep 2021 10:48:01 GMT, Alan Bateman wrote: > > @AlanBateman Sure, I am interested in it. > > Great! I think there are several parts to this. The removal of the JAR index > support from the URLClassLoader implementation, the `jar i` option, the JAR > file spec, and the jar tool man page. > > It would be good to create a patch for the removal to see if there are any > issues. There will probably need to be some discussion on what to do with the > jar tool. I suspect we will need to keep the code that updates the index when > updating a JAR file that has an existing index, this means keeping JarIndex > and maybe moving it to the jdk.jartool module. We can change `jar i` to print > a warning when the tool is called to generate an index. Don't worry about the > JAR spec and man page for now. I will first create the patch to remove JAR index support from the URLClassLoader implementation, the `jar i` option. - PR: https://git.openjdk.java.net/jdk/pull/5316
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 13:22:54 GMT, Сергей Цыпанов
wrote:
> Current implementation looks like this:
>
> public byte[] getBytes(String charsetName)
> throws UnsupportedEncodingException {
> if (charsetName == null) throw new NullPointerException();
> return encode(lookupCharset(charsetName), coder(), value);
> }
>
> Null check seems to be redundant here because the same check of `charsetName`
> is done within `String.lookupCharset(String)`:
>
> private static Charset lookupCharset(String csn) throws
> UnsupportedEncodingException {
> Objects.requireNonNull(csn);
> try {
> return Charset.forName(csn);
> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
> throw new UnsupportedEncodingException(csn);
> }
> }
Looks good. Please add some `noreg` keyword to the JIRA issue.
-
Marked as reviewed by naoto (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods [v2]
On Fri, 3 Sep 2021 23:19:22 GMT, Brian Burkhalter wrote: >> This request proposes to modify `java.io.FilterInputStream` to override >> `readAllBytes()`, `readNBytes(int)`, `skipNBytes(long)`, and >> `transferTo(OutputStream)` in order to leverage any performance advantage >> that the wrapped stream might have over the `java.io.InputStream` >> implementations of these methods. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8255878: Add @implSpec where appropriate This will probably break all existing subclasses that assume that they only need to override the single `int read()` method - because overriding this single method will now no longer be sufficient. Did you conduct a survey of existing subclasses of FilterInputStream (recursively), in the JDK and elsewhere, to evaluate the impact of this change? - PR: https://git.openjdk.java.net/jdk/pull/5367
Re: Possible ClassCastException in java.util.regex.Pattern.BmpCharPredicate#union(java.util.regex.Pattern.CharPredicate...)
Yeah, not only does this static union() method seem like dead code, it seems wrong
as well. Perhaps igraves should take a look at this.
The whole area seems suspicious. There doesn't seem to be any semantic difference
between BmpCharPredicate and CharPredicate. The and() & union() combinators
preserve class: combining BmpCharPredicates gives a BmpCharPredicate, otherwise a
CharPredicate. But evaluating one versus the other doesn't seem to make a
difference. However, there are a couple instanceof checks that change logic paths in
places where they're used... urghhh
s'marks
On 8/27/21 8:37 AM, Pavel Rappo wrote:
Has that method been ever used? If nothing else its name seems strange. To me,
a union has OR semantics, not AND.
On 27 Aug 2021, at 15:37, Andrey Turbanov wrote:
Hello.
I found suspicious code in the method
"java.util.regex.Pattern.BmpCharPredicate#union(java.util.regex.Pattern.CharPredicate...)"
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/regex/Pattern.java#L5639
static CharPredicate union(CharPredicate... predicates) {
CharPredicate cp = ch -> {
for (CharPredicate p : predicates) {
if (!p.is(ch))
return false;
}
return true;
};
for (CharPredicate p : predicates) {
if (! (p instanceof BmpCharPredicate))
return cp;
}
return (BmpCharPredicate)cp;
}
Variable `cp` has type CharPredicate initially. And then it's casted
to BmpCharPredicate. This cast always fails with ClassCastException
when reached.
Can reproduced in small sample class:
public static void main(String[] args) {
CharPredicate result = BmpCharPredicate.union();
System.out.println(result);
}
interface CharPredicate {
boolean is(int ch);
}
interface BmpCharPredicate extends CharPredicate {
static CharPredicate union(CharPredicate... predicates) {
CharPredicate cp = ch -> true;
for (CharPredicate p : predicates) {
if (! (p instanceof BmpCharPredicate))
return cp;
}
return (BmpCharPredicate)cp;
}
}
Exception in thread "main" java.lang.ClassCastException: class
org.RegexpBug$BmpCharPredicate$$Lambda$14/0x000800c028f0 cannot be
cast to class org.RegexpBug$BmpCharPredicate
(org.RegexpBug$BmpCharPredicate$$Lambda$14/0x000800c028f0 and
org.RegexpBug$BmpCharPredicate are in unnamed module of loader 'app')
at org.RegexpBug$BmpCharPredicate.union(RegexpBug.java:20)
at org.RegexpBug.main(RegexpBug.java:5)
As I can see this method is never used. Perhaps it should be removed?
Andrey Turbanov
Re: RFR: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods [v2]
> This request proposes to modify `java.io.FilterInputStream` to override > `readAllBytes()`, `readNBytes(int)`, `skipNBytes(long)`, and > `transferTo(OutputStream)` in order to leverage any performance advantage > that the wrapped stream might have over the `java.io.InputStream` > implementations of these methods. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8255878: Add @implSpec where appropriate - Changes: - all: https://git.openjdk.java.net/jdk/pull/5367/files - new: https://git.openjdk.java.net/jdk/pull/5367/files/ceed73ac..3064baee Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5367=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5367=00-01 Stats: 31 lines in 1 file changed: 13 ins; 3 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/5367.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5367/head:pull/5367 PR: https://git.openjdk.java.net/jdk/pull/5367
Re: RFR: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods
On Fri, 3 Sep 2021 22:29:19 GMT, Brian Burkhalter wrote: > This request proposes to modify `java.io.FilterInputStream` to override > `readAllBytes()`, `readNBytes(int)`, `skipNBytes(long)`, and > `transferTo(OutputStream)` in order to leverage any performance advantage > that the wrapped stream might have over the `java.io.InputStream` > implementations of these methods. A quick and dirty benchmark of `readAllBytes()` and `readNBytes(int)` revealed throughput thrice that of the version without these overrides. The benchmark consisted of calling these methods on a `BufferedInputStream` with a buffer size of 1 MB wrapping a `FileInputStream` opened on a file of size 1 MB. **Before:** Benchmark Mode Cnt ScoreError Units ReadNBytes.readAllBytes thrpt5 2735.935 ± 26.461 ops/s ReadNBytes.readNBytesthrpt5 2735.949 ± 16.348 ops/s **After:** Benchmark Mode Cnt Score Error Units ReadNBytes.readAllBytes thrpt5 8245.034 ± 135.084 ops/s ReadNBytes.readNBytesthrpt5 8320.554 ± 104.707 ops/s - PR: https://git.openjdk.java.net/jdk/pull/5367
RFR: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods
This request proposes to modify `java.io.FilterInputStream` to override `readAllBytes()`, `readNBytes(int)`, `skipNBytes(long)`, and `transferTo(OutputStream)` in order to leverage any performance advantage that the wrapped stream might have over the `java.io.InputStream` implementations of these methods. - Commit messages: - 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods Changes: https://git.openjdk.java.net/jdk/pull/5367/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5367=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255878 Stats: 41 lines in 1 file changed: 40 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5367.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5367/head:pull/5367 PR: https://git.openjdk.java.net/jdk/pull/5367
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Thu, 2 Sep 2021 09:59:51 GMT, Daniel Fuchs wrote: >> The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" >> via an AppContext to support "applet logging isolation". The AppContext >> class became useless since the plugin and webstart are no longer supported >> and removed in jdk11. >> >> This is the request to delete the usage of AppContext in the LogManager and >> related tests. >> >> Tested by tier1/tier2/tier3 and jdk_desktop tests. > > test/jdk/java/util/logging/TestLoggingWithMainAppContext.java line 1: > >> 1: /* > > Humm... There's no direct reference to AppContext here. Maybe this test > should be preserved? It uses it indirectly, the next line under security manager trigger creation of the appcontext: `ImageIO.read(is); // triggers calls to system loggers & creation of main AppContext` but I can preserve/rename it for sure. - PR: https://git.openjdk.java.net/jdk/pull/5326
RFR: JDK-8273246 Amend the test java/nio/channels/DatagramChannel/ManySourcesAndTargets.java to execute in othervm mode
A number of nio DatagramChannel tests are intermittently failing on macosx-aarch64. In some instances this is a receive call blocking indefinitely waiting on data which has already been sent, and should be available immediately to the receive method call. Other test failure scenarios are problems during the test compilation phase with a SocketException being thrown and the message: "test result: Error. Agent communication error: java.net.SocketException: No buffer space available; check console log for any additional details" The ManySourcesAndTargets and other tests execute in agentvm mode. This results in certain test diagnostic Output being lost during the test failure handling capture process. To mitigate this lost diagnostics, the ManySourcesAndTargets test has been amended to execute in othervm mode. Additionally, to assist in the buffer allocation issue, the netstat command executed by the test failure_handler has an extra argument added to obtain additional details on mbuf usage. The failure handler will now execute with netstat -mm - Commit messages: - JDK-8273246 Amend the test java/nio/channels/DatagramChannel/ManySourcesAndTargets.java to execute in othervm mode Changes: https://git.openjdk.java.net/jdk/pull/5366/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5366=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273246 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5366.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5366/head:pull/5366 PR: https://git.openjdk.java.net/jdk/pull/5366
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov wrote: > The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" > via an AppContext to support "applet logging isolation". The AppContext class > became useless since the plugin and webstart are no longer supported and > removed in jdk11. > > This is the request to delete the usage of AppContext in the LogManager and > related tests. > > Tested by tier1/tier2/tier3 and jdk_desktop tests. Right. I am also a bit uncomfortable about the inconsistency. That said - if everybody agrees that this should be done I have no objection. The changes to the java.util.logging implementation correspond to my expectation if this were to be carried through. test/jdk/java/util/logging/TestLoggingWithMainAppContext.java line 1: > 1: /* Humm... There's no direct reference to AppContext here. Maybe this test should be preserved? - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 14:04:27 GMT, Roger Riggs wrote: > Redundant null checks get collapsed by HotSpot, so not a performance > improvement. Hi Roger, indeed, no improvement observed. How did you know it beforehand? I was pretty sure we are going to win a couple of ns on this wide-used method. P.S. This double-check of null seems to be a left-over from https://github.com/openjdk/jdk/pull/2102. Previously there was only one NPE-throwing check there, but after `String.encode(Charset, byte, byte[])` was introduced we have two of them. - PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 17:37:08 GMT, Sergey Bylokhov wrote:
>> Current implementation looks like this:
>>
>> public byte[] getBytes(String charsetName)
>> throws UnsupportedEncodingException {
>> if (charsetName == null) throw new NullPointerException();
>> return encode(lookupCharset(charsetName), coder(), value);
>> }
>>
>> Null check seems to be redundant here because the same check of
>> `charsetName` is done within `String.lookupCharset(String)`:
>>
>> private static Charset lookupCharset(String csn) throws
>> UnsupportedEncodingException {
>> Objects.requireNonNull(csn);
>> try {
>> return Charset.forName(csn);
>> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
>> throw new UnsupportedEncodingException(csn);
>> }
>> }
>
> In such cases when the specific exception throwing is removed from the method
> because it can be produced by some other used method, the test might be
> useful. So if the code in the method will be changed, or the usage of other
> method will be removed the exception still be thrown. Probably such test
> exists already, then just point to it here.
> @mrserb you are right, there's such test, see
> `/test/jdk/java/lang/String/Exceptions.getBytes()`, line 384.
Thank you for confirmation, looks fine.
-
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 17:37:08 GMT, Sergey Bylokhov wrote:
>> Current implementation looks like this:
>>
>> public byte[] getBytes(String charsetName)
>> throws UnsupportedEncodingException {
>> if (charsetName == null) throw new NullPointerException();
>> return encode(lookupCharset(charsetName), coder(), value);
>> }
>>
>> Null check seems to be redundant here because the same check of
>> `charsetName` is done within `String.lookupCharset(String)`:
>>
>> private static Charset lookupCharset(String csn) throws
>> UnsupportedEncodingException {
>> Objects.requireNonNull(csn);
>> try {
>> return Charset.forName(csn);
>> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
>> throw new UnsupportedEncodingException(csn);
>> }
>> }
>
> In such cases when the specific exception throwing is removed from the method
> because it can be produced by some other used method, the test might be
> useful. So if the code in the method will be changed, or the usage of other
> method will be removed the exception still be thrown. Probably such test
> exists already, then just point to it here.
@mrserb you are right, there's such test, see
`/test/jdk/java/lang/String/Exceptions.getBytes()`, line 384.
-
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v4]
On Fri, 3 Sep 2021 14:41:45 GMT, Vladimir Ivanov wrote: >> `MethodHandle.asTypeCache` keeps a strong reference to adapted >> `MethodHandle` and it can introduce a class loader leak through its >> `MethodType`. >> >> Proposed fix introduces a 2-level cache (1 element each) where 1st level can >> only contain `MethodHandle`s which are guaranteed to not introduce any >> dependencies on new class loaders compared to the original `MethodHandle`. >> 2nd level is backed by a `SoftReference` and is used as a backup when the >> result of `MethodHandle.asType()` conversion can't populate the higher level >> cache. >> >> The fix is based on [the >> work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) >> made by Peter Levart @plevart back in 2015. >> >> Testing: tier1 - tier6 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Other MethodHandle/LambdaForm caches in `java.lang.invoke` deliberately rely on `SoftReference`s. (Only `MethodType` table uses `WeakReference`, but it is there for interning purposes.) MH adaptations are quite expensive (and may involve class loading), so it's beneficial to keep the result around for an extended period of time when circumstances allow. On the other hand, `MH.asType()` case is special because it can hold user classes around which makes effective memory footprint of cached value much higher. So, `WeakReference` would enable more prompt recycling of heap memory at the expense of higher cache miss rate. Personally, I'm in favor `SoftReference` here since it allows to get most of performance benefits out of the cache while preserving the correctness w.r.t. heap exhaustion. - PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 13:22:54 GMT, Сергей Цыпанов
wrote:
> Current implementation looks like this:
>
> public byte[] getBytes(String charsetName)
> throws UnsupportedEncodingException {
> if (charsetName == null) throw new NullPointerException();
> return encode(lookupCharset(charsetName), coder(), value);
> }
>
> Null check seems to be redundant here because the same check of `charsetName`
> is done within `String.lookupCharset(String)`:
>
> private static Charset lookupCharset(String csn) throws
> UnsupportedEncodingException {
> Objects.requireNonNull(csn);
> try {
> return Charset.forName(csn);
> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
> throw new UnsupportedEncodingException(csn);
> }
> }
Marked as reviewed by iris (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8273314: Add tier4 test groups
On Fri, 3 Sep 2021 18:32:21 GMT, Igor Ignatyev wrote: > > <...> I have excluded `vmTestbase` and `hotspot:tier4`<...> I have also > > excluded `applications` from `hotspot:tier4` <...> > > assuming the goal of tier4 is to catch the rest of the tests, I don't think > we should exclude `vmTestbase`, `applications` or any other tests from tier4. > unless you also want to create tier5 for them. Apart from practicality of using `tier4`, I think `vmTestbase` and `applications` are separate test suites in their own right. `tier4` is catching all the assorted Hotspot tests that are not part of larger suites. Makes sense? - PR: https://git.openjdk.java.net/jdk/pull/5357
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Fri, 3 Sep 2021 10:48:01 GMT, Alan Bateman wrote: > There will probably need to be some discussion on what to do with the jar > tool. I suspect we will need to keep the code that updates the index when > updating a JAR file that has an existing index, this means keeping > JarIndex Depends on how draconian we're willing to be. At some point I can see the tool _removing_ the index if it's updating a jar file that has one. Suitable warning messages would need to be emitted. - PR: https://git.openjdk.java.net/jdk/pull/5316
Re: RFR: 8273314: Add tier4 test groups
On Fri, 3 Sep 2021 09:10:20 GMT, Aleksey Shipilev wrote:
> During the review of JDK-8272914 that added hotspot:tier{2,3} groups,
> @iignatev suggested to create tier4 groups that capture all tests not in
> tiers{1,2,3}. I have excluded `vmTestbase` and `hotspot:tier4,` because they
> take 10+ hours on my highly parallel machine. I have also excluded
> `applications` from `hotspot:tier4`, because they require test dependencies
> (e.g. jcstress).
>
> Sample run:
>
>
> ==
> Test summary
> ==
>TEST TOTAL PASS FAIL ERROR
>
>>> jtreg:test/hotspot/jtreg:tier4 426 425 1 0 <<
>>> jtreg:test/jdk:tier4 2891 2885 4 2 <<
>jtreg:test/langtools:tier40 0 0 0
>
>jtreg:test/jaxp:tier4 0 0 0 0
>
> ==
>
> real 64m13.994s
> user 1462m1.213s
> sys 39m38.032s
>
>
> There are interesting test failures on my machine, which I would address
> separately.
> <...> I have excluded `vmTestbase` and `hotspot:tier4`<...> I have also
> excluded `applications` from `hotspot:tier4` <...>
assuming the goal of tier4 is to catch the rest of the tests, I don't think we
should exclude `vmTestbase`, `applications` or any other tests from tier4.
unless you also want to create tier5 for them.
-- Igor
-
PR: https://git.openjdk.java.net/jdk/pull/5357
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v4]
On Fri, 3 Sep 2021 14:41:45 GMT, Vladimir Ivanov wrote: >> `MethodHandle.asTypeCache` keeps a strong reference to adapted >> `MethodHandle` and it can introduce a class loader leak through its >> `MethodType`. >> >> Proposed fix introduces a 2-level cache (1 element each) where 1st level can >> only contain `MethodHandle`s which are guaranteed to not introduce any >> dependencies on new class loaders compared to the original `MethodHandle`. >> 2nd level is backed by a `SoftReference` and is used as a backup when the >> result of `MethodHandle.asType()` conversion can't populate the higher level >> cache. >> >> The fix is based on [the >> work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) >> made by Peter Levart @plevart back in 2015. >> >> Testing: tier1 - tier6 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Did we want this to use a soft reference or a weak reference? The original bug report mentions weak references, and the typical approach for preventing caches from holding onto things unnecessarily is to use weak references. I haven't thought through the implications of using soft instead of weak references. (Sorry if I missed any discussion on this issue.) - PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Fri, 3 Sep 2021 17:19:05 GMT, Phil Race wrote: > Perhaps this isn't the change that requires the CSR but it then leaves an > inconsistent state where desktop supports AppContext still but other modules > don't ... Even java.desktop does not support it fully, since for a couple of years nobody care about appcontext when write a new code. Note that I mentioned the "threadgroup sandboxing" in the subject, which is not necessary implemented via Appcontext. The JavaBeans and JavaSound use the thread group directly, I plan to remove them as well. - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 13:22:54 GMT, Сергей Цыпанов
wrote:
> Current implementation looks like this:
>
> public byte[] getBytes(String charsetName)
> throws UnsupportedEncodingException {
> if (charsetName == null) throw new NullPointerException();
> return encode(lookupCharset(charsetName), coder(), value);
> }
>
> Null check seems to be redundant here because the same check of `charsetName`
> is done within `String.lookupCharset(String)`:
>
> private static Charset lookupCharset(String csn) throws
> UnsupportedEncodingException {
> Objects.requireNonNull(csn);
> try {
> return Charset.forName(csn);
> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
> throw new UnsupportedEncodingException(csn);
> }
> }
In such cases when the specific exception throwing is removed from the method
because it can be produced by some other used method, the test might be useful.
So if the code in the method will be changed, or the usage of other method will
be removed the exception still be thrown. Probably such test exists already,
then just point to it here.
-
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov wrote: > The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" > via an AppContext to support "applet logging isolation". The AppContext class > became useless since the plugin and webstart are no longer supported and > removed in jdk11. > > This is the request to delete the usage of AppContext in the LogManager and > related tests. > > Tested by tier1/tier2/tier3 and jdk_desktop tests. @aivanov-jdk will help make sure the closed changes are pushed at exactly the same time as this gets pushed. - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov wrote: > The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" > via an AppContext to support "applet logging isolation". The AppContext class > became useless since the plugin and webstart are no longer supported and > removed in jdk11. > > This is the request to delete the usage of AppContext in the LogManager and > related tests. > > Tested by tier1/tier2/tier3 and jdk_desktop tests. Hmm I was under the impression this was removing AppContext itself but it is just removing the backdoor needed by logging Perhaps this isn't the change that requires the CSR but it then leaves an inconsistent state where desktop supports AppContext still but other modules don't ... - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov wrote: > The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" > via an AppContext to support "applet logging isolation". The AppContext class > became useless since the plugin and webstart are no longer supported and > removed in jdk11. > > This is the request to delete the usage of AppContext in the LogManager and > related tests. > > Tested by tier1/tier2/tier3 and jdk_desktop tests. I believe we should have a CSR for this. It removes a long standing capability in the platform that was used by components such as plugin and webstart. - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging
On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov wrote: > The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" > via an AppContext to support "applet logging isolation". The AppContext class > became useless since the plugin and webstart are no longer supported and > removed in jdk11. > > This is the request to delete the usage of AppContext in the LogManager and > related tests. > > Tested by tier1/tier2/tier3 and jdk_desktop tests. This fix requires coordinated closed changes so needs an Oracle sponsor. And actually is probably better handled entirely by an Oracle engineer because pushes need to be very co-ordinated. - PR: https://git.openjdk.java.net/jdk/pull/5326
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v4]
On Fri, 3 Sep 2021 14:41:45 GMT, Vladimir Ivanov wrote: >> `MethodHandle.asTypeCache` keeps a strong reference to adapted >> `MethodHandle` and it can introduce a class loader leak through its >> `MethodType`. >> >> Proposed fix introduces a 2-level cache (1 element each) where 1st level can >> only contain `MethodHandle`s which are guaranteed to not introduce any >> dependencies on new class loaders compared to the original `MethodHandle`. >> 2nd level is backed by a `SoftReference` and is used as a backup when the >> result of `MethodHandle.asType()` conversion can't populate the higher level >> cache. >> >> The fix is based on [the >> work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) >> made by Peter Levart @plevart back in 2015. >> >> Testing: tier1 - tier6 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Marked as reviewed by plevart (Reviewer). This looks good to me now. - PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v3]
On Fri, 3 Sep 2021 12:51:13 GMT, Peter Levart wrote:
>> Vladimir Ivanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address review comments
>
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 877:
>
>> 875: }
>> 876: if (asTypeSoftCache != null) {
>> 877: atc = asTypeSoftCache.get();
>
> NPE is possible here too! asTypeSoftCache is a non-volatile field which is
> read twice. First time in the if (...) condition, 2nd time in the line that
> de-references it to call .get(). This is a data-race since concurrent thread
> may be setting this field from null to non-null. Those two reads may get
> reordered. 1st read may return non-null while 2nd may return null. This can
> be avoided if the field is read just once by introducing a local variable to
> store its value.
Fixed.
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 878:
>
>> 876: if (asTypeSoftCache != null) {
>> 877: atc = asTypeSoftCache.get();
>> 878: if (newType == atc.type) {
>
> NPE is possible here! act can be null as it is a result of SoftReference::get
Good catch! Fixed.
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 933:
>
>> 931: }
>> 932:
>> 933: /* Returns true when {@code loader} keeps {@code mt} either
>> directly or indirectly through the loader delegation chain. */
>
> Well, to be precise, loader can't keep mt alive. It would be better to say
> "keeps mt components alive" ...
Fixed.
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 948:
>
>> 946: if (isBuiltinLoader(defLoader)) {
>> 947: return true; // built-in loaders are always reachable
>> 948: }
>
> No need for special case here. isAncestorLoaderOf(defLoader, loader) already
> handles this case.
Though the check is redundant, I find the current version clearer.
-
PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v4]
> `MethodHandle.asTypeCache` keeps a strong reference to adapted `MethodHandle` > and it can introduce a class loader leak through its `MethodType`. > > Proposed fix introduces a 2-level cache (1 element each) where 1st level can > only contain `MethodHandle`s which are guaranteed to not introduce any > dependencies on new class loaders compared to the original `MethodHandle`. > 2nd level is backed by a `SoftReference` and is used as a backup when the > result of `MethodHandle.asType()` conversion can't populate the higher level > cache. > > The fix is based on [the > work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) > made by Peter Levart @plevart back in 2015. > > Testing: tier1 - tier6 Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5246/files - new: https://git.openjdk.java.net/jdk/pull/5246/files/7a87aee3..365dd454 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5246=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5246=02-03 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/5246.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5246/head:pull/5246 PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8272600: (test) Use native "sleep" in Basic.java [v4]
> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified > unexpected messages from a child Java VM > as the cause of the test failure. Attempts to control the output of the > child VM have failed, the VM is unrepentant . > > There is no functionality in the child except to wait long enough for the > test to finish and the child is destroyed. > The fix is to switch from using a Java child to using a native child; a new > executable `sleepmillis`. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Revert to using BasicSleep on Windows Added diagnostic output for a test that sometimes fails on Linux when using /bin/sleep. Addressed review comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5239/files - new: https://git.openjdk.java.net/jdk/pull/5239/files/05d009de..2a9c33fb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5239=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5239=02-03 Stats: 23 lines in 2 files changed: 11 ins; 3 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/5239.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5239/head:pull/5239 PR: https://git.openjdk.java.net/jdk/pull/5239
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v3]
On Thu, 2 Sep 2021 11:35:52 GMT, Vladimir Ivanov wrote:
>> `MethodHandle.asTypeCache` keeps a strong reference to adapted
>> `MethodHandle` and it can introduce a class loader leak through its
>> `MethodType`.
>>
>> Proposed fix introduces a 2-level cache (1 element each) where 1st level can
>> only contain `MethodHandle`s which are guaranteed to not introduce any
>> dependencies on new class loaders compared to the original `MethodHandle`.
>> 2nd level is backed by a `SoftReference` and is used as a backup when the
>> result of `MethodHandle.asType()` conversion can't populate the higher level
>> cache.
>>
>> The fix is based on [the
>> work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/)
>> made by Peter Levart @plevart back in 2015.
>>
>> Testing: tier1 - tier6
>
> Vladimir Ivanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Address review comments
src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 877:
> 875: }
> 876: if (asTypeSoftCache != null) {
> 877: atc = asTypeSoftCache.get();
NPE is possible here too! asTypeSoftCache is a non-volatile field which is read
twice. First time in the if (...) condition, 2nd time in the line that
de-references it to call .get(). This is a data-race since concurrent thread
may be setting this field from null to non-null. Those two reads may get
reordered. 1st read may return non-null while 2nd may return null. This can be
avoided if the field is read just once by introducing a local variable to store
its value.
src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 878:
> 876: if (asTypeSoftCache != null) {
> 877: atc = asTypeSoftCache.get();
> 878: if (newType == atc.type) {
NPE is possible here! act can be null as it is a result of SoftReference::get
src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 933:
> 931: }
> 932:
> 933: /* Returns true when {@code loader} keeps {@code mt} either directly
> or indirectly through the loader delegation chain. */
Well, to be precise, loader can't keep mt alive. It would be better to say
"keeps mt components alive" ...
src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 948:
> 946: if (isBuiltinLoader(defLoader)) {
> 947: return true; // built-in loaders are always reachable
> 948: }
No need for special case here. isAncestorLoaderOf(defLoader, loader) already
handles this case.
-
PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 13:22:54 GMT, Сергей Цыпанов
wrote:
> Current implementation looks like this:
>
> public byte[] getBytes(String charsetName)
> throws UnsupportedEncodingException {
> if (charsetName == null) throw new NullPointerException();
> return encode(lookupCharset(charsetName), coder(), value);
> }
>
> Null check seems to be redundant here because the same check of `charsetName`
> is done within `String.lookupCharset(String)`:
>
> private static Charset lookupCharset(String csn) throws
> UnsupportedEncodingException {
> Objects.requireNonNull(csn);
> try {
> return Charset.forName(csn);
> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
> throw new UnsupportedEncodingException(csn);
> }
> }
Redundant null checks get collapsed by HotSpot, so not a performance
improvement.
Having null checks at public entry points also shows a stack trace that is more
specific about where the null came from. So not much value in changing this.
-
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base
On Wed, 1 Sep 2021 07:37:53 GMT, Andrey Turbanov wrote: > There are few places in code where manual while loop is used with Iterator to > iterate over Collection. > Instead of manual while cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: java compiler generates similar code > when compiling enhanced-for cycle. > > Similar cleanups: > * https://bugs.openjdk.java.net/browse/JDK-8258006 > * https://bugs.openjdk.java.net/browse/JDK-8257912 The security related changes look fine to me. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5328
RFR: 8273329: Remove redundant null check from String.getBytes(String charsetName)
Current implementation looks like this:
public byte[] getBytes(String charsetName)
throws UnsupportedEncodingException {
if (charsetName == null) throw new NullPointerException();
return encode(lookupCharset(charsetName), coder(), value);
}
Null check seems to be redundant here because the same check of `charsetName`
is done within `String.lookupCharset(String)`:
private static Charset lookupCharset(String csn) throws
UnsupportedEncodingException {
Objects.requireNonNull(csn);
try {
return Charset.forName(csn);
} catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
throw new UnsupportedEncodingException(csn);
}
}
-
Commit messages:
- 8273329: Remove redundant null check from String.getBytes(String charsetName)
Changes: https://git.openjdk.java.net/jdk/pull/5361/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5361=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8273329
Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
Patch: https://git.openjdk.java.net/jdk/pull/5361.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5361/head:pull/5361
PR: https://git.openjdk.java.net/jdk/pull/5361
Re: RFR: 8272600: (test) Use native "sleep" in Basic.java [v3]
On Sat, 28 Aug 2021 02:34:48 GMT, Roger Riggs wrote: >> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified >> unexpected messages from a child Java VM >> as the cause of the test failure. Attempts to control the output of the >> child VM have failed, the VM is unrepentant . >> >> There is no functionality in the child except to wait long enough for the >> test to finish and the child is destroyed. >> The fix is to switch from using a Java child to using a native child; a new >> executable `sleepmillis`. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Revised to use native /bin/sleep program on Unix* (non-Windows). > For Windows, a native "BasicSleep" program is used. On Windows Timeout, /NOBREAK does not change the behavior, still does not support if input is redirected. - PR: https://git.openjdk.java.net/jdk/pull/5239
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v3]
On Fri, 3 Sep 2021 00:09:17 GMT, Mandy Chung wrote:
> For the change of MethodHandle::asType to a final method, this needs a CSR.
It is not allowed to extend/subclass `MethodHandle` outside `java.lang.invoke`
package.
So, the aforementioned change doesn't have any compatibility risks. Do I miss
something important?
/**
* Package-private constructor for the method handle implementation
hierarchy.
* Method handle inheritance will be contained completely within
* the {@code java.lang.invoke} package.
*/
// @param type type (permanently assigned) of the new method handle
/*non-public*/
MethodHandle(MethodType type, LambdaForm form) {
-
PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Thu, 2 Sep 2021 11:43:46 GMT, Alan Bateman wrote:
>> Using jarIndex for Hibench, there is an unexpected behavior with the
>> exception "Exception in thread "main"
>> org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for
>> scheme "hdfs"".
>>
>> After investigating it, it is related to the usage of ServiceLoader with
>> JarIndex.
>> The below stack shows the issue with JDK11:
>>
>> getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
>> getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
>> findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
>> next:341, URLClassPath$1 (jdk.internal.loader)
>> hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
>> hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
>> hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
>> next:3032, CompoundEnumeration (java.lang)
>> hasMoreElements:3041, CompoundEnumeration (java.lang)
>> nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNext:1300, ServiceLoader$2 (java.util)
>> hasNext:1385, ServiceLoader$3 (java.util)
>>
>> The below API tries to get all the resources with the same name.
>>
>> public Enumeration findResources(final String name,
>> final boolean check)
>> ```
>> After using JarIndex, URLClassPath.findResources only returns 1 URL.
>> It is the same as the description in JDK-6957241.
>>
>> The issue still exists in JDK18.
>>
>> Root cause:
>>
>> public Enumeration findResources(final String name,
>> final boolean check) {
>> return new Enumeration<>() {
>> private int index = 0;
>> private URL url = null;
>>
>> private boolean next() {
>> if (url != null) {
>> return true;
>> } else {
>> Loader loader;
>> while ((loader = getLoader(index++)) != null) {
>> url = loader.findResource(name, check);
>> if (url != null) {
>> return true;
>> }
>> }
>> return false;
>> }
>> }
>> ...
>> };
>> }
>>
>> With the JarIndex, there is only one loader which is corresponding to the
>> jar with the index due to the implementation in JarLoader.getResource(final
>> String name, boolean check, Set visited).
>>
>> Loaders corresponding to other jar packages will not appear in this while.
>> So it only returns 1 instance.
>>
>> To solve the issue, I change the implementation "private boolean next()".
>> If the loader has index, traverse the index and get all the resource from
>> the loader.
>
> @wxiang I think there is at least some support for removing the JAR indexing
> support rather than trying to fix findResources. The issue of what to do with
> the legacy JAR index mechanism came up during JDK 9 in the context of modular
> JARs and also Multi-Release JARs but it was too much to take on at the time.
> Would you be interested in working out the changes to remove it?
> @AlanBateman Sure, I am interested in it.
Great! I think there are several parts to this. The removal of the JAR index
support from the URLClassLoader implementation, the `jar i` option, the JAR
file spec, and the jar tool man page.
It would be good to create a patch for the removal to see if there are any
issues. There will probably need to be some discussion on what to do with the
jar tool. I suspect we will need to keep the code that updates the index when
updating a JAR file that has an existing index, this means keeping JarIndex and
maybe moving it to the jdk.jartool module. We can change `jar i` to print a
warning when the tool is called to generate an index. Don't worry about the JAR
spec and man page for now.
-
PR: https://git.openjdk.java.net/jdk/pull/5316
RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test
This test runs a lot of configurations, and spends a lot of time serially. This is especially pronounced when run in prospective tier4 runs (JDK-8273314). There are reports of multi-hour runs (see JDK-8271613). We can parallelize the test configurations for this test to make it hurt less. Also, timeouts need to be increased for `TestUpcall` test configs, because some of them are very slow in fastdebug mode. Sample run: $ time CONF=linux-x86_64-server-fastdebug make run-test TEST=java/foreign/TestMatrix.java | ts -s 00:00:00 Building target 'run-test' in configuration 'linux-x86_64-server-fastdebug' 00:00:03 Test selection 'java/foreign/TestMatrix.java', will run: 00:00:03 * jtreg:test/jdk/java/foreign/TestMatrix.java 00:00:03 00:00:03 Running test 'jtreg:test/jdk/java/foreign/TestMatrix.java' 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFT 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTF 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTT 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTT 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFFT 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTTF 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTF 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFT 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTF 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFF 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFF 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTT 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFF 00:00:38 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFT 00:01:50 Passed: java/foreign/TestMatrix.java#Downcall-FF 00:02:27 Passed: java/foreign/TestMatrix.java#Downcall-TF 00:03:03 Passed: java/foreign/TestMatrix.java#Downcall-FT 00:03:47 Passed: java/foreign/TestMatrix.java#Downcall-TT 00:04:17 Passed: java/foreign/TestMatrix.java#Upcall-FTFF 00:04:23 Passed: java/foreign/TestMatrix.java#Upcall-TFFF 00:05:46 Passed: java/foreign/TestMatrix.java#Upcall-TTFF 00:06:03 Passed: java/foreign/TestMatrix.java#Upcall-TFFT 00:06:44 Passed: java/foreign/TestMatrix.java#Upcall-FTFT 00:08:24 Passed: java/foreign/TestMatrix.java#Upcall-TFTF 00:08:39 Passed: java/foreign/TestMatrix.java#Upcall-TTFT 00:09:16 Passed: java/foreign/TestMatrix.java#Upcall-FTTF 00:09:19 Passed: java/foreign/TestMatrix.java#Upcall-TFTT 00:10:01 Passed: java/foreign/TestMatrix.java#Upcall-FTTT 00:10:37 Passed: java/foreign/TestMatrix.java#Upcall-TTTF 00:12:16 Passed: java/foreign/TestMatrix.java#Upcall- 00:12:17 Test results: passed: 32 00:12:21 00:12:21 == 00:12:21 Test summary 00:12:21 == 00:12:21TEST TOTAL PASS FAIL ERROR 00:12:21jtreg:test/jdk/java/foreign/TestMatrix.java 3232 0 0 00:12:21 == real12m20.538s user131m54.043s sys 0m59.944s If we don't parallelize, then those 130 minutes are spent serially. - Commit messages: - Timeouts and cleanups - Parallelize Changes: https://git.openjdk.java.net/jdk/pull/5358/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5358=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273315 Stats: 238 lines in 1 file changed: 195 ins; 14 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/5358.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5358/head:pull/5358 PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v4]
On Wed, 28 Jul 2021 08:51:38 GMT, Andrew Haley wrote: >> I don't think we want to keep two copies of the compareTo intrinsic. If >> there are no cases where the LDP version is worse than the original version >> then we should just delete the old one and replace it with this. > >> I don't think we want to keep two copies of the compareTo intrinsic. If >> there are no cases where the LDP version is worse than the original version >> then we should just delete the old one and replace it with this. > > I agree. The trouble is, what does "worse" mean? I'm looking at SDEN-1982442, > Neoverse N2 errata, 2001293, and I see that LDP has to be slowed down on > streaming workloads, which will affect this. (That's just an example: I'm > making the point that implementations differ.) > > The trouble with this patch is that it (probably) makes things better for > long strings, which are very rare. What we actually need to care about is > performance for a large number of typical-sized strings, which are names, > identifiers, passwords, and so on: about 10-30 characters. @theRealAph do you have any other questions about this patch? - PR: https://git.openjdk.java.net/jdk/pull/4722
RFR: 8273314: Add tier4 test groups
During the review of JDK-8272914 that added hotspot:tier{2,3} groups, @iignatev
suggested to create tier4 groups that capture all tests not in tiers{1,2,3}. I
have excluded `vmTestbase` and `hotspot:tier4,` because they take 10+ hours on
my highly parallel machine. I have also excluded `applications` from
`hotspot:tier4`, because they require test dependencies (e.g. jcstress).
Sample run:
==
Test summary
==
TEST TOTAL PASS FAIL ERROR
>> jtreg:test/hotspot/jtreg:tier4 426 425 1 0 <<
>> jtreg:test/jdk:tier4 2891 2885 4 2 <<
jtreg:test/langtools:tier40 0 0 0
jtreg:test/jaxp:tier4 0 0 0 0
==
real64m13.994s
user1462m1.213s
sys 39m38.032s
There are interesting test failures on my machine, which I would address
separately.
-
Commit messages:
- Add tier4 test groups
Changes: https://git.openjdk.java.net/jdk/pull/5357/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5357=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8273314
Stats: 22 lines in 4 files changed: 22 ins; 0 del; 0 mod
Patch: https://git.openjdk.java.net/jdk/pull/5357.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5357/head:pull/5357
PR: https://git.openjdk.java.net/jdk/pull/5357
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Thu, 2 Sep 2021 11:43:46 GMT, Alan Bateman wrote:
>> Using jarIndex for Hibench, there is an unexpected behavior with the
>> exception "Exception in thread "main"
>> org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for
>> scheme "hdfs"".
>>
>> After investigating it, it is related to the usage of ServiceLoader with
>> JarIndex.
>> The below stack shows the issue with JDK11:
>>
>> getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
>> getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
>> findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
>> next:341, URLClassPath$1 (jdk.internal.loader)
>> hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
>> hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
>> hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
>> next:3032, CompoundEnumeration (java.lang)
>> hasMoreElements:3041, CompoundEnumeration (java.lang)
>> nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNext:1300, ServiceLoader$2 (java.util)
>> hasNext:1385, ServiceLoader$3 (java.util)
>>
>> The below API tries to get all the resources with the same name.
>>
>> public Enumeration findResources(final String name,
>> final boolean check)
>> ```
>> After using JarIndex, URLClassPath.findResources only returns 1 URL.
>> It is the same as the description in JDK-6957241.
>>
>> The issue still exists in JDK18.
>>
>> Root cause:
>>
>> public Enumeration findResources(final String name,
>> final boolean check) {
>> return new Enumeration<>() {
>> private int index = 0;
>> private URL url = null;
>>
>> private boolean next() {
>> if (url != null) {
>> return true;
>> } else {
>> Loader loader;
>> while ((loader = getLoader(index++)) != null) {
>> url = loader.findResource(name, check);
>> if (url != null) {
>> return true;
>> }
>> }
>> return false;
>> }
>> }
>> ...
>> };
>> }
>>
>> With the JarIndex, there is only one loader which is corresponding to the
>> jar with the index due to the implementation in JarLoader.getResource(final
>> String name, boolean check, Set visited).
>>
>> Loaders corresponding to other jar packages will not appear in this while.
>> So it only returns 1 instance.
>>
>> To solve the issue, I change the implementation "private boolean next()".
>> If the loader has index, traverse the index and get all the resource from
>> the loader.
>
> @wxiang I think there is at least some support for removing the JAR indexing
> support rather than trying to fix findResources. The issue of what to do with
> the legacy JAR index mechanism came up during JDK 9 in the context of modular
> JARs and also Multi-Release JARs but it was too much to take on at the time.
> Would you be interested in working out the changes to remove it?
@AlanBateman Sure, I am interested in it.
-
PR: https://git.openjdk.java.net/jdk/pull/5316
