Re: RFR: Here are some URLClassPath patches

Thanks! Delivered without further changes. That should be the end of this batch of classloader changes. On Fri, Mar 2, 2018 at 7:00 AM, Roger Riggs wrote: > Hi Martin, > > I created the subtask for the release note[1], please review and update > the summary and description. > When it is done,

Re: RFR: Here are some URLClassPath patches

Hi Martin, I created the subtask for the release note[1], please review and update the summary and description. When it is done, close it as 'delivered'. Thanks, Roger [1] https://bugs.openjdk.java.net/browse/JDK-8198956 On 3/1/2018 10:32 PM, Martin Buchholz wrote: On Wed, Feb 28, 2018 at

Re: RFR: Here are some URLClassPath patches

On 2/03/2018 2:59 PM, Martin Buchholz wrote: I have a patch to move those old jdi tests out of the ProblemList jail. I thought we had an existing bug in hotspot-svc to do this, but now I can't find it. I've created a sub-task of 8198803 for this and assigned it to you. https://bugs.openjdk.ja

Re: RFR: Here are some URLClassPath patches

I have a patch to move those old jdi tests out of the ProblemList jail. I thought we had an existing bug in hotspot-svc to do this, but now I can't find it. We can just use well-formed URLs. http://cr.openjdk.java.net/~martin/webrevs/jdk/jdi-tests-malformed-urls/

Re: RFR: Here are some URLClassPath patches

On 2/03/2018 2:21 PM, Martin Buchholz wrote: Ops. 8198810 is the id of the CSR, not the real bug - I should have used 8198803. So I may have broken a jira invariant. Probably jcheck should have caught my mistake. What now? Now you have to manually update the real bug with the changeset

Re: RFR: Here are some URLClassPath patches

Ops. 8198810 is the id of the CSR, not the real bug - I should have used 8198803. So I may have broken a jira invariant. Probably jcheck should have caught my mistake. What now? changeset: 49117:0a93645a57f1 qparent user:martin date:2018-03-01 19:01 -0800 8198810: URLCla

Re: RFR: Here are some URLClassPath patches

On Wed, Feb 28, 2018 at 3:58 PM, Martin Buchholz wrote: > > > 8198810: URLClassLoader does not specify behavior when URL array contains > null > http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassLoader-NPE/ > https://bugs.openjdk.java.net/browse/JDK-8198810 > Pushed. Can someone at Oracle

Re: RFR: Here are some URLClassPath patches

OK, that was weird... All I did for testing was follow // This section should be uncommented if 8026517 is fixed. but ... 8026517 is marked fixed! So switching to ArrayDeque accidentally fixed 8026517 for real?! 8198810: URLClassLoader does not specify behavior when URL array contains null ht

Re: RFR: Here are some URLClassPath patches

On 28/02/2018 18:04, Martin Buchholz wrote: : Too bad the URL[] argument is not consistently the last one, else one could  convert them all to varargs. Varargs-friendliness seems like an independent change. Yes, completely separate issue and one that is already tracked: https://bugs.openjdk

Re: RFR: Here are some URLClassPath patches

On Wed, Feb 28, 2018 at 6:17 AM, Alan Bateman wrote: > On 28/02/2018 03:02, Martin Buchholz wrote: > > I should probably do more testing than usual when touching classloading... > > We have a jdi test that does this (hg blame finds duke as only author): > > public static ClassLoader classLoad

Re: RFR: Here are some URLClassPath patches

On 28/02/2018 03:02, Martin Buchholz wrote: I should probably do more testing than usual when touching classloading... We have a jdi test that does this (hg blame finds duke as only author):     public static ClassLoader classLoaderValue;     {         try {             urls[0] = new URL("hi th

Re: RFR: Here are some URLClassPath patches

On 2/27/18 9:03 PM, Martin Buchholz wrote: 8198808: jdi tests failing after JDK-8198484 http://cr.openjdk.java.net/~martin/webrevs/jdk/jdi-ProblemList/ https://bugs.openjdk.java.net/browse/JDK-8198808 +1 Mandy

Re: RFR: Here are some URLClassPath patches

Looks good. Thanks, David On 28/02/2018 3:03 PM, Martin Buchholz wrote: 8198808: jdi tests failing after JDK-8198484 http://cr.openjdk.java.net/~martin/webrevs/jdk/jdi-ProblemList/ https://bugs.openjdk.java.net/browse/JDK-8198808

Re: RFR: Here are some URLClassPath patches

8198808: jdi tests failing after JDK-8198484 http://cr.openjdk.java.net/~martin/webrevs/jdk/jdi-ProblemList/ https://bugs.openjdk.java.net/browse/JDK-8198808

Re: RFR: Here are some URLClassPath patches

Hi Martin, Until we figure out the best course of action, please problem list the failing test. Thanks, -Joe On 2/27/2018 7:02 PM, Martin Buchholz wrote: I should probably do more testing than usual when touching classloading... We have a jdi test that does this (hg blame finds duke as onl

Re: RFR: Here are some URLClassPath patches

Hi Martin, On 28/02/2018 1:02 PM, Martin Buchholz wrote: I should probably do more testing than usual when touching classloading... We have a jdi test that does this (hg blame finds duke as only author):     public static ClassLoader classLoaderValue;     {         try {             urls[0

Re: RFR: Here are some URLClassPath patches

I should probably do more testing than usual when touching classloading... We have a jdi test that does this (hg blame finds duke as only author): public static ClassLoader classLoaderValue; { try { urls[0] = new URL("hi there"); } catch (java.net.MalformedURLE

Re: RFR: Here are some URLClassPath patches

On Mon, Feb 26, 2018 at 3:38 PM, mandy chung wrote: > > > On 2/26/18 3:26 PM, Martin Buchholz wrote: > > > Looks okay. I also think no need to have a separate copyToArrayDeque >> method and just inline in the constructor. >> > > It's used twice. Also, it's likely to be replaced someday when we

Re: RFR: Here are some URLClassPath patches

On 2/26/18 3:26 PM, Martin Buchholz wrote: Looks okay.  I also think no need to have a separate copyToArrayDeque method and just inline in the constructor. It's used twice.  Also, it's likely to be replaced someday when we decide what to do with lambdas, so good to keep as a separa

Re: RFR: Here are some URLClassPath patches

On Mon, Feb 26, 2018 at 3:06 PM, mandy chung wrote: > > > On 2/21/18 12:30 PM, Martin Buchholz wrote: > line 63-64: ident further to the right as line 62 is the condition. > > Whitespace rejiggered. > Looks okay. I also think no need to have a separate copyToArrayDeque > method and just inline

Re: RFR: Here are some URLClassPath patches

On 2/26/18 3:10 PM, Martin Buchholz wrote: On Mon, Feb 26, 2018 at 3:06 PM, mandy chung > wrote: 8198480: Improve ClassLoaders static init block http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/

Re: RFR: Here are some URLClassPath patches

On Mon, Feb 26, 2018 at 3:06 PM, mandy chung wrote: > > 8198480: Improve ClassLoaders static init block > http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/ > https://bugs.openjdk.java.net/browse/JDK-8198480 > > > Can you rename initialModuleName to mainModule as Alan suggests (a

Re: RFR: Here are some URLClassPath patches

On 2/21/18 12:30 PM, Martin Buchholz wrote: OK, we have a reworked set of patches. (In my corner of openjdk we generally use real javadoc on private elements.) I reverted private doc comment style to the current maddening inconsistency, except I couldn't restrain myself from fixing -   

Re: RFR: Here are some URLClassPath patches

> On Feb 26, 2018, at 1:25 PM, Martin Buchholz wrote: > > > > On Mon, Feb 26, 2018 at 9:28 AM, Paul Sandoz > wrote: > > > > On Feb 23, 2018, at 2:09 PM, Martin Buchholz > > wrote: > > > > [+Paul] > > > > On Fri, Feb 23, 2018 at 6:

Re: RFR: Here are some URLClassPath patches

On Mon, Feb 26, 2018 at 9:28 AM, Paul Sandoz wrote: > > > > On Feb 23, 2018, at 2:09 PM, Martin Buchholz > wrote: > > > > [+Paul] > > > > On Fri, Feb 23, 2018 at 6:28 AM, Alan Bateman > wrote: > >> > >> 8198484: URLClassPath should use an ArrayDeque instead of a Stack > >> http://cr.openjdk.jav

Re: RFR: Here are some URLClassPath patches

> On Feb 23, 2018, at 2:09 PM, Martin Buchholz wrote: > > [+Paul] > > On Fri, Feb 23, 2018 at 6:28 AM, Alan Bateman wrote: >> >> 8198484: URLClassPath should use an ArrayDeque instead of a Stack >> http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-ArrayDeque/ >> https://bugs.openjd

Re: RFR: Here are some URLClassPath patches

[+Paul] On Fri, Feb 23, 2018 at 6:28 AM, Alan Bateman wrote: > > 8198484: URLClassPath should use an ArrayDeque instead of a Stack > http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-ArrayDeque/ > https://bugs.openjdk.java.net/browse/JDK-8198484 > > Can copyToArrayDeque use addAll? >

Re: RFR: Here are some URLClassPath patches

On Fri, Feb 23, 2018 at 6:28 AM, Alan Bateman wrote: > Just getting to the updated webrev now. > > On 21/02/2018 20:30, Martin Buchholz wrote: > > : > > > 8198480: Improve ClassLoaders static init block > http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/ > https://bugs.openjdk.j

Re: RFR: Here are some URLClassPath patches

Just getting to the updated webrev now. On 21/02/2018 20:30, Martin Buchholz wrote: : 8198480: Improve ClassLoaders static init block http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/ https://bugs.openj

Re: RFR: Here are some URLClassPath patches

On Wed, Feb 21, 2018 at 7:48 AM, Roger Riggs wrote: > > On the style changes in URLClassPath-ArrayDeque, about declarations like: > > ArrayList path = new ArrayList<>(); > > It had been a recommended style to use the abstract type (List) on the > left hand side > and only use the concrete type wh

Re: RFR: Here are some URLClassPath patches

OK, we have a reworked set of patches. (In my corner of openjdk we generally use real javadoc on private elements.) I reverted private doc comment style to the current maddening inconsistency, except I couldn't restrain myself from fixing -// ACC used when loading classes and resources */ +

Re: RFR: Here are some URLClassPath patches

2018/2/21 2:28:19 -0500, alan.bate...@oracle.com: > On 21/02/2018 06:08, Martin Buchholz wrote: >> : >> >> 8198481: Coding style cleanups for >> src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java >> http://cr.openjdk.java.net/~martin/webrevs/jdk/loader-style/ >>

Re: RFR: Here are some URLClassPath patches

Hi Martin, Generally, I would leave code alone that does not have a good reason for changing. That goes for commenting conventions (// vs /*) too. On the style changes in URLClassPath-ArrayDeque, about declarations like: ArrayList path = new ArrayList<>(); It had been a recommended style to

Re: RFR: Here are some URLClassPath patches

On 21/02/2018 06:08, Martin Buchholz wrote: : 8198485: Simplify a URLClassPath constructor http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-simplify-constructor/ https://bugs.openjdk.java.net/brows

Re: RFR: Here are some URLClassPath patches

Hi Martin, I checked this one... On 02/21/2018 07:08 AM, Martin Buchholz wrote: 8198484: URLClassPath should use an ArrayDeque instead of a Stack http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-ArrayDeque/ https://bugs.openjdk.java.net/browse/JDK-8198484 I admit I had to study the

Re: RFR: Here are some URLClassPath patches

On 21/02/2018 06:08, Martin Buchholz wrote: : 8198480: Improve ClassLoaders static init block http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/ https://bugs.openjdk.java.net/browse/JDK-8198480 This ini

Re: RFR: Here are some URLClassPath patches

On 21/02/2018 06:08, Martin Buchholz wrote: : 8198482: The URLClassPath field "urls" should be renamed to "unopenedUrls" http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-rename-urls/ https://bugs.openjdk.

Re: RFR: Here are some URLClassPath patches

On 21/02/2018 06:08, Martin Buchholz wrote: : 8198481: Coding style cleanups for src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java http://cr.openjdk.java.net/~martin/webrevs/jdk/loader-style/ https://bugs.openjdk

RFR: Here are some URLClassPath patches

At Google I spend a lot of time staring unproductively at classloader code, and it's always hard for me to resist the urge to clean it up. So here are some patches that should be relatively uncontroversial, and may prepare for more radical changes later. 8198480: Improve ClassLoaders static init