Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

2019-11-08 Thread Kevin Rushforth
I disagree with a couple of these changes, but they can be fixed in a 
subsequent pass, since you've already pushed the changes.


By convention, class names are camel case with leading upper-case 
letter, so ClassName expresses that better than className. Similarly, 
MyJar.jar seems better to me than myJar.jar. By convention, package 
names don't have upper-case letters at all, so package.name or `mypkg` 
might be better than packageName.


-- Kevin

On 11/9/2019 2:50 AM, Alexey Semenyuk wrote:

Looks good.

- Alexey

On 11/8/2019 4:31 PM, Andy Herrick wrote:

revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks 
like we have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, 
inputdir, MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section of 
help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy









Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

2019-11-08 Thread Alexey Semenyuk

Looks good.

- Alexey

On 11/8/2019 4:31 PM, Andy Herrick wrote:

revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks like 
we have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, 
inputdir, MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section of 
help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy







Re: [PATCH] Enhancement proposal for usage of Method::getParameterTypes

2019-11-08 Thread Claes Redestad

Hi,

some or all of these were pointed out as part of
https://bugs.openjdk.java.net/browse/JDK-8029019

There was a patch out for review early 2017. I'm not sure what happened
to that?

Either way I think it might make sense to get this small and trivial
enhancement separated out and fixed.

Thanks!

/Claes

On 2019-11-08 23:04, Сергей Цыпанов wrote:

Hello,

it seems like Method.getParameterTypes() is often misused in JDK (and beyond): 
array returned from the method is used only to acquire number of method params 
by retrieving array.length.
The problem here is that Method.getPatameterTypes() clones underlying array and 
returns the copy.
Instead we can use Method.getParameterCount() which doesn't allocate any 
additional memory but returns directly the length of underlying  array.

To measure probable performance difference I've created a benchmark for the 
most simple case when tested method has no parameters:

@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class MethodToStringBenchmark {
   private Method method;

   @Setup
   public void setup() throws Exception { method = 
getClass().getMethod("toString"); }

   @Benchmark
   public int getParameterCount() { return method.getParameterCount(); }

   @Benchmark
   public int getParameterTypes() { return method.getParameterTypes().length; }
}

on my i7-7700 with JDK 11 it produces these results:


Benchmark   Mode  
Cnt ScoreError   Units

MethodToStringBenchmark.getParameterCount   avgt   
25 2,528 ±  0,085   ns/op
MethodToStringBenchmark.getParameterCount:·gc.alloc.rateavgt   
25≈ 10⁻⁴   MB/sec
MethodToStringBenchmark.getParameterCount:·gc.alloc.rate.norm   avgt   
25≈ 10⁻⁷ B/op
MethodToStringBenchmark.getParameterCount:·gc.count avgt   
25   ≈ 0   counts

MethodToStringBenchmark.getParameterTypes   avgt   
25 7,299 ±  0,410   ns/op
MethodToStringBenchmark.getParameterTypes:·gc.alloc.rateavgt   
25  1999,454 ± 89,929  MB/sec
MethodToStringBenchmark.getParameterTypes:·gc.alloc.rate.norm   avgt   
2516,000 ±  0,001B/op
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Eden_Space   avgt   
25  2003,360 ± 91,537  MB/sec
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Eden_Space.norm  avgt   
2516,030 ±  0,045B/op
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Old_Gen  avgt   
25 0,004 ±  0,001  MB/sec
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Old_Gen.norm avgt   
25≈ 10⁻⁵ B/op
MethodToStringBenchmark.getParameterTypes:·gc.count avgt   
25  2380,000   counts
MethodToStringBenchmark.getParameterTypes:·gc.time  avgt   
25  1325,000   ms

I've prepared a small patch to replace usage of getParameterTypes() with 
getParameterCount() where appropriate in java.base module.

The patch is attached.

Regards,
Sergey Tsypanov



Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

2019-11-08 Thread Alexander Matveev

Looks good.

On 11/8/2019 1:31 PM, Andy Herrick wrote:

revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks like 
we have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, 
inputdir, MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section of 
help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy







Re: RFR: JDK-8233636: Make jpackage an incubator and remove tool provider implementation

2019-11-08 Thread Alexander Matveev

Looks good.

On 11/8/2019 11:38 AM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [3].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This webrev (webrev.03) addresses feedback from previous version 
(webrev.02).


[1] https://bugs.openjdk.java.net/browse/JDK-8233636

[2] https://cr.openjdk.java.net/~herrick/8233636/webrev.02

[3] https://cr.openjdk.java.net/~herrick/8233636/webrev.03

/Andy




[PATCH] Enhancement proposal for usage of Method::getParameterTypes

2019-11-08 Thread Сергей Цыпанов
Hello,

it seems like Method.getParameterTypes() is often misused in JDK (and beyond): 
array returned from the method is used only to acquire number of method params 
by retrieving array.length.
The problem here is that Method.getPatameterTypes() clones underlying array and 
returns the copy.
Instead we can use Method.getParameterCount() which doesn't allocate any 
additional memory but returns directly the length of underlying  array.

To measure probable performance difference I've created a benchmark for the 
most simple case when tested method has no parameters:

@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class MethodToStringBenchmark {
  private Method method;

  @Setup
  public void setup() throws Exception { method = 
getClass().getMethod("toString"); }

  @Benchmark
  public int getParameterCount() { return method.getParameterCount(); }

  @Benchmark
  public int getParameterTypes() { return method.getParameterTypes().length; }
}

on my i7-7700 with JDK 11 it produces these results:


Benchmark   Mode  
Cnt ScoreError   Units

MethodToStringBenchmark.getParameterCount   avgt   
25 2,528 ±  0,085   ns/op
MethodToStringBenchmark.getParameterCount:·gc.alloc.rateavgt   
25≈ 10⁻⁴   MB/sec
MethodToStringBenchmark.getParameterCount:·gc.alloc.rate.norm   avgt   
25≈ 10⁻⁷ B/op
MethodToStringBenchmark.getParameterCount:·gc.count avgt   
25   ≈ 0   counts

MethodToStringBenchmark.getParameterTypes   avgt   
25 7,299 ±  0,410   ns/op
MethodToStringBenchmark.getParameterTypes:·gc.alloc.rateavgt   
25  1999,454 ± 89,929  MB/sec
MethodToStringBenchmark.getParameterTypes:·gc.alloc.rate.norm   avgt   
2516,000 ±  0,001B/op
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Eden_Space   avgt   
25  2003,360 ± 91,537  MB/sec
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Eden_Space.norm  avgt   
2516,030 ±  0,045B/op
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Old_Gen  avgt   
25 0,004 ±  0,001  MB/sec
MethodToStringBenchmark.getParameterTypes:·gc.churn.G1_Old_Gen.norm avgt   
25≈ 10⁻⁵ B/op
MethodToStringBenchmark.getParameterTypes:·gc.count avgt   
25  2380,000   counts
MethodToStringBenchmark.getParameterTypes:·gc.time  avgt   
25  1325,000   ms

I've prepared a small patch to replace usage of getParameterTypes() with 
getParameterCount() where appropriate in java.base module.

The patch is attached.

Regards,
Sergey Tsypanovdiff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java
@@ -281,13 +281,13 @@
 switch (m.getName()) {
 case "toString":
 return (m.getReturnType() == String.class
-&& m.getParameterTypes().length == 0);
+&& m.getParameterCount() == 0);
 case "hashCode":
 return (m.getReturnType() == int.class
-&& m.getParameterTypes().length == 0);
+&& m.getParameterCount() == 0);
 case "equals":
 return (m.getReturnType() == boolean.class
-&& m.getParameterTypes().length == 1
+&& m.getParameterCount() == 1
 && m.getParameterTypes()[0] == Object.class);
 }
 return false;
diff --git a/src/java.base/share/classes/java/lang/reflect/Executable.java b/src/java.base/share/classes/java/lang/reflect/Executable.java
--- a/src/java.base/share/classes/java/lang/reflect/Executable.java
+++ b/src/java.base/share/classes/java/lang/reflect/Executable.java
@@ -378,7 +378,7 @@
 private void verifyParameters(final Parameter[] parameters) {
 final int mask = Modifier.FINAL | Modifier.SYNTHETIC | Modifier.MANDATED;
 
-if (getParameterTypes().length != parameters.length)
+if (getParameterCount() != parameters.length)
 throw new MalformedParametersException("Wrong number of parameters in MethodParameters attribute");
 
 for (Parameter parameter : parameters) {
diff --git a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
--- a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
+++ b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
@@ -593,10 +593,10 @@
 Constructor noArgCtor = null;
 // public ctors only
 for (Constructor c : 

Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

2019-11-08 Thread Andy Herrick

revised [3] as per below suggestions.

[3] http://cr.openjdk.java.net/~herrick/8233591/webrev.03

/Andy


On 11/8/2019 4:07 PM, Alexey Semenyuk wrote:
This is minor and not directly related to this change, but looks like 
we have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, 
inputdir, MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section of 
help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy





Re: RFR: JDK-8233591: Reorder jpackage help text to focus on package

2019-11-08 Thread Alexey Semenyuk
This is minor and not directly related to this change, but looks like we 
have inconsistency in names in Sample usages.
Names used: modulePath, ModulePath, moduleName, className, 
moduleName/ClassName, moduleName/className, package.ClassName, inputdir, 
MyJar.jar, appRuntimeImage, name, .
I'd suggest the following changes if we are touching this section of 
help anyways:

package.ClassName -> packageName.className
MyJar.jar -> myJar.jar
inputdir -> inputDir
ModulePath -> modulePath
moduleName/ClassName -> moduleName/className
 -> appImageDir

- Alexey

On 11/8/2019 3:41 PM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy





Re: RFR: JDK-8233636: Make jpackage an incubator and remove tool provider implementation

2019-11-08 Thread Alexey Semenyuk

Looks good.

- Alexey

On 11/8/2019 2:38 PM, Andy Herrick wrote:

Please review the revised jpackage fix for bug [1] at [3].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This webrev (webrev.03) addresses feedback from previous version 
(webrev.02).


[1] https://bugs.openjdk.java.net/browse/JDK-8233636

[2] https://cr.openjdk.java.net/~herrick/8233636/webrev.02

[3] https://cr.openjdk.java.net/~herrick/8233636/webrev.03

/Andy




RFR: JDK-8233591: Reorder jpackage help text to focus on package

2019-11-08 Thread Andy Herrick

Please review the revised jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix (webrev.02) was revised after feedback from webrev.01.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] http://cr.openjdk.java.net/~herrick/8233591/webrev.02/

/Andy



Re: RFR: JDK-8233636: Make jpackage an incubator and remove tool provider implementation

2019-11-08 Thread Andy Herrick

Please review the revised jpackage fix for bug [1] at [3].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This webrev (webrev.03) addresses feedback from previous version 
(webrev.02).


[1] https://bugs.openjdk.java.net/browse/JDK-8233636

[2] https://cr.openjdk.java.net/~herrick/8233636/webrev.02

[3] https://cr.openjdk.java.net/~herrick/8233636/webrev.03

/Andy


Re: RFR [14/java.xml] 8233686: XML transformer uses excessive amount of memory

2019-11-08 Thread Joe Wang

Thanks for looking at the changeset.

A small change to eliminate unnecessary object allocations, that's the 
whole purpose, and what makes it worth the effort. As for the exact 
numbers (of how much it might save), it depends on users/customers' 
environment.


In terms of keeping in sync, we do that for formal releases.

Best,
Joe

On 11/8/19 4:33 AM, Bernd Eckenfels wrote:

This does save object allocations and churn, not memory footprint I guess. The 
namespace mapping contains multiple stacks (with object arrays) and a hashtable 
and initialized records, so it seems to allocate a few kb on every node 
visited. (But 100MB allocation does sound like a very constructed case)

BTW the thing I wondered, is there a process to keep xerces in sync?

Gruss
Bernd


--
http://bernd.eckenfels.net


Von: core-libs-dev  im Auftrag von Vyom 
Tiwari 
Gesendet: Freitag, November 8, 2019 11:14 AM
An: Joe Wang
Cc: core-libs-dev
Betreff: Re: RFR [14/java.xml] 8233686: XML transformer uses excessive amount 
of memory

Hi Joe,

Fix looks OK to me , but i am not able to understand how come
"NamespaceMappings" instance can increase memory uses from (20MB to 140MB
).

Current scope of "ns" is "case Node.ELEMENT_NODE:" block and
"NamespaceMapping" seems to be very lightweight class.

Thanks,
Vyom

On Fri, Nov 8, 2019 at 12:33 AM Joe Wang  wrote:


Please review a quick fix that reduces unnecessary object allocations.

JBS: https://bugs.openjdk.java.net/browse/JDK-8233686
webrev: http://cr.openjdk.java.net/~joehw/jdk14/8233686/webrev/

Thanks,
Joe



--
Thanks,
Vyom




Re: RFR: JDK-823359: Reorder jpackage help text to focus on package

2019-11-08 Thread Kevin Rushforth
It seems fine, but it might be even better to have both the non-modular 
and modular application examples be for the default package case and 
have a single example of --app-image. Also, I presume `--package-type` 
will be replaced with `--type` (or `-t`) to match the change in command 
line option name?


-- Kevin


On 11/8/2019 6:39 PM, Alexey Semenyuk wrote:

Looks good

On 11/8/2019 11:03 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix reorders the sample usage in the help text.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] https://cr.openjdk.java.net/~herrick/8233591

/Andy







Re: RFR: JDK-823359: Reorder jpackage help text to focus on package

2019-11-08 Thread Alexey Semenyuk

Looks good

On 11/8/2019 11:03 AM, Andy Herrick wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix reorders the sample usage in the help text.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] https://cr.openjdk.java.net/~herrick/8233591

/Andy





Re: RFR: JDK-8233117 Escape Sequences For Line Continuation and White Space (Preview)

2019-11-08 Thread Maurizio Cimadamore

Hi Jim,
the tokenizer changes looks good.

I was very confused by the dance between:

// Indicate that the final string should have escapes translated.
 shouldTranslateEscapes = true;

This:

// Conditionally translate or validate escape sequence and add result to 
string buffer.

    scanLitChar(pos, !shouldTranslateEscapes, openCount != 1);


And then this:

    private void scanLitChar(int pos, boolean translate, boolean 
multiline) {



As discussed offline, I think the confusion arises that, in the 
tokenizer, the 'shouldTranslateEscape' is some kind of a global variable 
which means "should we translate escapes AFTER reading the string 
literal", whereas the 'translate' parameter in scanLitChar means "should 
we translate escapes NOW".


The overlapping (but not identical) meaning of the two is confusing. I 
also wonder, if one is always the negation of the other, can we refer to 
the global 'shouldTranslateEscapes' from inside scanLitChar - maybe that 
will help getting rid of some of the indirection.


Thanks
Maurizio


On 08/11/2019 13:15, Jim Laskey wrote:

Updated

webrev: http://cr.openjdk.java.net/~jlaskey/8233116/webrev.01/index.html 




On Nov 7, 2019, at 5:23 PM, Brent Christian  wrote:

Should the new escapes be added to the table in the String.translateEscapes() 
JavaDoc?

-Brent

On 11/7/19 6:22 AM, Jim Laskey wrote:

Please review the following code changes. Provides for the introduction of two new escape 
sequences \ and \s.  \ allows developers to 
express unwieldy string literals in a text block as a cluster of short single line 
segments. The second is to allow developers to express ASCII space, much like \t for tab. 
The changes are primarily in the compiler, with some small changes in 
String::translateEscapes. Small changeset overall.
Thank you.
-- Jim
webrev: http://cr.openjdk.java.net/~jlaskey/8233116/webrev/index.html 

jbs: https://bugs.openjdk.java.net/browse/JDK-8233116 

csr: https://bugs.openjdk.java.net/browse/JDK-8233117 



RFR: JDK-823359: Reorder jpackage help text to focus on package

2019-11-08 Thread Andy Herrick

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


This fix reorders the sample usage in the help text.

[1] https://bugs.openjdk.java.net/browse/JDK-8233591

[2] https://cr.openjdk.java.net/~herrick/8233591

/Andy



Re: RFR: 8231863: Crash if classpath is read from @argument file and the main gets option argument

2019-11-08 Thread Mat Carter
Hi Alan

The method you propose: [nio.]Files[.write](aFile.toPath, lines) adds a 
trailing blank line to the file; the regression test needs to generate a file 
without a trailing blank line as this is the condition in which the bug occurs. 
 This is why it now writes out an array of bytes

Cheers
Mat


From: core-libs-dev  on behalf of Alan 
Bateman 
Sent: Friday, November 8, 2019 2:56 AM
To: Henry Jen ; core-libs-dev@openjdk.java.net 

Subject: Re: RFR: 8231863: Crash if classpath is read from @argument file and 
the main gets option argument

On 07/11/2019 22:55, Henry Jen wrote:
> Hi,
>
> Please review the webrev[1], contributed by Mat Carter. You can find the bug 
> details at JBS[2]. I have reviewed and tested the fix, I still need an 
> official review before I can push this.
>
Looks okay although in the test, the createAFile helper method could be
replaced with Files(aFile.toPath, lines) and that would avoid the need
to concatenate all the lines. You can specify the defaultCharset to that
method if you need really it.

-Alan


RE: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-11-08 Thread Adam Farley8
Hi Mandy,

Sorry for the delay in responding.

Mandy Chung  wrote on 29/10/2019 19:30:55:

> From: Mandy Chung 
> To: Adam Farley8 
> Cc: core-libs-dev 
> Date: 29/10/2019 19:31
> Subject: [EXTERNAL] Re: RFR: JDK-8232773: ClassLoading Debug Output 
> for Specific Classes
> 
> Hi Adam,
> 
> It'd be useful to provide a few typical scenarios that customers run 
into.
> 
> That would give better understanding on the problem "hard to diagnose 
> why a given class was not loaded" and help the consideration on 
> alternatives.   

I'm happy to provide some hypothetical example scenarios, which I've
included below. We should note that this is not limited to customers, and
that this sort of diagnostic output is also extremely useful for people
helping customers to resolve issues.

I've also included the way the webrev's debug output aids people trying
to reproduce this problem.

Scenario 1: A user gets a ClassNotFoundException because the location
of the class is not on the classpath used by the ClassLoader/s that
attempted to load it, despite the fact that one ClassLoader definitely
has the location of the class on its classpath.

Aid: This webrev's debug output tells the user, or the tech support 
aiding the user:
- If the ClassLoader with the classpath location for that class is
ever tasked with trying to load that class.
- If the ClassLoader with the classpath location for that class 
actually tries to load that class from the classpath (vs a module).
- If the ClassLoader with the classpath location actually checks that
location at all.
- What other ClassLoaders are used to attempt to load that class,
allowing the user/support agent to adjust the classpath for another 
ClassLoader if needed.

Scenario 2: A program uses the wrong version of a class.

Aid: This webrev's debug output tells the user, or the tech support 
aiding the user:
- Which classpath was used by the ClassLoader that loaded the class
at the time that the class was loaded.
- Which locations were checked by that ClassLoader before finding the
wrong version of the class. This, among other things, helps identify
access issues for the location of the correct class.
- Which ClassLoaders attempted to load the class, and in which order,
(increasingly useful in the current "nested" ClassLoading model).

Scenario 3: A program is unable to find a class because none of the
ClassLoaders could find it, and you lack both familiarity with the
ClassLoader code, and the time to figure out how all of the different
ones work.

Aid: This webrev's debug output tells the user, or the tech support 
aiding the user:
- Which ClassLoaders were involved in each attempt to load the class.
- What the logic path was for each ClassLoader asked to load the
class.
- Which module/s, if any, were identified by the ClassLoader as the 
potential owner/s of the class, and how that worked out in each
case.

> The debug output could produce lots of trace output but 
> the output does not clearly indicate the initiating class is.  

Correct. The scope of my debug output was limited to the actions
of the ClassLoader/s attempting to load a class that matches a
specific regex.

> I wonder 
> if the problem you are looking at is related to JDK-6747450 [1] or 
> really tracing the class loader delegation and search path. 

The latter. If I implied that I was interested in the initiating class,
then I was wrong. Also, though I am not fundamentally opposed to
expanding the scope to incorporate JDK-6747450, the fact that it's been
drifting for nine years tells me I'd just be adding complexity without
adding anything that would support the inclusion of the change.

Short version: I think this issue's scope should not include details
of the initiating class.

> Or maybe 
> Java Flight Recorder is a better candidate? 

I don't know much about Java Flight Recorder, as it currently only 
supports one of the VMs I work with.

>  Without knowing specific 
> of the problems customers have, perhaps VM logging can be enhanced to 
> trace the initiating class and class loader?
> 
> Mandy

It's possible. I wouldn't know where to start with that. The simplest,
and most straight-forward approach (in my mind) is to add the debug
code directly to the code we're debugging.

Plus, using a Class Library -based approach ensures that the debug
output is available regardless of the VM being used, or the user's
familiarity with Flight Recorder.

Though this might just be a "me" problem. ;)

What do you think?

Best Regards

Adam Farley



> [1] https://urldefense.proofpoint.com/v2/url?
> 
u=https-3A__bugs.openjdk.java.net_browse_JDK-2D6747450=DwIDaQ=jf_iaSHvJObTbx-
> siA1ZOg=P5m8KWUXJf-
> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=BL4kVqghO4gn5XZF6vk60QTPyLdZ5Q0i-
> YQhYV5AAXM=CRaeo9i-X27ZxNvgxD0IOJVROj6VpmLk5YINn7iJaVA= 
> 
> On 10/29/19 5:08 AM, Adam Farley8 wrote:
> > Hey All,
> >
> > To restart (and re-centre) the chat on this:
> >
> > The issue I'm trying to solve is that it's hard to determine why a 
given
> > class 

Re: RFR: JDK-8233117 Escape Sequences For Line Continuation and White Space (Preview)

2019-11-08 Thread Jim Laskey
Updated

webrev: http://cr.openjdk.java.net/~jlaskey/8233116/webrev.01/index.html 



> On Nov 7, 2019, at 5:23 PM, Brent Christian  
> wrote:
> 
> Should the new escapes be added to the table in the String.translateEscapes() 
> JavaDoc?
> 
> -Brent
> 
> On 11/7/19 6:22 AM, Jim Laskey wrote:
>> Please review the following code changes. Provides for the introduction of 
>> two new escape sequences \ and \s.  \ 
>> allows developers to express unwieldy string literals in a text block as a 
>> cluster of short single line segments. The second is to allow developers to 
>> express ASCII space, much like \t for tab. The changes are primarily in the 
>> compiler, with some small changes in String::translateEscapes. Small 
>> changeset overall.
>> Thank you.
>> -- Jim
>> webrev: http://cr.openjdk.java.net/~jlaskey/8233116/webrev/index.html 
>> 
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8233116 
>> 
>> csr: https://bugs.openjdk.java.net/browse/JDK-8233117 
>> 



Re: RFR: JDK-8233636: Make jpackage an incubator and remove tool provider implementation

2019-11-08 Thread Andy Herrick



On 11/7/2019 5:38 PM, Alexey Semenyuk wrote:


http://cr.openjdk.java.net/~herrick/8233636/webrev.02/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.sdiff.html:50 


I'd suggest to replace
---
if (name.equals("jpackage"))
---

with more robust
---
if (this == JPACKAGE)
---

http://cr.openjdk.java.net/~herrick/8233636/webrev.02/test/jdk/tools/jpackage/share/jdk/jpackage/tests/BasicTest.java.sdiff.html: 


Did you drop JPackageCommand.filterOutput() calls intentionally?


yes - this form of filter just removes the first line (where the other 
one removes a line starting with some specific strings). When we were 
printing the warning ourselves,  we would always print it.  Now that we 
are causing the system to print it by having the "jdk.incubator" package 
name prefix,  waning is printed when we invoke the tool, but not when we 
call into it's API (that warning was printed when we compiled the code 
that was linked to that package).


Anyway - these two tests began failing when we changed package names, 
because we were filtering out the first line of the output, then not 
matching.


side note - I noticed a bug in what is printed (in no arg case):


MSG_Help_no_args=Usage: jpackage  \n\
\Use jpackage --help (or -h) for a list of possible options\


there should be no more  in Usage.

(will fix this as part of JDK-8233591 
)


/Andy



The rest looks good.

- Alexey 


Re: RFR [14/java.xml] 8233686: XML transformer uses excessive amount of memory

2019-11-08 Thread Bernd Eckenfels
This does save object allocations and churn, not memory footprint I guess. The 
namespace mapping contains multiple stacks (with object arrays) and a hashtable 
and initialized records, so it seems to allocate a few kb on every node 
visited. (But 100MB allocation does sound like a very constructed case)

BTW the thing I wondered, is there a process to keep xerces in sync?

Gruss
Bernd


--
http://bernd.eckenfels.net


Von: core-libs-dev  im Auftrag von Vyom 
Tiwari 
Gesendet: Freitag, November 8, 2019 11:14 AM
An: Joe Wang
Cc: core-libs-dev
Betreff: Re: RFR [14/java.xml] 8233686: XML transformer uses excessive amount 
of memory

Hi Joe,

Fix looks OK to me , but i am not able to understand how come
"NamespaceMappings" instance can increase memory uses from (20MB to 140MB
).

Current scope of "ns" is "case Node.ELEMENT_NODE:" block and
"NamespaceMapping" seems to be very lightweight class.

Thanks,
Vyom

On Fri, Nov 8, 2019 at 12:33 AM Joe Wang  wrote:

> Please review a quick fix that reduces unnecessary object allocations.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8233686
> webrev: http://cr.openjdk.java.net/~joehw/jdk14/8233686/webrev/
>
> Thanks,
> Joe
>
>

--
Thanks,
Vyom


Re: RFR: 8231863: Crash if classpath is read from @argument file and the main gets option argument

2019-11-08 Thread Alan Bateman

On 07/11/2019 22:55, Henry Jen wrote:

Hi,

Please review the webrev[1], contributed by Mat Carter. You can find the bug 
details at JBS[2]. I have reviewed and tested the fix, I still need an official 
review before I can push this.

Looks okay although in the test, the createAFile helper method could be 
replaced with Files(aFile.toPath, lines) and that would avoid the need 
to concatenate all the lines. You can specify the defaultCharset to that 
method if you need really it.


-Alan


Re: RFR [14/java.xml] 8233686: XML transformer uses excessive amount of memory

2019-11-08 Thread Vyom Tiwari
Hi Joe,

Fix looks OK to me , but i am not able to understand how come
"NamespaceMappings"  instance can increase memory uses from (20MB to 140MB
).

Current scope of "ns"  is  "case Node.ELEMENT_NODE:"  block and
"NamespaceMapping"  seems to be very lightweight class.

Thanks,
Vyom

On Fri, Nov 8, 2019 at 12:33 AM Joe Wang  wrote:

> Please review a quick fix that reduces unnecessary object allocations.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8233686
> webrev: http://cr.openjdk.java.net/~joehw/jdk14/8233686/webrev/
>
> Thanks,
> Joe
>
>

-- 
Thanks,
Vyom