Re: Code Review Request, 8152237 Support BigInteger.TWO
On 3/23/2016 12:10 PM, Wang Weijun wrote: Only 3 files touched. Are you going to make the s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with another bug fix? There are also uses in security components. I will make the update in another bug. Thanks, Xuelei Thanks Max On Mar 23, 2016, at 11:26 AM, Xuelei Fan wrote: Hi, Please review the update for the supporting of BigInteger.TWO: http://cr.openjdk.java.net/~xuelei/8152237/webrev/ BigInteger.valueOf(2) is a common BigInteger value used in binary and cryptography operation calculation. The BigInteger.TWO is not exported, and hence BigInteger.valueOf(2) is used instead in applications and JDK components. The export of static BigInteger.TWO can improve performance and simplify existing code. Thanks, Xuelei
Re: Code Review Request, 8152237 Support BigInteger.TWO
Only 3 files touched. Are you going to make the s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with another bug fix? Thanks Max > On Mar 23, 2016, at 11:26 AM, Xuelei Fan wrote: > > Hi, > > Please review the update for the supporting of BigInteger.TWO: > > http://cr.openjdk.java.net/~xuelei/8152237/webrev/ > > BigInteger.valueOf(2) is a common BigInteger value used in binary and > cryptography operation calculation. The BigInteger.TWO is not exported, and > hence BigInteger.valueOf(2) is used instead in applications and JDK > components. The export of static BigInteger.TWO can improve performance and > simplify existing code. > > Thanks, > Xuelei
Code Review Request, 8152237 Support BigInteger.TWO
Hi, Please review the update for the supporting of BigInteger.TWO: http://cr.openjdk.java.net/~xuelei/8152237/webrev/ BigInteger.valueOf(2) is a common BigInteger value used in binary and cryptography operation calculation. The BigInteger.TWO is not exported, and hence BigInteger.valueOf(2) is used instead in applications and JDK components. The export of static BigInteger.TWO can improve performance and simplify existing code. Thanks, Xuelei
Re: Review request: 8152503: tools/javac/completionDeps/DepsAndAnno.java fails after jigsaw m3
Looks fine Mandy; thanks, -Joe On 3/22/2016 6:46 PM, Mandy Chung wrote: Also problem list a new test tools/jdeps/moidules/GenModuleInfo.java diff --git a/test/tools/javac/completionDeps/DepsAndAnno.java b/test/tools/javac/completionDeps/DepsAndAnno.java --- a/test/tools/javac/completionDeps/DepsAndAnno.java +++ b/test/tools/javac/completionDeps/DepsAndAnno.java @@ -26,6 +26,10 @@ * @bug 8078600 * @summary Make sure -XDcompletionDeps does not cause an infinite loop. * @library /tools/lib + * @modules jdk.compiler/com.sun.tools.javac.api + * jdk.compiler/com.sun.tools.javac.file + * jdk.compiler/com.sun.tools.javac.main + * jdk.jdeps/com.sun.tools.javap * @build ToolBox * @run main/othervm/timeout=10 DepsAndAnno */ diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -91,3 +91,9 @@ tools/sjavac/IncCompileFullyQualifiedRef.java 8152055generic-allRequires dependency code to deal with in-method dependencies. tools/sjavac/IncCompileWithChanges.java 8152055generic-allRequires dependency code to deal with in-method dependencies. +### +# +# jdeps + +tools/jdeps/moidules/GenModuleInfo.java 8152502windows-allfails to clean up files +
Review request: 8152503: tools/javac/completionDeps/DepsAndAnno.java fails after jigsaw m3
Also problem list a new test tools/jdeps/moidules/GenModuleInfo.java diff --git a/test/tools/javac/completionDeps/DepsAndAnno.java b/test/tools/javac/completionDeps/DepsAndAnno.java --- a/test/tools/javac/completionDeps/DepsAndAnno.java +++ b/test/tools/javac/completionDeps/DepsAndAnno.java @@ -26,6 +26,10 @@ * @bug 8078600 * @summary Make sure -XDcompletionDeps does not cause an infinite loop. * @library /tools/lib + * @modules jdk.compiler/com.sun.tools.javac.api + * jdk.compiler/com.sun.tools.javac.file + * jdk.compiler/com.sun.tools.javac.main + * jdk.jdeps/com.sun.tools.javap * @build ToolBox * @run main/othervm/timeout=10 DepsAndAnno */ diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -91,3 +91,9 @@ tools/sjavac/IncCompileFullyQualifiedRef.java 8152055generic-allRequires dependency code to deal with in-method dependencies. tools/sjavac/IncCompileWithChanges.java 8152055generic-allRequires dependency code to deal with in-method dependencies. +### +# +# jdeps + +tools/jdeps/moidules/GenModuleInfo.java 8152502windows-allfails to clean up files +
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
> On Mar 22, 2016, at 5:24 AM, Per Liden wrote: > One thing I like about this approach is that it's only the ReferenceHandler > thread that pops of elements from the pending list and enqueues them. That > simplifies things a lot. I like that too. And hopefully we really can get rid of sun.misc.Cleaner (under whatever name). > From a GC perspective I would however like to get away from the shared > pending list and the pending list lock entirety and instead provide a VM > downcall to get the pending list. The goal would of course be to have a more > robust way of transferring the pending list to Java land, instead of today's > secret handshake which is easy to get wrong. Also, not requiring the pending > list lock (which is a Java monitor) to be held during a GC would also > simplify things a lot on the GC side. E.g. the > ReferencePendingListLockerThread could be removed completely. I’ve been thinking along the same lines. I think having the pending list (and associated locking and notification) in Java is just making life difficult for ourselves, and that things could be much simpler if that whole protocol was owned by the VM. Once the reference handler thread has obtained the latest list, if it then wants to publish that list for other Java threads to help process, that’s a policy choice that can be explored on the Java side, with no impact on the VM (including the GC).
Re: Commenting on bug JDK-8151981
On 03/18/2016 08:40 AM, Jan Bauer Nielsen wrote: > If there is anything we can do to help You investigate this problem, > please let us know? It's pretty much impossible to fix a bug if we don't have a reproducer. Andrew.
Re: RFR: regex changes
Thanks Roger!
webrev has been updated accordingly. See comments below.
http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/webrev/
On 03/22/2016 12:48 PM, Roger Riggs wrote:
Hi Sherman,
A few more comments,
Pattern.java:
- 1782: typo: "deterministci"
fixed.
- 2176: commented out code?
Yes, commented out for now. It helps the case below, which has "lots"
of ".*" lined up in a single patter. But I have not decided if it's worth the
potential cost of adding a "backtracking stopper" for each ".*". In most
this kind of cases, it gets slow, very slow, but still come back alive,
instead of a complete hangup.
// 5014450-> top level single greedy ...
{ "^\\s*sites\\[sites\\.length\\+\\+\\]\\s*=\\s*new\\s+Array\\(.*" +
"\\s*//\\s*(\\d+).*" +
"\\s*//\\s*([^-]+).*" +
"\\s*//\\s*([^-]+).*" +
"\\s*//\\s*([^-]+).*" +
"/(?sui).*$",
"\tsites[sites.length++] = new Array(\n" +
"// 1079193366647 - creation time\n" +
"// 1078902678663 1078852539723 1078753482632 0 0 0 0 0 0 0 0 0 0 0 -
creation time last 14 days\n" +
"// 0 1 0 0 0 0 0 0 0 0 0 0 0 0 - bad\n" +
"// 0.0030 0.0080 0.01 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
-\n\n",
false
},
I would leave it for now. There is a bugid for it. So we can address it
separately,
might with better tradeoff.
- 2667: indentation
fixed.
- 5660,5655: lambda syntax use of simply "ch" is preferred over "(ch)" for
single parameter lambdas
and for consistency within the file.
fixed.
PrintPattern.java needs the standard copyright text if it is going to be in the
repo.
fixed.
- 29: The Print(fmt, args...) method should follow the method naming
convention. (initial lowercase)
fixed.
IntHashSet: does performance matter enough to warrant adding this extra code.
The measurement I did suggests it's still worth adding such a small piece code,
given
this one probably will be used for most of the greedy {}, with lots of raw
"int" in and
out, without boxing, and much smaller footprint.
Thanks again,
Sherman
On 3/18/2016 4:05 PM, Xueming Shen wrote:
Hi,
There are couple regex related changes waiting for review. I have pull them
together here (with the notes) to make it easy to review.
http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/webrev/
(1) Exponential backtracking
Note:
http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/backtracking
https://bugs.openjdk.java.net/browse/JDK-6328855
https://bugs.openjdk.java.net/browse/JDK-6192895
https://bugs.openjdk.java.net/browse/JDK-6345469
https://bugs.openjdk.java.net/browse/JDK-6988218
https://bugs.openjdk.java.net/browse/JDK-6693451
https://bugs.openjdk.java.net/browse/JDK-7006761
https://bugs.openjdk.java.net/browse/JDK-8140212
(2) Anonymous class to lambda function cleanup
Note:
http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/lambdafunction
https://bugs.openjdk.java.net/browse/JDK-8151481
https://bugs.openjdk.java.net/browse/JDK-6609854
(3) Canonical Equivalents
Note: http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/canonEQ
https://bugs.openjdk.java.net/browse/JDK-4916384
https://bugs.openjdk.java.net/browse/JDK-4867170
https://bugs.openjdk.java.net/browse/JDK-6995635
https://bugs.openjdk.java.net/browse/JDK-6728861
https://bugs.openjdk.java.net/browse/JDK-6736245
https://bugs.openjdk.java.net/browse/JDK-7080302
Thanks
Sherman
Re: Review request for 8151571: System/LoggerFinder tests fail after JDK-8149925
Hi Mandy, Nice to see this fixed. It looks good to me. Thanks! Regads, Peter On 03/22/2016 07:41 PM, Mandy Chung wrote: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8151571/webrev.00/ This restores the change in NativeBuffer to use the common Cleaner created by the system. JDK-8149925 causes InnocuousThread be created early during startup before the system class loader is initialized. Since the common Cleaner is intended for system cleaning code to use, this patch now creates InnocuousThread with null TCCL and expects the system cleaning code to handle null TCCL only if it uses it. Mandy
Commenting on bug JDK-8151981
Hello I'm from the company experiencing issue [1] mentioned in bug JDK-8151981. If there is anything we can do to help You investigate this problem, please let us know? Kind regards, Jan Bauer Nielsen Software developer DBC as http://www.dbc.dk/english
Re: RFR: regex changes
Hi Sherman, A few more comments, Pattern.java: - 1782: typo: "deterministci" - 2176: commented out code? - 2667: indentation - 5660,5655: lambda syntax use of simply "ch" is preferred over "(ch)" for single parameter lambdas and for consistency within the file. PrintPattern.java needs the standard copyright text if it is going to be in the repo. - 29: The Print(fmt, args...) method should follow the method naming convention. (initial lowercase) IntHashSet: does performance matter enough to warrant adding this extra code. Roger On 3/18/2016 4:05 PM, Xueming Shen wrote: Hi, There are couple regex related changes waiting for review. I have pull them together here (with the notes) to make it easy to review. http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/webrev/ (1) Exponential backtracking Note: http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/backtracking https://bugs.openjdk.java.net/browse/JDK-6328855 https://bugs.openjdk.java.net/browse/JDK-6192895 https://bugs.openjdk.java.net/browse/JDK-6345469 https://bugs.openjdk.java.net/browse/JDK-6988218 https://bugs.openjdk.java.net/browse/JDK-6693451 https://bugs.openjdk.java.net/browse/JDK-7006761 https://bugs.openjdk.java.net/browse/JDK-8140212 (2) Anonymous class to lambda function cleanup Note: http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/lambdafunction https://bugs.openjdk.java.net/browse/JDK-8151481 https://bugs.openjdk.java.net/browse/JDK-6609854 (3) Canonical Equivalents Note: http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/canonEQ https://bugs.openjdk.java.net/browse/JDK-4916384 https://bugs.openjdk.java.net/browse/JDK-4867170 https://bugs.openjdk.java.net/browse/JDK-6995635 https://bugs.openjdk.java.net/browse/JDK-6728861 https://bugs.openjdk.java.net/browse/JDK-6736245 https://bugs.openjdk.java.net/browse/JDK-7080302 Thanks Sherman
Re: JDK 9 RFR of 8151957: ObjectInputStream.java -incorrect HashMap initial size allocation, and useless variable set
On 2016-03-22 19:44, Brian Burkhalter wrote: On Mar 22, 2016, at 11:38 AM, Brian Burkhalter wrote: Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8151957 Patch: http://cr.openjdk.java.net/~bpb/8151957/webrev.00/ Summary: Modify the initialization of primClasses primitive type name-to-class object table to use the new Map.of() convenience method with nine key-value pairs. This patch applies to JDK 9 only. As indicated in the comments, the JDK aspects are not worth addressing. Correction: I intended “the JDK *8* aspects are not worth addressing.” Brian Looks good to me! /Claes
Re: JDK 9 RFR of 8151957: ObjectInputStream.java -incorrect HashMap initial size allocation, and useless variable set
Looks good. Like these new convienence methods. On Mar 22, 2016, at 2:38 PM, Brian Burkhalter wrote: > Please review at your convenience. > > Issue:https://bugs.openjdk.java.net/browse/JDK-8151957 > Patch:http://cr.openjdk.java.net/~bpb/8151957/webrev.00/ > > Summary: > Modify the initialization of primClasses primitive type name-to-class object > table to use the new Map.of() convenience method with nine key-value pairs. > This patch applies to JDK 9 only. As indicated in the comments, the JDK > aspects are not worth addressing. > > Thanks, > > Brian Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: JDK 9 RFR of 8151957: ObjectInputStream.java -incorrect HashMap initial size allocation, and useless variable set
On Mar 22, 2016, at 11:38 AM, Brian Burkhalter wrote: > Please review at your convenience. > > Issue:https://bugs.openjdk.java.net/browse/JDK-8151957 > Patch:http://cr.openjdk.java.net/~bpb/8151957/webrev.00/ > > Summary: > Modify the initialization of primClasses primitive type name-to-class object > table to use the new Map.of() convenience method with nine key-value pairs. > This patch applies to JDK 9 only. As indicated in the comments, the JDK > aspects are not worth addressing. Correction: I intended “the JDK *8* aspects are not worth addressing.” Brian
Review request for 8151571: System/LoggerFinder tests fail after JDK-8149925
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8151571/webrev.00/ This restores the change in NativeBuffer to use the common Cleaner created by the system. JDK-8149925 causes InnocuousThread be created early during startup before the system class loader is initialized. Since the common Cleaner is intended for system cleaning code to use, this patch now creates InnocuousThread with null TCCL and expects the system cleaning code to handle null TCCL only if it uses it. Mandy
JDK 9 RFR of 8151957: ObjectInputStream.java -incorrect HashMap initial size allocation, and useless variable set
Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8151957 Patch: http://cr.openjdk.java.net/~bpb/8151957/webrev.00/ Summary: Modify the initialization of primClasses primitive type name-to-class object table to use the new Map.of() convenience method with nine key-value pairs. This patch applies to JDK 9 only. As indicated in the comments, the JDK aspects are not worth addressing. Thanks, Brian
Re: 8152436: Add a test to verify that the root logger correctly reports the caller's information
Understood. When I tested a subclassed logger all of the JDKs work correctly.
I didn't check but it just seems like something that could regress if there is
no test.
Regards,
Jason
From: Daniel Fuchs
Sent: Tuesday, March 22, 2016 11:29 AM
To: Jason Mehrens; core-libs-dev
Subject: Re: 8152436: Add a test to verify that the root logger correctly
reports the caller's information
On 22/03/16 17:11, Jason Mehrens wrote:
> Hi Daniel,
>
> I think we just need to add a test where a subclass logger logs the message
> with the root logger as the parent. Should be no need to register that
> logger with the LogManager.
> Not sure if it is worth checking that custom filters installed on handlers or
> logger and or formatters fool anything.
Hi Jason,
That would be another issue.
I believe the current behavior in 9 is that the custom logger
subclass will appear as the emitter of the log message, unless
it also implements System.Logger.
Now that we use the StackWalker API we have the opportunity
to use Class.isAssignableFrom to do the filtering - so we
could fix LogRecord::inferLogger to skip subclasses of
java.util.logging.Logger as well - which we can't do in 8
(well we could use Reflection.getCallerClass(int) in 8, but
that would not be a great idea - so I'd prefer to keep that as
a limitation for 8).
The test below is mainly to verify JDK-8152389 - which is about
verifying that calling Logger.getLogger("").() does
not report LogManager$RootLogger as the calling frame.
I should probably however log an RFE against 9 to skip
custom subclasses of java.util.logging.Logger when
inferring caller information. And then I'll just have to
extend the new test to also test this new scenario.
best regards,
-- daniel
>
> Regards,
>
> Jason
>
>
> From: core-libs-dev on behalf of
> Daniel Fuchs
> Sent: Tuesday, March 22, 2016 10:03 AM
> To: core-libs-dev
> Subject: RFR: 8152436: Add a test to verify that the root logger correctly
> reports the caller's information
>
> Hi,
>
> Please find below a new test that verifies that JDK-8152389 does
> not occur in JDK 9.
>
> bug:
> 8152436: Add a test to verify that the root logger correctly
>reports the caller's information
> https://bugs.openjdk.java.net/browse/JDK-8152436
>
> Issue being verified by the test:
> https://bugs.openjdk.java.net/browse/JDK-8152389
>
> Webrev (test only):
> http://cr.openjdk.java.net/~dfuchs/webrev_8152436/webrev.00/
>
> best regards,
>
> -- daniel
>
Re: 8152436: Add a test to verify that the root logger correctly reports the caller's information
On 22/03/16 17:11, Jason Mehrens wrote:
Hi Daniel,
I think we just need to add a test where a subclass logger logs the message
with the root logger as the parent. Should be no need to register that logger
with the LogManager.
Not sure if it is worth checking that custom filters installed on handlers or
logger and or formatters fool anything.
Hi Jason,
That would be another issue.
I believe the current behavior in 9 is that the custom logger
subclass will appear as the emitter of the log message, unless
it also implements System.Logger.
Now that we use the StackWalker API we have the opportunity
to use Class.isAssignableFrom to do the filtering - so we
could fix LogRecord::inferLogger to skip subclasses of
java.util.logging.Logger as well - which we can't do in 8
(well we could use Reflection.getCallerClass(int) in 8, but
that would not be a great idea - so I'd prefer to keep that as
a limitation for 8).
The test below is mainly to verify JDK-8152389 - which is about
verifying that calling Logger.getLogger("").() does
not report LogManager$RootLogger as the calling frame.
I should probably however log an RFE against 9 to skip
custom subclasses of java.util.logging.Logger when
inferring caller information. And then I'll just have to
extend the new test to also test this new scenario.
best regards,
-- daniel
Regards,
Jason
From: core-libs-dev on behalf of Daniel
Fuchs
Sent: Tuesday, March 22, 2016 10:03 AM
To: core-libs-dev
Subject: RFR: 8152436: Add a test to verify that the root logger correctly
reports the caller's information
Hi,
Please find below a new test that verifies that JDK-8152389 does
not occur in JDK 9.
bug:
8152436: Add a test to verify that the root logger correctly
reports the caller's information
https://bugs.openjdk.java.net/browse/JDK-8152436
Issue being verified by the test:
https://bugs.openjdk.java.net/browse/JDK-8152389
Webrev (test only):
http://cr.openjdk.java.net/~dfuchs/webrev_8152436/webrev.00/
best regards,
-- daniel
RFR: 8152436: Add a test to verify that the root logger correctly reports the caller's information
Hi, Please find below a new test that verifies that JDK-8152389 does not occur in JDK 9. bug: 8152436: Add a test to verify that the root logger correctly reports the caller's information https://bugs.openjdk.java.net/browse/JDK-8152436 Issue being verified by the test: https://bugs.openjdk.java.net/browse/JDK-8152389 Webrev (test only): http://cr.openjdk.java.net/~dfuchs/webrev_8152436/webrev.00/ best regards, -- daniel
Re: JAXP default implementation and JDK-8152063
Hi David, On 22/03/16 13:53, David M. Lloyd wrote: Am I understanding it correctly that you're pointing the system property to a "proxy" as stated in JDK-8152063, not an actual implementation? So it's sort of a provider-locating mechanism outside of jaxp, that locates the actual implementation, and if it cannot find one, it attempts to fall back to and directly instantiate the JDK's default. Is that why "it will fail because we can't access the JDK's default implementations"? Correct. Was it because the ServiceLoader mechanism was not meeting your requirement that you needed a proxy? Right. Most user code using JAXP factories will just call e.g. XMLInputFactory.newFactory() with no arguments. So the ServiceLoader mechanism is fine when we want to allow the user to override the implementation with one they've bundled in their WAR or whatever. But if they do not do so, we are forced to ensure that another implementation is on the deployment class path. It's even worse in contexts where we cannot control what the TCCL is set to when the factory methods are called. There's just no practical way to ensure that our implementation is visible from every class loader. We can however "hack" our proxy implementations on to every class loader, and then later set the default factory class loader to use (which then just uses the same ServiceLoader-style load again, but from the class loader we choose). If I understand well, what you need is to be able to install a service provider that can create an instance of the JDK default implementation in order to wrap it. I assume such a service provider would have to be deployed in the System ClassLoader - so that it could be shadowed by whichever provider is declared in the context class loader. In order to create an instance of the JDK default implementation, have you explored using the factory method that takes a ClassLoader, and passing the extension class loader as parameter? But maybe I'm misunderstanding the whole issue. Would adding a method that resolves the concrete service implementation against a Layer supplied by the caller be of any help in your scenario? best regards, -- daniel
Re: RFR: JDK-8152352 Compiling warnings in zip_util.c blocks devkit to build with --with-zlib=system
Looks fine. Roger On 5/21/2016 5:06 PM, Xueming Shen wrote: Hi, Please help review the change to remove two compiling warnings in zip_util.c issue: https://bugs.openjdk.java.net/browse/JDK-8152352 webrev: http://cr.openjdk.java.net/~sherman/8152352/webrev/ thanks, Sherman
Re: JAXP default implementation and JDK-8152063
On 03/21/2016 07:42 PM, huizhe wang wrote: On 3/21/2016 12:55 PM, David M. Lloyd wrote: On 03/21/2016 01:38 PM, Alan Bateman wrote: On 21/03/2016 14:39, David M. Lloyd wrote: This message is in reference to the aforementioned issue [1] that I've requested my colleague Andrew Hughes to open on my behalf. As expressed in that issue, the means of specifying a JAXP default provider (via system property which specifies a class name on the application class path) is relatively antiquated at this point. It is actually quite difficult for containers which ship their own JAXP implementation to use the JDK APIs and, at the same time, ship their own implementations, especially without polluting the class path of applications which use JAXP. It would be good to expand a bit on what you are looking to do. Are you looking to change the system-wide default or are you looking for the container/app server to see a different default to the applications that it runs? As things currently stand then then default can be overridden by deploying the XML parser on the class path or as a service provider module on the module path. The JPMS requirements document includes a requirement for ordering service providers that isn't implemented yet. If/when that happens then it might help with part of this. I need to (optionally) change the system default globally, but from code that is (probably) not on the module path of the base module layer (in other words, it's coming from a layer which is loaded after the boot module is run). I can use the service provider logic to always hook in a redirection layer (like we do today by way of the system properties), however if we do this but there is/are no overriding implementation(s) for one or more of these APIs, then AFAICT it will fail with Jigsaw because we can't access the JDK's default implementations (or even know what they are, barring reflection & etc.), which pretty well puts us back to square one. Am I understanding it correctly that you're pointing the system property to a "proxy" as stated in JDK-8152063, not an actual implementation? So it's sort of a provider-locating mechanism outside of jaxp, that locates the actual implementation, and if it cannot find one, it attempts to fall back to and directly instantiate the JDK's default. Is that why "it will fail because we can't access the JDK's default implementations"? Correct. Was it because the ServiceLoader mechanism was not meeting your requirement that you needed a proxy? Right. Most user code using JAXP factories will just call e.g. XMLInputFactory.newFactory() with no arguments. So the ServiceLoader mechanism is fine when we want to allow the user to override the implementation with one they've bundled in their WAR or whatever. But if they do not do so, we are forced to ensure that another implementation is on the deployment class path. It's even worse in contexts where we cannot control what the TCCL is set to when the factory methods are called. There's just no practical way to ensure that our implementation is visible from every class loader. We can however "hack" our proxy implementations on to every class loader, and then later set the default factory class loader to use (which then just uses the same ServiceLoader-style load again, but from the class loader we choose). -- - DML
RFR 8150829: Enhanced drop-args, identity and default constant, varargs adjustment
Hi All, Please review the following- https://bugs.openjdk.java.net/browse/JDK-8150829 http://cr.openjdk.java.net/~srastogi/8150829/webrev.04 Thanks, Shilpi
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
Hi Peter,
On 2016-03-21 16:30, Peter Levart wrote:
Hi Per,
May I point you to my proposed change in Reference(Handler) for JDK 9,
being discussed in the thread about JDK-8149925. It will hopefully
remove the special-casing of sun.misc.Cleaner, change the way how
pending references are being enqueued by ReferenceHandler thread and how
other thread(s) can synchronize with it. Since you seem to have a great
knowledge of VM part of things, I would very much like to hear what you
think of that change. Here's the latest webrev:
http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.08.part2/
(see Reference.java and Bits.java for an example of how this
synchronization with ReferenceHandler thread is to be used)
One thing I like about this approach is that it's only the
ReferenceHandler thread that pops of elements from the pending list and
enqueues them. That simplifies things a lot.
From a GC perspective I would however like to get away from the shared
pending list and the pending list lock entirety and instead provide a VM
downcall to get the pending list. The goal would of course be to have a
more robust way of transferring the pending list to Java land, instead
of today's secret handshake which is easy to get wrong. Also, not
requiring the pending list lock (which is a Java monitor) to be held
during a GC would also simplify things a lot on the GC side. E.g. the
ReferencePendingListLockerThread could be removed completely.
So, I imagine the ReferenceHandler could do something like this:
while (true) {
// getPendingReferences() is a downcall to the VM which
// blocks until the pending list becomes non-empty and
// returns the whole list, transferring it to from VM-land
// to Java-land in a safe and robust way.
Reference pending = getPendingReferences();
// Enqueue the references
while (pending != null) {
Reference r = pending;
pending = r.discovered;
r.discovered = null;
ReferenceQueue q = r.queue;
if (q != ReferenceQueue.NULL) {
q.enqueue(r);
}
}
}
I haven't thought through the details when it comes having additional
Java threads helping out with Cleaners. The ReferenceHandler would be
free to use whatever lists/locks is wants to handle this and the GC
wouldn't know anything about it. But, with the above approach at least
the interface between the ReferenceHandler and the VM would be pretty
clear and hard(er) to misuse.
cheers,
Per
Regards, Peter
On 03/21/2016 04:13 PM, Peter Levart wrote:
Hi Per, David,
As things stand, there is a very good chance that sun.misc.Cleaner
will go away in JDK9, so all this speculation about the source of
OOME(s) can be put to rest. But for JDK 8u, I agree that this should
be sorted out.
My feeling is that (instanceof Cleaner) can not result in allocation
and therefore can not trigger OOME if the Cleaner class is already
loaded at that time. I think that we were chasing the wrong rabbit. As
I have found later, there is a much more probable cause for
ReferenceHandler thread dying with OOME after the fix to catch OOME
from lock.wait(). It is triggered by the invocation of Cleaner.clean()
later down in the code. I even created a reproducer for it. See my
last two comments of the following issue:
https://bugs.openjdk.java.net/browse/JDK-8066859
(but don't look at the proposed fix since it is not very good)
I think that for JDK 8u we could revert the code and do (instanceof
Cleaner) checks outside the synchronized block and in addition, find a
way to handle the OOME thrown from Cleaner.clean().
What do you think?
Regards, Peter
On 03/21/2016 02:41 PM, Per Liden wrote:
Hi David,
On 2016-03-21 13:49, David Holmes wrote:
Hi Per,
On 21/03/2016 10:20 PM, Per Liden wrote:
Hi Peter & David,
(Resurrecting an old thread here...)
On 2014-01-22 03:19, David Holmes wrote:
Hi Peter,
On 22/01/2014 12:00 AM, Peter Levart wrote:
Hi, David, Kalyan,
Summing up the discussion, I propose the following patch for
ReferenceHandler:
http://cr.openjdk.java.net/~plevart/jdk9-dev/OOMEInReferenceHandler/webrev.01/
I can live with it, though it maybe that once Cleaner has been
preloaded
instanceof can no longer throw OOME. Can't be 100% sure. And there's
some duplication/verbosity in the commentary that could be trimmed
down :)
While investigating a Reference pending list issue on the GC side of
things I looked at the ReferenceHandler thread and noticed something
which made me uneasy. The fix for JDK-8022321 added pre-loading of the
Cleaer class to avoid OMME, but also moved the "instanceof Cleaner"
inside the try/catch with a comment that it "sometimes" can throw an
OOME. I understand this was done because we're not 100% sure if a OOME
can still happen here, despite the pre-loading.
However, if it can throw an OOME that means it's allocating, which in
turn means it can provoke a GC. If that happens, it looks to me
like we
have a bug he
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 2016-03-21 18:32, Kim Barrett wrote: On Mar 21, 2016, at 8:20 AM, Per Liden wrote: Hi Peter & David, (Resurrecting an old thread here...) On 2014-01-22 03:19, David Holmes wrote: Hi Peter, On 22/01/2014 12:00 AM, Peter Levart wrote: Hi, David, Kalyan, Summing up the discussion, I propose the following patch for ReferenceHandler: http://cr.openjdk.java.net/~plevart/jdk9-dev/OOMEInReferenceHandler/webrev.01/ I can live with it, though it maybe that once Cleaner has been preloaded instanceof can no longer throw OOME. Can't be 100% sure. And there's some duplication/verbosity in the commentary that could be trimmed down :) While investigating a Reference pending list issue on the GC side of things I looked at the ReferenceHandler thread and noticed something which made me uneasy. The fix for JDK-8022321 added pre-loading of the Cleaer class to avoid OMME, but also moved the "instanceof Cleaner" inside the try/catch with a comment that it "sometimes" can throw an OOME. I understand this was done because we're not 100% sure if a OOME can still happen here, despite the pre-loading. However, if it can throw an OOME that means it's allocating, which in turn means it can provoke a GC. If that happens, it looks to me like we have a bug here. The ReferenceHandler thread is not allowed to provoke a GC while it's holding on to the pending list lock, since the pending list might be updated during a GC and "pending = r.discovered" will than overwrite something other than "r", silently dropping any newly discovered References which will never be discovered by the the GC again. On the other hand, if an OOME can never happen (i.e. no GC) here then we're good the comment is just incorrect. The instanceof check could be moved out of the try/catch block again, like it was prior to this change, just to make it obvious that we will not be able to cause new allocations inside the critical section. Or at a minimum, the comment saying OOME can still happen should be adjusted. Thoughts? thanks, Per Btw, to the best of my knowledge, the pre-loading of Cleaner should avoid any GC activity from instanceof, but I can't say that am a 100% sure either. Per - I think you are raising the same issue as discussed in https://bugs.openjdk.java.net/browse/JDK-8055232. Ah, thanks Kim for pointing that out. cheers, Per
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
Hi Peter,
On 2016-03-21 16:13, Peter Levart wrote:
Hi Per, David,
As things stand, there is a very good chance that sun.misc.Cleaner will
go away in JDK9, so all this speculation about the source of OOME(s) can
be put to rest. But for JDK 8u, I agree that this should be sorted out.
My feeling is that (instanceof Cleaner) can not result in allocation and
therefore can not trigger OOME if the Cleaner class is already loaded at
that time. I think that we were chasing the wrong rabbit. As I have
found later, there is a much more probable cause for ReferenceHandler
thread dying with OOME after the fix to catch OOME from lock.wait(). It
is triggered by the invocation of Cleaner.clean() later down in the
code. I even created a reproducer for it. See my last two comments of
the following issue:
https://bugs.openjdk.java.net/browse/JDK-8066859
(but don't look at the proposed fix since it is not very good)
I think that for JDK 8u we could revert the code and do (instanceof
Cleaner) checks outside the synchronized block and in addition, find a
way to handle the OOME thrown from Cleaner.clean().
What do you think?
That sound good to me. With the addition of the try/catch around
Cleaner.clean() catching not just OOME, but all Throwables, right?
cheers,
Per
Regards, Peter
On 03/21/2016 02:41 PM, Per Liden wrote:
Hi David,
On 2016-03-21 13:49, David Holmes wrote:
Hi Per,
On 21/03/2016 10:20 PM, Per Liden wrote:
Hi Peter & David,
(Resurrecting an old thread here...)
On 2014-01-22 03:19, David Holmes wrote:
Hi Peter,
On 22/01/2014 12:00 AM, Peter Levart wrote:
Hi, David, Kalyan,
Summing up the discussion, I propose the following patch for
ReferenceHandler:
http://cr.openjdk.java.net/~plevart/jdk9-dev/OOMEInReferenceHandler/webrev.01/
I can live with it, though it maybe that once Cleaner has been
preloaded
instanceof can no longer throw OOME. Can't be 100% sure. And there's
some duplication/verbosity in the commentary that could be trimmed
down :)
While investigating a Reference pending list issue on the GC side of
things I looked at the ReferenceHandler thread and noticed something
which made me uneasy. The fix for JDK-8022321 added pre-loading of the
Cleaer class to avoid OMME, but also moved the "instanceof Cleaner"
inside the try/catch with a comment that it "sometimes" can throw an
OOME. I understand this was done because we're not 100% sure if a OOME
can still happen here, despite the pre-loading.
However, if it can throw an OOME that means it's allocating, which in
turn means it can provoke a GC. If that happens, it looks to me like we
have a bug here. The ReferenceHandler thread is not allowed to
provoke a
GC while it's holding on to the pending list lock, since the pending
list might be updated during a GC and "pending = r.discovered" will
than
overwrite something other than "r", silently dropping any newly
discovered References which will never be discovered by the the GC
again.
Then the code was completely broken because it was obviously capable of
allocating whilst holding the lock. There is nothing in the Java code to
indicate allocation should not happen and no way that Java code can
directly control that! We were only fixing the problem of the exception
killing the thread, not trying to address an undisclosed illegal
allocation problem!
JDK-8022321 did indeed fix a real issue. It might also have
unintentionally introduced a new one. Prior to JDK-8022321 we knew
that the ReferenceHandler couldn't provoke a GC while manipulating the
pending list, since the code was:
synchronized (lock) {
if (pending != null) {
r = pending;
pending = r.discovered;
r.discovered = null;
} else {
}
}
The manipulation of the pending list is built on some secret/ugly
rules and handshakes between the GC and the ReferenceHandler, which
only works because we control of both.
How would a GC thread update pending if the ReferenceHandlerThread holds
the lock?
The pending list lock is grabbed by the Java thread issuing the VM
operation, on behalf of the GC to allow the GC the manipulate the
pending list. If the thread issuing the VM operation is the
ReferenceHandler, then the monitor is taken recursively, which is ok
as long as ReferenceHandler isn't in the middle of unlinking an element.
On the other hand, if an OOME can never happen (i.e. no GC) here then
we're good the comment is just incorrect. The instanceof check could be
moved out of the try/catch block again, like it was prior to this
change, just to make it obvious that we will not be able to cause new
allocations inside the critical section. Or at a minimum, the comment
saying OOME can still happen should be adjusted.
I found it very difficult to determine with 100% certainty whether or
not the instanceof could ever trigger an allocation and hence
potentially an OOME.
I agree, it's not obvious.
cheers,
Per
With JVMCI it is now easier to imagine that compilation of this
