Re: java.io.File#toPath() on Windows doesn't understand "NUL:" (null device)?
On 17/03/21 8:51 am, Jaikiran Pai wrote: Test results are from latest Java 16 release on a Windows setup. Just gave a quick try against Java 8 (java.version=1.8.0_252) and it fails on that version too with the same error: Exception in thread "main" java.nio.file.InvalidPathException: Illegal char <:> at index 3: NUL: at sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77) at sun.nio.fs.WindowsPath.parse(WindowsPath.java:94) at sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:255) at java.io.File.toPath(File.java:2234) at FileTest.main(FileTest.java:18) -Jaikiran
java.io.File#toPath() on Windows doesn't understand "NUL:" (null device)?
Please consider this trivial code:
import java.io.*;
import java.nio.file.*;
public class FileTest {
public static void main(final String[] args) throws Exception {
System.getProperties().list(System.out);
final File f = new File("NUL:");
try (final InputStream fis = Files.newInputStream(f.toPath())) {
int numBytes = 0;
while (fis.read() != -1) {
System.out.println("Files.newInputStream - Read a
byte from " + f);
numBytes++;
}
System.out.println("Files.newInputStream - Done reading
" + numBytes + " bytes from " + f);
}
}
}
The code tries to read from NUL: on a Windows setup. This code runs into
the following exception on Windows when the java.io.File#toPath() gets
invoked:
Exception in thread "main" java.nio.file.InvalidPathException: Illegal
char <:> at index 3: NUL:
at
java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
at
java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:230)
at java.base/java.io.File.toPath(File.java:2316)
at FileTest.main(FileTest.java:18)
So it looks like java.io.File.toPath() on Windows isn't able to
recognize the null device construct?
Just to make sure this issue resides only this specific API
implementation, I switched the above code to use new FileInputStream(f)
as follows and that works as expected - reads 0 bytes and completes
successfully. So the NUL: construct is indeed understood by the other APIs.
public class FileTest {
public static void main(final String[] args) throws Exception {
System.getProperties().list(System.out);
final File f = new File("NUL:");
try (final FileInputStream fis = new FileInputStream(f)) {
int numBytes = 0;
while (fis.read() != -1) {
System.out.println("FileInputStream() - Read a byte
from " + f);
numBytes++;
}
System.out.println("FileInputStream() - Done reading "
+ numBytes + " bytes from " + f);
}
}
}
Output:
FileInputStream() - Done reading 0 bytes from NUL:
Test results are from latest Java 16 release on a Windows setup.
-Jaikiran
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM
On 16/03/2021 2:59 pm, David Holmes wrote: On 16/03/2021 11:58 am, Sergey Bylokhov wrote: On Sun, 14 Mar 2021 23:34:55 GMT, Henry Jen wrote: This patch ensure launcher won't crash JVM for the new static Methods from local/anonymous class on MacOS. As @dholmes-ora pointed out in the analysis, this is a MacOS specific bug when the launcher trying to grab class name to be displayed as the Application name on the menu. The fix is to not setting name, test shows that GUI java application shows 'bin' as the application name. It's possible for us to set the name to something more friendly, for example, "Java", but I am not sure that should be launcher's responsibility to choose such a default name. It seems to me the consumer of the JAVA_MAIN_CLASS_%d environment variable should be responsible to pick such name in case the environment variable is not set. This bug is similar to https://bugs.openjdk.java.net/browse/JDK-8076264, and the fix looks fine. Both issues involve a problem trying to use the canonical name, but I'd consider both fixes deficient when an alternative name could be used. Except I overlooked that this is an anonymous class so no simple name either. I agree with Henry's later proposal - fix the crash simply then outlaw the usecase later. Cheers, David But this isn't my code so ... David - PR: https://git.openjdk.java.net/jdk/pull/2999
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM [v3]
> This patch ensure launcher won't crash JVM for the new static Methods from > local/anonymous class on MacOS. > > As @dholmes-ora pointed out in the analysis, this is a MacOS specific bug > when the launcher trying to grab class name to be displayed as the > Application name on the menu. > > The fix is to not setting name, test shows that GUI java application shows > 'bin' as the application name. It's possible for us to set the name to > something more friendly, for example, "Java", but I am not sure that should > be launcher's responsibility to choose such a default name. It seems to me > the consumer of the JAVA_MAIN_CLASS_%d environment variable should be > responsible to pick such name in case the environment variable is not set. Henry Jen has updated the pull request incrementally with one additional commit since the last revision: Add copyright and another test case - Changes: - all: https://git.openjdk.java.net/jdk/pull/2999/files - new: https://git.openjdk.java.net/jdk/pull/2999/files/58f197f4..f68b0919 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2999=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2999=01-02 Stats: 30 lines in 2 files changed: 29 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2999.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2999/head:pull/2999 PR: https://git.openjdk.java.net/jdk/pull/2999
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info [v2]
On Tue, 16 Mar 2021 21:39:26 GMT, Joe Wang wrote: >> Consolidate and move javadoc for the lookup mechanism to the module summary. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typos: s/XMLEventFactory/XMLInputFactory > s/XMLEventFactory/XMLOutputFactory Nice consolidation. I've added myself as a reviewer of the CSR too. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3020
Re: RFR: JDK-8263154: [macos] DMG builds have finder errors
On Sat, 13 Mar 2021 14:39:40 GMT, Andy Herrick wrote: > JDK-8263154: [macos] DMG builds have finder errors Marked as reviewed by almatvee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2987
RFR: 8254979: Class.getSimpleName() returns non-empty for lambda and method
java.lang.Class has a number of methods to return some kind of textual token associated with the class, including a type name, canonical name, simple name, and separately the results of toString(). Bug 8254979 notes that getSimpleName mentions returning the name in source code, but operationally returns a non-null result for synthetic classes, ones without benefit of source code representation. Since it is useful to return a non-null result in such cases, I've updated the spec to mention this case. The names of synthetic classes are not covered by the JLS; using a "$" in such names is customary, but not strictly required. The synthetic classes used to implement lambdas and method references as cited in the bug report include "$"'s. Looking over the other "getFooName" methods, I don't think they need to be updated to handle this case. - Commit messages: - 8254979: Class.getSimpleName() returns non-empty for lambda and method Changes: https://git.openjdk.java.net/jdk/pull/3038/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3038=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8254979 Stats: 10 lines in 1 file changed: 3 ins; 1 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3038.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3038/head:pull/3038 PR: https://git.openjdk.java.net/jdk/pull/3038
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info [v2]
On Tue, 16 Mar 2021 16:59:47 GMT, Naoto Sato wrote:
>> Joe Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix typos: s/XMLEventFactory/XMLInputFactory
>> s/XMLEventFactory/XMLOutputFactory
>
> src/java.xml/share/classes/javax/xml/stream/XMLInputFactory.java line 172:
>
>> 170:* Creates a new instance of the factory. This method uses the
>> 171:* JAXP Lookup
>> Mechanism
>> 172:* to determine the {@code XMLEventFactory} implementation class to
>> load.
>
> Should it be `XMLInputFactory`?
Fixed. Updated CSR as well.
> src/java.xml/share/classes/javax/xml/stream/XMLOutputFactory.java line 149:
>
>> 147:* Creates a new instance of the factory. This method uses the
>> 148:* JAXP Lookup
>> Mechanism
>> 149:* to determine the {@code XMLEventFactory} implementation class to
>> load.
>
> And this one as `XMLOutputFactory`?
Fixed. Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/3020
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info [v2]
On Tue, 16 Mar 2021 21:39:26 GMT, Joe Wang wrote: >> Consolidate and move javadoc for the lookup mechanism to the module summary. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typos: s/XMLEventFactory/XMLInputFactory > s/XMLEventFactory/XMLOutputFactory Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3020
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info [v2]
On Tue, 16 Mar 2021 21:36:40 GMT, Joe Wang wrote: >> Consolidate and move javadoc for the lookup mechanism to the module summary. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typos: s/XMLEventFactory/XMLInputFactory > s/XMLEventFactory/XMLOutputFactory Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3020
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info [v2]
> Consolidate and move javadoc for the lookup mechanism to the module summary. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: Fix typos: s/XMLEventFactory/XMLInputFactory s/XMLEventFactory/XMLOutputFactory - Changes: - all: https://git.openjdk.java.net/jdk/pull/3020/files - new: https://git.openjdk.java.net/jdk/pull/3020/files/9884e8ce..5554c8df Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3020=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3020=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3020.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3020/head:pull/3020 PR: https://git.openjdk.java.net/jdk/pull/3020
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators
On Mon, 15 Mar 2021 20:45:30 GMT, Paul Sandoz wrote:
>> I still don't like the fact that the factory uses reflection but i don't see
>> how to do better
>
> This is now looking very nicely structured.
>
> The only thing i am unsure are the details around `RandomGenerator` being a
> service provider interface. The documentation mentions this at various points
> (mostly as implementation notes), but it's not really called out on
> `RandomGenerator`.
>
> Trying out the patch, I can implement `RandomGenerator` and register it as a
> service:
>
> public class AlwaysZero implements RandomGenerator {
> @Override
> public long nextLong() {
> return 0;
> }
> }
> ...
> RandomGenerator alwaysZero = RandomGenerator.of("AlwaysZero");
>
>
> Is that intended? (esp. since the annotation `RandomGeneratorProperties` is
> not public). If i rename the above to `L32X64MixRandom` an
> `ExceptionInInitializerError` is produced due to duplicate keys.
>
> I suspect you want to filter out the service providers to those that only
> declare `RandomGeneratorProperties`, thereby restricting to providers only
> supplied by the platform.
Has anyone here benchmarked this? I've run BumbleBench benchmarks (one of the
AdoptOpenJDK team's tools, [available
here](https://github.com/adoptium/bumblebench)) and I'm skeptical of the
original claims in [JEP 356](https://openjdk.java.net/jeps/356), namely:
> ...a new class of splittable PRNG algorithms (LXM) has also been discovered
> that are almost as fast [as SplittableRandom]...
On my machine, L64X128MixRandom's algorithm is 53% slower than
SplittableRandom, a halving in performance that I think would be inaccurate to
describe as "almost as fast." I've benchmarked on Java 8 HotSpot (Windows 10,
x64) and Java 15 OpenJ9 (same machine). On HotSpot, which I think is the main
concern, I go from 1021770880 (over 1 billion) random longs per second with
SplittableRandom to 479769280 (over 479 million) with my (I believe faithful)
implementation of L64X128MixRandom. That is where I observed the 53% reduction.
While SplittableRandom specifically seems to perform relatively badly on
OpenJ9, with 893283072 longs per second (893 million), other similar random
number generators do extremely well on OpenJ9;
[LaserRandom](https://github.com/tommyettinger/jdkgdxds/blob/master/src/main/java/com/github/tommyettinger/ds/support/LaserRandom.java)
generates 4232752128 random longs per second (4.2 billion) where
L64X128MixRandom gets 840015872 (840 million). My benchmark repo is a
mess, but if anyone wants to see and verify, [here it
is](https://github.com/tommyettinger/assorted-benchmarks/tree/master/src/main/java/net/adoptopenjdk/bumblebench/examples).
JMH benchmarks might provide different or just additional useful information;
I've only run BumbleBench.
One could make the argument that getting a random long from a PRNG takes so
little time in the first place that it is unlikely to be a bottleneck, and by
that logic, LXM is "almost as fast." However, if random generation is not being
called often enough for its speed to matter, you are exceedingly unlikely to
need so many (over 9 quintillion) parallel streams or such a long period per
stream ((2 to the 192) minus (2 to the 64)). LXM is also 1-dimensionally
equidistributed, while the foundation it is built on should allow 4-dimensional
equidistribution (with the caveat of any LFSR that an all-zero state is
impossible), with the same memory use per generator (4 longs), a longer period,
and comparable quality using `xoshiro256**`, or possibly `xoshiro256++`, giving
up streams but permitting twice as many leaps as LXM has streams if you
maintain the same period as L64X128MixRandom.
If I were implementing a PRNG to operate in a future official version of the
JVM, I would definitely look into ways to use AES-NI, and I think the
interfaces provided here should be valuable for any similar addition, even if I
disagree with the provided implementations of those interfaces. Thank you for
your time.
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8262807: Note assumptions of core reflection modeling and parameter handling [v2]
> Please review the javadoc change below, written in response to recent > discussion on core-libs. > > The bulk of the change is to add a discussion to java.lang.reflect's > package-info file about a language vs JVM model for core reflection. That > discussion is then linked to from several relevant locations core reflection. > A discussion of generic parameter handling is also added along with various > small cleanups. > > I'll update copyright, etc. after agreement on the text is reached. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3036/files - new: https://git.openjdk.java.net/jdk/pull/3036/files/74b2bd59..3f102171 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3036=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3036=00-01 Stats: 19 lines in 3 files changed: 0 ins; 9 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/3036.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3036/head:pull/3036 PR: https://git.openjdk.java.net/jdk/pull/3036
Re: RFR: 8262807: Note assumptions of core reflection modeling and parameter handling
On Tue, 16 Mar 2021 17:22:13 GMT, Joe Darcy wrote:
> Please review the javadoc change below, written in response to recent
> discussion on core-libs.
>
> The bulk of the change is to add a discussion to java.lang.reflect's
> package-info file about a language vs JVM model for core reflection. That
> discussion is then linked to from several relevant locations core reflection.
> A discussion of generic parameter handling is also added along with various
> small cleanups.
>
> I'll update copyright, etc. after agreement on the text is reached.
src/java.base/share/classes/java/lang/reflect/package-info.java line 79:
> 77: * differ from the modifiers on the originating element in the source
> 78: * language, including {@link Modifier#FINAL final} on a {@linkplain
> 79: * Parameter#getModifiers() parameter} and and {@code protected},
and and -> "and"
src/java.base/share/classes/java/lang/reflect/package-info.java line 85:
> 83: * Besides differences in structural representation between the
> 84: * source language and the JVM representation, core reflection also
> 85: * exposed runtime specific information. For example, the {@linkplain
exposed -> exposes
src/java.base/share/classes/java/lang/reflect/Executable.java line 262:
> 260: * Returns an array of {@code Type} objects that represent the
> 261: * formal parameter types, in declaration order, of the executable
> 262: * represented by this object. Returns an array of length 0 if the
Missing subject of the sentence? "This method returns"...
Or "An array of length 0 is returned"...
-
PR: https://git.openjdk.java.net/jdk/pull/3036
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM [v2]
On Tue, 16 Mar 2021 17:39:34 GMT, Henry Jen wrote: >> test/jdk/tools/launcher/8261785/CrashTheJVM.java line 1: >> >>> 1: import java.io.IOException; >> >> Copyright? > > This file is mostly based on the bug report as I just adjust static keyword > to make sure we cover different cases, thus I am not sure whether to add > copyright or not. you need to clarify this, if we cannot add copyright here, we should not use this file. - PR: https://git.openjdk.java.net/jdk/pull/2999
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v5]
On Tue, 16 Mar 2021 17:24:56 GMT, Mandy Chung wrote: >> Hi Rémi, >> There seems to be a deeper issue here - Patrick pointed that out to me - the >> specification of equals above speaks of comparing class names where the >> actual implementation compares classes. Maybe the specification should be >> updated - but that would be better done in a separate issue with CSR etc... >> Maybe we can do the optimization you suggest at the same time? > > `declaringClass` is a string representing the class name (not the `Class` > object). The variable name indeed causes confusion. Doh. My mistake. Was thinking about `StackFrame`. Thanks Mandy! - PR: https://git.openjdk.java.net/jdk/pull/2913
Integrated: 8263536: Add @build tags to jpackage tests
On Fri, 12 Mar 2021 17:52:30 GMT, Alexey Semenyuk wrote: > 8263536: Add @build tags to jpackage tests This pull request has now been integrated. Changeset: 422eba81 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/422eba81 Stats: 15 lines in 14 files changed: 13 ins; 2 del; 0 mod 8263536: Add @build tags to jpackage tests Reviewed-by: almatvee, iklam, herrick - PR: https://git.openjdk.java.net/jdk/pull/2975
Re: RFR: 8263536: Add @build tags to jpackage tests [v3]
On Tue, 16 Mar 2021 17:50:26 GMT, Alexey Semenyuk wrote: >> 8263536: Add @build tags to jpackage tests > > Alexey Semenyuk has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. could update the copyrights at the same time, but OK. - Marked as reviewed by herrick (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2975
Re: RFR: 8263536: Add @build tags to jpackage tests [v3]
On Tue, 16 Mar 2021 17:50:26 GMT, Alexey Semenyuk wrote: >> 8263536: Add @build tags to jpackage tests > > Alexey Semenyuk has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Marked as reviewed by iklam (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2975
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM [v2]
> This patch ensure launcher won't crash JVM for the new static Methods from > local/anonymous class on MacOS. > > As @dholmes-ora pointed out in the analysis, this is a MacOS specific bug > when the launcher trying to grab class name to be displayed as the > Application name on the menu. > > The fix is to not setting name, test shows that GUI java application shows > 'bin' as the application name. It's possible for us to set the name to > something more friendly, for example, "Java", but I am not sure that should > be launcher's responsibility to choose such a default name. It seems to me > the consumer of the JAVA_MAIN_CLASS_%d environment variable should be > responsible to pick such name in case the environment variable is not set. Henry Jen has updated the pull request incrementally with one additional commit since the last revision: Adjust width used for Copyright - Changes: - all: https://git.openjdk.java.net/jdk/pull/2999/files - new: https://git.openjdk.java.net/jdk/pull/2999/files/0fdea41c..58f197f4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2999=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2999=00-01 Stats: 15 lines in 1 file changed: 3 ins; 0 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/2999.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2999/head:pull/2999 PR: https://git.openjdk.java.net/jdk/pull/2999
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM
On Tue, 16 Mar 2021 15:33:37 GMT, Alan Bateman wrote: > Using an anonymous class for the main class looks strange and hard to believe > anyone is relying on this. I wonder if we should do more checking > LauncherHelper.validateMainClass to reject cases like this. I raised that same question, and people tends to agree launcher could reject anonymous/local classes. But pointed out that should require a CSR review. Therefore I chose to fix crash first, and we can file another ticket to address main class requirements. > This is not the last attempt to set the name, the JAVA_MAIN_CLASS_ variable > is used in the middle of the name selection, there are some others. And the > "bin" is selected by some of the next step, I agree it is not a friendly name > that could be improved. I tried to do a quick search on JAVA_MAIN_CLASS_%pid variable, didn't find other code to set this. I had a version that would set the variable to "Java", I can extend that to cover exception case as well. - PR: https://git.openjdk.java.net/jdk/pull/2999
Re: RFR: 8263536: Add @build tags to jpackage tests [v3]
> 8263536: Add @build tags to jpackage tests Alexey Semenyuk has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8263536: Add @build tags to jpackage tests - Changes: - all: https://git.openjdk.java.net/jdk/pull/2975/files - new: https://git.openjdk.java.net/jdk/pull/2975/files/34960c62..537d407a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2975=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2975=01-02 Stats: 26 lines in 13 files changed: 13 ins; 13 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2975.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2975/head:pull/2975 PR: https://git.openjdk.java.net/jdk/pull/2975
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM
On Tue, 16 Mar 2021 01:59:33 GMT, Sergey Bylokhov wrote: >> This patch ensure launcher won't crash JVM for the new static Methods from >> local/anonymous class on MacOS. >> >> As @dholmes-ora pointed out in the analysis, this is a MacOS specific bug >> when the launcher trying to grab class name to be displayed as the >> Application name on the menu. >> >> The fix is to not setting name, test shows that GUI java application shows >> 'bin' as the application name. It's possible for us to set the name to >> something more friendly, for example, "Java", but I am not sure that should >> be launcher's responsibility to choose such a default name. It seems to me >> the consumer of the JAVA_MAIN_CLASS_%d environment variable should be >> responsible to pick such name in case the environment variable is not set. > > test/jdk/tools/launcher/8261785/CrashTheJVM.java line 1: > >> 1: import java.io.IOException; > > Copyright? This file is mostly based on the bug report as I just adjust static keyword to make sure we cover different cases, thus I am not sure whether to add copyright or not. - PR: https://git.openjdk.java.net/jdk/pull/2999
RFR: 8262807: Note assumptions of core reflection modeling and parameter handling
Please review the javadoc change below, written in response to recent discussion on core-libs. The bulk of the change is to add a discussion to java.lang.reflect's package-info file about a language vs JVM model for core reflection. That discussion is then linked to from several relevant locations core reflection. A discussion of generic parameter handling is also added along with various small cleanups. I'll update copyright, etc. after agreement on the text is reached. - Commit messages: - Appease jcheck. - 8262807: Note assumptions of core reflection modeling and parameter handling Changes: https://git.openjdk.java.net/jdk/pull/3036/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3036=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262807 Stats: 90 lines in 4 files changed: 69 ins; 6 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/3036.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3036/head:pull/3036 PR: https://git.openjdk.java.net/jdk/pull/3036
Integrated: JDK-8248904: Add support to jpackage for the Mac App Store
On Wed, 24 Feb 2021 21:59:22 GMT, Andy Herrick wrote: > Implementation of Mac App Support including three new mac specific CLI > options. This pull request has now been integrated. Changeset: 11c8c78c Author:Andy Herrick URL: https://git.openjdk.java.net/jdk/commit/11c8c78c Stats: 310 lines in 23 files changed: 205 ins; 41 del; 64 mod 8248904: Add support to jpackage for the Mac App Store Reviewed-by: asemenyuk, almatvee, kizune, kcr - PR: https://git.openjdk.java.net/jdk/pull/2716
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v5]
On Tue, 16 Mar 2021 11:13:50 GMT, Daniel Fuchs wrote: >> src/java.base/share/classes/java/lang/StackTraceElement.java line 413: >> >>> 411: && Objects.equals(moduleName, e.moduleName) >>> 412: && Objects.equals(moduleVersion, e.moduleVersion) >>> 413: && e.declaringClass.equals(declaringClass) >> >> testing the declaring class and the line number should be done first given >> they are primitive values, it will be a little more efficient if two >> StackTraceElement are not equals and one is using non interned String. >> return (obj instanceof StackTraceElement e) >> && e.lineNumber == lineNumber >> && e.declaringClass == declaringClass >> && ... > > Hi Rémi, > There seems to be a deeper issue here - Patrick pointed that out to me - the > specification of equals above speaks of comparing class names where the > actual implementation compares classes. Maybe the specification should be > updated - but that would be better done in a separate issue with CSR etc... > Maybe we can do the optimization you suggest at the same time? `declaringClass` is a string representing the class name (not the `Class` object). The variable name indeed causes confusion. - PR: https://git.openjdk.java.net/jdk/pull/2913
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info
On Tue, 16 Mar 2021 00:52:24 GMT, Joe Wang wrote:
> Consolidate and move javadoc for the lookup mechanism to the module summary.
src/java.xml/share/classes/javax/xml/stream/XMLInputFactory.java line 172:
> 170:* Creates a new instance of the factory. This method uses the
> 171:* JAXP Lookup
> Mechanism
> 172:* to determine the {@code XMLEventFactory} implementation class to
> load.
Should it be `XMLInputFactory`?
src/java.xml/share/classes/javax/xml/stream/XMLOutputFactory.java line 149:
> 147:* Creates a new instance of the factory. This method uses the
> 148:* JAXP Lookup
> Mechanism
> 149:* to determine the {@code XMLEventFactory} implementation class to
> load.
And this one as `XMLOutputFactory`?
-
PR: https://git.openjdk.java.net/jdk/pull/3020
Re: RFR: 8263536: Add @compile tags to jpackage tests [v2]
On Tue, 16 Mar 2021 16:18:48 GMT, Alexey Semenyuk wrote: > > @compile will always run javac every time you run the test > > Didn't know that. Thank you for explanation. Would this be OK to leave > > `@compile` tags? Since you're making changes, I think we should do it correctly. We should use `@build` tags. - PR: https://git.openjdk.java.net/jdk/pull/2975
Re: RFR: 8263536: Add @compile tags to jpackage tests [v2]
On Mon, 15 Mar 2021 20:02:02 GMT, Ioi Lam wrote: > @compile will always run javac every time you run the test Didn't know that. Thank you for explanation. Would this be OK to leave `@compile` tags? - PR: https://git.openjdk.java.net/jdk/pull/2975
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM
On Tue, 16 Mar 2021 07:43:54 GMT, Sergey Bylokhov wrote: >> This bug is similar to https://bugs.openjdk.java.net/browse/JDK-8076264, and >> the fix looks fine. > >> Maybe the AWT folk should decide what name should be displayed in this >> case. The canonical name was chosen when all main classes had to have a >> canonical name. So perhaps a simple name will suffice in the case where >> there is no canonical name? > > This is not the last attempt to set the name, the JAVA_MAIN_CLASS_ variable > is used in the middle of the name selection, there are some others. And the > "bin" is selected by some of the next step, I agree it is not a friendly name > that could be improved. Using an anonymous class for the main class looks strange and hard to believe anyone is relying on this. I wonder if we should do more checking LauncherHelper.validateMainClass to reject cases like this. - PR: https://git.openjdk.java.net/jdk/pull/2999
Integrated: 8263450: Simplify LambdaForm.useCount
On Thu, 11 Mar 2021 14:50:46 GMT, Claes Redestad wrote: > Simplify useCount and avoid calling lastUseIndex for a small startup win. This pull request has now been integrated. Changeset: e33bfb39 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/e33bfb39 Stats: 16 lines in 1 file changed: 4 ins; 6 del; 6 mod 8263450: Simplify LambdaForm.useCount Reviewed-by: rriggs - PR: https://git.openjdk.java.net/jdk/pull/2940
Re: RFR: 8263450: Simplify LambdaForm.useCount
On Mon, 15 Mar 2021 15:29:47 GMT, Roger Riggs wrote: >> Simplify useCount and avoid calling lastUseIndex for a small startup win. > > Marked as reviewed by rriggs (Reviewer). Thanks, Roger! - PR: https://git.openjdk.java.net/jdk/pull/2940
Re: RFR: 8263108: Class initialization deadlock in java.lang.constant [v4]
On Tue, 16 Mar 2021 12:19:11 GMT, Peter Levart wrote: > Perhaps you could just add a warning to the DynamicConstantDesc.ofCanonical() > method javadoc/comment about what NOT to do in order to not fall into the > deadlock trap again... (like: don't call this method from static initializer > of ConstantDescs or such). I think that's a good idea. I went ahead and added a regular comment at the top of that method to warn about this potential deadlock. I decided not to use a javadoc comment since IMO, this is too much of an internal implementation detail to end up being part of the javadoc. I ran `make docs-jdk` after this change just to sure this comment doesn't cause any javadoc generation failures. The build went fine and the generated javadoc isn't impacted. - PR: https://git.openjdk.java.net/jdk/pull/2893
Re: RFR: 8263108: Class initialization deadlock in java.lang.constant [v5]
> Can I please get a review for this proposed patch for the issue reported in > https://bugs.openjdk.java.net/browse/JDK-8263108? > > As noted in that issue, the `java.lang.constant.DynamicConstantDesc` and > `java.lang.constant.ConstantDescs` can end up in a classloading deadlock due > to the nature of the code involved in their static blocks. A thread dump of > one such deadlock (reproduced using the code attached to that issue) is as > follows: > > "Thread A" #14 prio=5 os_prio=31 cpu=101.45ms elapsed=8.30s > tid=0x7ff88e01ca00 nid=0x6003 in Object.wait() [0x7a4c6000] >java.lang.Thread.State: RUNNABLE > at > java.lang.constant.DynamicConstantDesc.(java.base@16-ea/DynamicConstantDesc.java:67) > - waiting on the Class initialization monitor for > java.lang.constant.ConstantDescs > at Deadlock.threadA(Deadlock.java:14) > at Deadlock$$Lambda$1/0x000800c00a08.run(Unknown Source) > at java.lang.Thread.run(java.base@16-ea/Thread.java:831) > > "Thread B" #15 prio=5 os_prio=31 cpu=103.15ms elapsed=8.30s > tid=0x7ff88b936a00 nid=0x9b03 in Object.wait() [0x7a5c9000] >java.lang.Thread.State: RUNNABLE > at > java.lang.constant.ClassDesc.ofDescriptor(java.base@16-ea/ClassDesc.java:145) > - waiting on the Class initialization monitor for > java.lang.constant.DynamicConstantDesc > at > java.lang.constant.ConstantDescs.(java.base@16-ea/ConstantDescs.java:239) > at Deadlock.threadB(Deadlock.java:24) > at Deadlock$$Lambda$2/0x000800c00c28.run(Unknown Source) > at java.lang.Thread.run(java.base@16-ea/Thread.java:831) > > The commit in this PR resolves that deadlock by moving the `canonicalMap` > initialization in `DynamicConstantDesc`, from the static block, to a lazily > initialized map, into the `tryCanonicalize` (private) method of the same > class. That's the only method which uses this map. > > The implementation in `tryCanonicalize` carefully ensures that the deadlock > isn't shifted from the static block to this method, by making sure that the > `synchronization` on `DynamicConstantDesc` in this method happens only when > it's time to write the state to the shared `canonicalMap`. This has an > implication that the method local variable `canonDescs` can get initialized > by multiple threads unnecessarily but from what I can see, that's the only > way we can avoid this deadlock. This penalty of multiple threads initializing > and then throwing away that map isn't too huge because that will happen only > once when the `canonicalMap` is being initialized for the first time. > > An alternate approach that I thought of was to completely get rid of this > shared cache `canonicalMap` and instead just use method level map (that gets > initialized each time) in the `tryCanonicalize` method (thus requiring no > locks/synchronization). I ran a JMH benchmark with the current change > proposed in this PR and with the alternate approach of using the method level > map. The performance number from that run showed that the approach of using > the method level map has relatively big impact on performance (even when > there's a cache hit in that method). So I decided to not pursue that > approach. I'll include the benchmark code and the numbers in a comment in > this PR, for reference. > > The commit in this PR also includes a jtreg testcase which (consistently) > reproduces the deadlock without the fix and passes when this fix is applied. > Extra manual testing has been done to verify that the classes of interest > (noted in that testcase) are indeed getting loaded in those concurrent > threads and not in the main thread. For future maintainers, there's a > implementation note added on that testcase explaining why it cannot be moved > into testng test. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: Add a comment to instruct future maintainers of the code to avoid calling DynamicConstantDesc.ofCanonical() from static initialization of java.lang.constant.ConstantDescs - Changes: - all: https://git.openjdk.java.net/jdk/pull/2893/files - new: https://git.openjdk.java.net/jdk/pull/2893/files/3b8e4a5d..ed8a3045 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2893=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2893=03-04 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2893.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2893/head:pull/2893 PR: https://git.openjdk.java.net/jdk/pull/2893
Re: RFR: JDK-8263154: [macos] DMG builds have finder errors
On Mon, 15 Mar 2021 21:39:15 GMT, Alexander Matveev
wrote:
>
>
> Can you explain what problem is and how it is fixed? Does it fails when
> "install-dir" is specified? "install-dir" is not relevant for DMG image and
> we should ignore it.
1.) The problem is, at least on Catalina, when in AppleScript you tell Finder
to:
'make new alias file at POSIX file to POSIX file
with properties {name:""}'
if and are the same string, it fails
result is you don't see the folder icon to drag payload into.
This is where "install dir" does mater on DMG. It is the folder the
AppleScript will show in finder to suggest user to drag payload to. By default
this is "/Applications" but behavior is exactly the same if I set it to, for
example, "~/LocalApplications".
The fix only removes the leading slash from the label, so the folder will be
labeled either "Applications" or (in my example)
"Users/aherrick/LocalApplications".
-
PR: https://git.openjdk.java.net/jdk/pull/2987
Re: RFR: 8263108: Class initialization deadlock in java.lang.constant [v4]
On Tue, 16 Mar 2021 07:29:34 GMT, Vyom Tewari wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use Chris' suggested solution of avoiding deadlock > > Looks good to me. Holder pattern is easier to understand, I agree. Perhaps you could just add a warning to the DynamicConstantDesc.ofCanonical() method javadoc/comment about what *NOT* to do in order to not fall into the deadlock trap again... (like: don't call this method from static initializer of ConstantDescs or such). Otherwise it looks good to me to. - PR: https://git.openjdk.java.net/jdk/pull/2893
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v5]
On Mon, 15 Mar 2021 09:35:27 GMT, Rémi Forax wrote: >> Patrick Concannon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8263358: Refactored missed equals method > > src/java.base/share/classes/java/lang/StackTraceElement.java line 413: > >> 411: && Objects.equals(moduleName, e.moduleName) >> 412: && Objects.equals(moduleVersion, e.moduleVersion) >> 413: && e.declaringClass.equals(declaringClass) > > testing the declaring class and the line number should be done first given > they are primitive values, it will be a little more efficient if two > StackTraceElement are not equals and one is using non interned String. > return (obj instanceof StackTraceElement e) > && e.lineNumber == lineNumber > && e.declaringClass == declaringClass > && ... Hi Rémi, There seems to be a deeper issue here - Patrick pointed that out to me - the specification of equals above speaks of comparing class names where the actual implementation compares classes. Maybe the specification should be updated - but that would be better done in a separate issue with CSR etc... Maybe we can do the optimization you suggest at the same time? - PR: https://git.openjdk.java.net/jdk/pull/2913
Re: RFR: 8261673: Move javadoc for the lookup mechanism to module-info
On Tue, 16 Mar 2021 00:52:24 GMT, Joe Wang wrote: > Consolidate and move javadoc for the lookup mechanism to the module summary. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3020
Integrated: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev wrote:
> SonarCloud rightfully says:
> The length of "values" is always ">=0", so update this test to either "==0"
> or ">0".
>
> // make sure at least one value was returned
> if(values.length < 0) { // <--- here
> throw new InvalidAttributeValueException("no values for " +
> "attribute "" +
> tagName + """);
> }
>
> There is a subsequent access to values[0], which means the failure would
> throw `AIOOB`, not `InvalidAttributeValueException`.
>
> Additional testing:
> - [x] Linux x86_64 fastdebug, `com/sun/jndi`
This pull request has now been integrated.
Changeset: 83a9a029
Author:Aleksey Shipilev
URL: https://git.openjdk.java.net/jdk/commit/83a9a029
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
8263509: LdapSchemaParser.readNextTag checks array length incorrectly
Reviewed-by: stuefe, aefimov
-
PR: https://git.openjdk.java.net/jdk/pull/2968
Re: RFR: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly
On Tue, 16 Mar 2021 08:56:29 GMT, Aleksey Shipilev wrote: >> Seems fine. Lets hope no caller relies on this throwing AIOOBE. >> >> ..Thomas > >> This looks right but I'm surprised it isn't caught by tests (@AlekseiEfimov >> - do you have suggests for tests that would be useful to add here?) > > Can we go without adding tests here? This seems quite trivial. Hi @shipilev, We do not have tests for LDAP schema parser, and creating one wouldn't be trivial: environment with real LDAP server will be needed for, at least, to collect LDAP message dumps with required schema value. Therefore, I think it is reasonable to continue without a test here. -Aleksei - PR: https://git.openjdk.java.net/jdk/pull/2968
Re: RFR: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly
On Tue, 16 Mar 2021 10:52:25 GMT, Aleksei Efimov wrote:
>> SonarCloud rightfully says:
>> The length of "values" is always ">=0", so update this test to either
>> "==0" or ">0".
>>
>> // make sure at least one value was returned
>> if(values.length < 0) { // <--- here
>> throw new InvalidAttributeValueException("no values for " +
>> "attribute "" +
>> tagName + """);
>> }
>>
>> There is a subsequent access to values[0], which means the failure would
>> throw `AIOOB`, not `InvalidAttributeValueException`.
>>
>> Additional testing:
>> - [x] Linux x86_64 fastdebug, `com/sun/jndi`
>
> Marked as reviewed by aefimov (Committer).
Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/2968
Re: RFR: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev wrote:
> SonarCloud rightfully says:
> The length of "values" is always ">=0", so update this test to either "==0"
> or ">0".
>
> // make sure at least one value was returned
> if(values.length < 0) { // <--- here
> throw new InvalidAttributeValueException("no values for " +
> "attribute "" +
> tagName + """);
> }
>
> There is a subsequent access to values[0], which means the failure would
> throw `AIOOB`, not `InvalidAttributeValueException`.
>
> Additional testing:
> - [x] Linux x86_64 fastdebug, `com/sun/jndi`
Marked as reviewed by aefimov (Committer).
-
PR: https://git.openjdk.java.net/jdk/pull/2968
Integrated: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy
On Sun, 20 Dec 2020 17:05:21 GMT, Andrey Turbanov wrote: > 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy This pull request has now been integrated. Changeset: 68deb24b Author:Andrey Turbanov Committer: Julia Boes URL: https://git.openjdk.java.net/jdk/commit/68deb24b Stats: 105 lines in 7 files changed: 3 ins; 78 del; 24 mod 8080272: Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy Reviewed-by: mcimadamore, alanb - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly
On Fri, 12 Mar 2021 15:28:03 GMT, Thomas Stuefe wrote:
>> SonarCloud rightfully says:
>> The length of "values" is always ">=0", so update this test to either
>> "==0" or ">0".
>>
>> // make sure at least one value was returned
>> if(values.length < 0) { // <--- here
>> throw new InvalidAttributeValueException("no values for " +
>> "attribute "" +
>> tagName + """);
>> }
>>
>> There is a subsequent access to values[0], which means the failure would
>> throw `AIOOB`, not `InvalidAttributeValueException`.
>>
>> Additional testing:
>> - [x] Linux x86_64 fastdebug, `com/sun/jndi`
>
> Seems fine. Lets hope no caller relies on this throwing AIOOBE.
>
> ..Thomas
> This looks right but I'm surprised it isn't caught by tests (@AlekseiEfimov -
> do you have suggests for tests that would be useful to add here?)
Can we go without adding tests here? This seems quite trivial.
-
PR: https://git.openjdk.java.net/jdk/pull/2968
Re: RFR: 8261785: Calling "main" method in anonymous nested class crashes the JVM
On Tue, 16 Mar 2021 01:55:49 GMT, Sergey Bylokhov wrote: >> This patch ensure launcher won't crash JVM for the new static Methods from >> local/anonymous class on MacOS. >> >> As @dholmes-ora pointed out in the analysis, this is a MacOS specific bug >> when the launcher trying to grab class name to be displayed as the >> Application name on the menu. >> >> The fix is to not setting name, test shows that GUI java application shows >> 'bin' as the application name. It's possible for us to set the name to >> something more friendly, for example, "Java", but I am not sure that should >> be launcher's responsibility to choose such a default name. It seems to me >> the consumer of the JAVA_MAIN_CLASS_%d environment variable should be >> responsible to pick such name in case the environment variable is not set. > > This bug is similar to https://bugs.openjdk.java.net/browse/JDK-8076264, and > the fix looks fine. > Maybe the AWT folk should decide what name should be displayed in this > case. The canonical name was chosen when all main classes had to have a > canonical name. So perhaps a simple name will suffice in the case where > there is no canonical name? This is not the last attempt to set the name, the JAVA_MAIN_CLASS_ variable is used in the middle of the name selection, there are some others. And the "bin" is selected by some of the next step, I agree it is not a friendly name that could be improved. - PR: https://git.openjdk.java.net/jdk/pull/2999
Re: RFR: 8263108: Class initialization deadlock in java.lang.constant [v4]
On Tue, 16 Mar 2021 02:37:24 GMT, Jaikiran Pai wrote: >> Can I please get a review for this proposed patch for the issue reported in >> https://bugs.openjdk.java.net/browse/JDK-8263108? >> >> As noted in that issue, the `java.lang.constant.DynamicConstantDesc` and >> `java.lang.constant.ConstantDescs` can end up in a classloading deadlock due >> to the nature of the code involved in their static blocks. A thread dump of >> one such deadlock (reproduced using the code attached to that issue) is as >> follows: >> >> "Thread A" #14 prio=5 os_prio=31 cpu=101.45ms elapsed=8.30s >> tid=0x7ff88e01ca00 nid=0x6003 in Object.wait() [0x7a4c6000] >>java.lang.Thread.State: RUNNABLE >> at >> java.lang.constant.DynamicConstantDesc.(java.base@16-ea/DynamicConstantDesc.java:67) >> - waiting on the Class initialization monitor for >> java.lang.constant.ConstantDescs >> at Deadlock.threadA(Deadlock.java:14) >> at Deadlock$$Lambda$1/0x000800c00a08.run(Unknown Source) >> at java.lang.Thread.run(java.base@16-ea/Thread.java:831) >> >> "Thread B" #15 prio=5 os_prio=31 cpu=103.15ms elapsed=8.30s >> tid=0x7ff88b936a00 nid=0x9b03 in Object.wait() [0x7a5c9000] >>java.lang.Thread.State: RUNNABLE >> at >> java.lang.constant.ClassDesc.ofDescriptor(java.base@16-ea/ClassDesc.java:145) >> - waiting on the Class initialization monitor for >> java.lang.constant.DynamicConstantDesc >> at >> java.lang.constant.ConstantDescs.(java.base@16-ea/ConstantDescs.java:239) >> at Deadlock.threadB(Deadlock.java:24) >> at Deadlock$$Lambda$2/0x000800c00c28.run(Unknown Source) >> at java.lang.Thread.run(java.base@16-ea/Thread.java:831) >> >> The commit in this PR resolves that deadlock by moving the `canonicalMap` >> initialization in `DynamicConstantDesc`, from the static block, to a lazily >> initialized map, into the `tryCanonicalize` (private) method of the same >> class. That's the only method which uses this map. >> >> The implementation in `tryCanonicalize` carefully ensures that the deadlock >> isn't shifted from the static block to this method, by making sure that the >> `synchronization` on `DynamicConstantDesc` in this method happens only when >> it's time to write the state to the shared `canonicalMap`. This has an >> implication that the method local variable `canonDescs` can get initialized >> by multiple threads unnecessarily but from what I can see, that's the only >> way we can avoid this deadlock. This penalty of multiple threads >> initializing and then throwing away that map isn't too huge because that >> will happen only once when the `canonicalMap` is being initialized for the >> first time. >> >> An alternate approach that I thought of was to completely get rid of this >> shared cache `canonicalMap` and instead just use method level map (that gets >> initialized each time) in the `tryCanonicalize` method (thus requiring no >> locks/synchronization). I ran a JMH benchmark with the current change >> proposed in this PR and with the alternate approach of using the method >> level map. The performance number from that run showed that the approach of >> using the method level map has relatively big impact on performance (even >> when there's a cache hit in that method). So I decided to not pursue that >> approach. I'll include the benchmark code and the numbers in a comment in >> this PR, for reference. >> >> The commit in this PR also includes a jtreg testcase which (consistently) >> reproduces the deadlock without the fix and passes when this fix is applied. >> Extra manual testing has been done to verify that the classes of interest >> (noted in that testcase) are indeed getting loaded in those concurrent >> threads and not in the main thread. For future maintainers, there's a >> implementation note added on that testcase explaining why it cannot be moved >> into testng test. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Use Chris' suggested solution of avoiding deadlock Looks good to me. - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/2893
