Re: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Alan Bateman]

On 11/10/2018 04:08, Igor Ignatyev wrote:

Hi Hamlin,

as ModuleTargetHelper uses non-public API, I'd prefer not to have in a common 
test library, and 8211976 suggests moving it closer to tests. could you please 
explain why you decided to put it into the library instead?

Right, ModuleTargetHelper is just a helper classes in the jlink tests, 
it's not general purpose and not suitable to put into the test 
infrastructure directory.


-Alan

Re: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Hamlin Li]

Thank you clarifying Igor.

Moving ModuleTargetHelper to local folder has a drawback: it's hard for 
future maintainer to found it if they need to use it in other places, 
that make it an "*invisible*" library class.


Although I don't fully agree with you, I have updated the webrev as you 
suggested: http://cr.openjdk.java.net/~mli/8186610/webrev.01/


Thank you

-Hamlin


On 2018/10/11 11:38 AM, Igor Ignatyev wrote:
b/c this will make test library modularization[1] somewhat 
troublesome, also I ain't sure if ModuleTargetHelper really needs to 
be placed into the top-level library regardless of its dependency on 
non-public API. "promoting" test library class to the top-level 
library comes w/ increased maintenance costs, the parent task[2] 
explains that in more details.


[1] https://bugs.openjdk.java.net/browse/JDK-8211358
[2] https://bugs.openjdk.java.net/browse/JDK-8211290

-- Igor

On Oct 10, 2018, at 8:26 PM, Hamlin Li > wrote:


Hi Igor,

Would you please clarify your concern further? I mean why 
ModuleTargetHelper can be put to top level when it use non-public 
APIs e.g. jdk.internal.module.*?


Thank you

-Hamlin

On 2018/10/11 11:08 AM, Igor Ignatyev wrote:

Hi Hamlin,

as ModuleTargetHelper uses non-public API, I'd prefer not to have in 
a common test library, and 8211976 suggests moving it closer to 
tests. could you please explain why you decided to put it into the 
library instead?


Thanks,
-- Igor

- Original Message -
From: huaming...@oracle.com 
To: core-libs-dev@openjdk.java.net 

Sent: Wednesday, October 10, 2018 7:40:34 PM GMT -08:00 US/Canada 
Pacific

Subject: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

Would you please review the following patch?

bug:

https://bugs.openjdk.java.net/browse/JDK-8186610

https://bugs.openjdk.java.net/browse/JDK-8211976

webrev: http://cr.openjdk.java.net/~mli/8186610/ 




Thank you

-Hamlin

Re: [12] RFR of JDK-8210353: Move java/util/Arrays/TimSortStackSize2.java back to tier1

[Amy Lu]

Thank you Rémi!

/Amy


On 2018/10/11 2:27 PM, Remi Forax wrote:

Looks good !

Rémi

- Mail original -

De: "Amy Lu" 
À: "core-libs-dev" 
Envoyé: Jeudi 11 Octobre 2018 08:13:44
Objet: Re: [12] RFR of JDK-8210353: Move 
java/util/Arrays/TimSortStackSize2.java back to tier1
Ping ~

It's no longer running this test in a separate job as test OOM issue
(JDK-8199265) fixed.

Thanks,
Amy

On 2018/9/4 5:46 PM, Amy Lu wrote:

test/jdk/java/util/Arrays/TimSortStackSize2.java

This test was demoted to tier2 due to JDK-8199265 (test fails with
OOM). This issue has been fixed (in jdk-11+20) and test has been run
(in tier2) without failure happening after that.

Let's move it back to tier1.

bug: https://bugs.openjdk.java.net/browse/JDK-8210353
webrev: http://cr.openjdk.java.net/~amlu/8210353/webrev.00/

Thanks,
Amy

diff --git a/test/jdk/TEST.groups b/test/jdk/TEST.groups
--- a/test/jdk/TEST.groups
+++ b/test/jdk/TEST.groups
@@ -35,8 +35,7 @@
  :jdk_lang

  tier1_part2 = \
-    :jdk_util \
-    -java/util/Arrays/TimSortStackSize2.java
+    :jdk_util

  tier1_part3 = \
  :jdk_math \
@@ -67,9 +66,7 @@
  -sun/nio/cs/ISO8859x.java \
  :jdk_other \
  :jdk_text \
-    :jdk_time \
-    java/util/Arrays/TimSortStackSize2.java
-
+    :jdk_time

  tier2_part3 = \
  :jdk_net
diff --git a/test/jdk/java/util/Arrays/TimSortStackSize2.java
b/test/jdk/java/util/Arrays/TimSortStackSize2.java
--- a/test/jdk/java/util/Arrays/TimSortStackSize2.java
+++ b/test/jdk/java/util/Arrays/TimSortStackSize2.java
@@ -25,7 +25,6 @@
   * @test
   * @bug 8072909
   * @summary Test TimSort stack size on big arrays
- * @key intermittent
   * @library /lib/testlibrary /test/lib
   * @modules java.management
   *  java.base/jdk.internal

Re: [12] RFR of JDK-8210353: Move java/util/Arrays/TimSortStackSize2.java back to tier1

[Remi Forax]

Looks good !

Rémi

- Mail original -
> De: "Amy Lu" 
> À: "core-libs-dev" 
> Envoyé: Jeudi 11 Octobre 2018 08:13:44
> Objet: Re: [12] RFR of JDK-8210353: Move 
> java/util/Arrays/TimSortStackSize2.java back to tier1

> Ping ~
> 
> It's no longer running this test in a separate job as test OOM issue
> (JDK-8199265) fixed.
> 
> Thanks,
> Amy
> 
> On 2018/9/4 5:46 PM, Amy Lu wrote:
>> test/jdk/java/util/Arrays/TimSortStackSize2.java
>>
>> This test was demoted to tier2 due to JDK-8199265 (test fails with
>> OOM). This issue has been fixed (in jdk-11+20) and test has been run
>> (in tier2) without failure happening after that.
>>
>> Let's move it back to tier1.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8210353
>> webrev: http://cr.openjdk.java.net/~amlu/8210353/webrev.00/
>>
>> Thanks,
>> Amy
>>
>> diff --git a/test/jdk/TEST.groups b/test/jdk/TEST.groups
>> --- a/test/jdk/TEST.groups
>> +++ b/test/jdk/TEST.groups
>> @@ -35,8 +35,7 @@
>>  :jdk_lang
>>
>>  tier1_part2 = \
>> -    :jdk_util \
>> -    -java/util/Arrays/TimSortStackSize2.java
>> +    :jdk_util
>>
>>  tier1_part3 = \
>>  :jdk_math \
>> @@ -67,9 +66,7 @@
>>  -sun/nio/cs/ISO8859x.java \
>>  :jdk_other \
>>  :jdk_text \
>> -    :jdk_time \
>> -    java/util/Arrays/TimSortStackSize2.java
>> -
>> +    :jdk_time
>>
>>  tier2_part3 = \
>>  :jdk_net
>> diff --git a/test/jdk/java/util/Arrays/TimSortStackSize2.java
>> b/test/jdk/java/util/Arrays/TimSortStackSize2.java
>> --- a/test/jdk/java/util/Arrays/TimSortStackSize2.java
>> +++ b/test/jdk/java/util/Arrays/TimSortStackSize2.java
>> @@ -25,7 +25,6 @@
>>   * @test
>>   * @bug 8072909
>>   * @summary Test TimSort stack size on big arrays
>> - * @key intermittent
>>   * @library /lib/testlibrary /test/lib
>>   * @modules java.management
>>   *  java.base/jdk.internal

Re: [12] RFR of JDK-8210353: Move java/util/Arrays/TimSortStackSize2.java back to tier1

[Amy Lu]

Ping ~

It's no longer running this test in a separate job as test OOM issue 
(JDK-8199265) fixed.


Thanks,
Amy

On 2018/9/4 5:46 PM, Amy Lu wrote:

test/jdk/java/util/Arrays/TimSortStackSize2.java

This test was demoted to tier2 due to JDK-8199265 (test fails with 
OOM). This issue has been fixed (in jdk-11+20) and test has been run 
(in tier2) without failure happening after that.


Let's move it back to tier1.

bug: https://bugs.openjdk.java.net/browse/JDK-8210353
webrev: http://cr.openjdk.java.net/~amlu/8210353/webrev.00/

Thanks,
Amy

diff --git a/test/jdk/TEST.groups b/test/jdk/TEST.groups
--- a/test/jdk/TEST.groups
+++ b/test/jdk/TEST.groups
@@ -35,8 +35,7 @@
 :jdk_lang

 tier1_part2 = \
-    :jdk_util \
-    -java/util/Arrays/TimSortStackSize2.java
+    :jdk_util

 tier1_part3 = \
 :jdk_math \
@@ -67,9 +66,7 @@
 -sun/nio/cs/ISO8859x.java \
 :jdk_other \
 :jdk_text \
-    :jdk_time \
-    java/util/Arrays/TimSortStackSize2.java
-
+    :jdk_time

 tier2_part3 = \
 :jdk_net
diff --git a/test/jdk/java/util/Arrays/TimSortStackSize2.java 
b/test/jdk/java/util/Arrays/TimSortStackSize2.java

--- a/test/jdk/java/util/Arrays/TimSortStackSize2.java
+++ b/test/jdk/java/util/Arrays/TimSortStackSize2.java
@@ -25,7 +25,6 @@
  * @test
  * @bug 8072909
  * @summary Test TimSort stack size on big arrays
- * @key intermittent
  * @library /lib/testlibrary /test/lib
  * @modules java.management
  *  java.base/jdk.internal

Why Stream.concat is a static method - type variable contravariance

[James Roper]

With the work I'm doing at the moment at creating a Reactive Streams
equivalent to java.util.stream, I've often wondered why Stream.concat is a
static method, rather than an instance method concating the given stream
onto this. But I think the reason has just dawned on me, and I wanted to
confirm that I'm correct.

Java doesn't support contravariant type variables - it does for type
declarations, but not type variables.

To put more concretely, if I had a Stream, and I wanted to concat
a Stream, this is a valid thing to do, the resulting stream would
be Stream. But doing that with an instance method would require
something like this:

public  Stream concat(Stream b);

Which is not supported (specifically,  type variable declaration
is not supported). In contrast, what we have in the actual API:

public static  Stream concat(Stream a, Stream b);

does allow me to concat a Stream and Stream with a
resulting type of Stream.

Is this right, or are there other reasons? Also, is there any possibility
that Java might support contravariance in type variables in future? My
reason for wanting it is to provide the following method for reactive
streams:

public  Publisher onErrorResumeWith(Function> f);

The intent of this method is when a stream encounters an error, the passed
function is invoked with the error, and that function returns a publisher
that gets concated to the current stream instead of the error being
emitted. This could possibly be implemented with a static method:

public static  Publisher onErrorResumeWith(Publisher a,
Function f);

But unlike concat, this method feels and reads much better as an instance
method, as a static method it's a little confusing.

Regards,

James

-- 
*James Roper*
*Senior Developer, Office of the CTO*

Lightbend  – Build reactive apps!
Twitter: @jroper 

Re: RFR(M) : 8211171 : move JarUtils to top-level testlibrary

[Weijun Wang]

Do we have a plan to move away from the deprecated methods? Is there a flag I 
can set to check how many classes are using them?

--Max

> On Sep 30, 2018, at 11:00 PM, Alan Bateman  wrote:
> 
> On 27/09/2018 00:38, Igor Ignatyev wrote:
>> here is the webrevs w/ JarUtils from default package inserted into 
>> jdk.test.lib.util.JarUtils:
>> whole patch: 
>> http://cr.openjdk.java.net/~iignatyev//8211171/webrev.01/index.html 
>> 
>>> 655 lines changed: 239 ins; 355 del; 61 mod;
>> incremental: 
>> http://cr.openjdk.java.net/~iignatyev//8211171/webrev.0-1/index.html 
>> 
>>> 476 lines changed: 239 ins; 203 del; 34 mod;
>> 
>> doing that, I noticed that both updateJarFile and createJarFile don't close 
>> Stream from Files::find, the current patch fixes that.
>> 
> I see you've also deprecated the String methods in the old class - good! I'd 
> probably carry over test/jdk/lib/testlibrary/JarUtils.java without changing 
> the format but your IDE must be setup differently and it will get changed 
> again by whoever next changes it so I think it's okay.
> 
> The update to tests using this look fine.
> 
> -Alan

Re: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Igor Ignatyev]

b/c this will make test library modularization[1] somewhat troublesome, also I 
ain't sure if ModuleTargetHelper really needs to be placed into the top-level 
library regardless of its dependency on non-public API. "promoting" test 
library class to the top-level library comes w/ increased maintenance costs, 
the parent task[2] explains that in more details. 

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

[2] https://bugs.openjdk.java.net/browse/JDK-8211290 


-- Igor

> On Oct 10, 2018, at 8:26 PM, Hamlin Li  wrote:
> 
> Hi Igor,
> 
> Would you please clarify your concern further? I mean why ModuleTargetHelper 
> can be put to top level when it use non-public APIs e.g. 
> jdk.internal.module.*?
> 
> Thank you
> 
> -Hamlin
> 
> On 2018/10/11 11:08 AM, Igor Ignatyev wrote:
>> Hi Hamlin,
>> 
>> as ModuleTargetHelper uses non-public API, I'd prefer not to have in a 
>> common test library, and 8211976 suggests moving it closer to tests. could 
>> you please explain why you decided to put it into the library instead?
>> 
>> Thanks,
>> -- Igor
>> 
>> - Original Message -
>> From: huaming...@oracle.com
>> To: core-libs-dev@openjdk.java.net
>> Sent: Wednesday, October 10, 2018 7:40:34 PM GMT -08:00 US/Canada Pacific
>> Subject: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary
>> 
>> Would you please review the following patch?
>> 
>> bug:
>> 
>>https://bugs.openjdk.java.net/browse/JDK-8186610
>> 
>>https://bugs.openjdk.java.net/browse/JDK-8211976
>> 
>> webrev: http://cr.openjdk.java.net/~mli/8186610/
>> 
>> 
>> Thank you
>> 
>> -Hamlin
>> 
> 

Re: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Hamlin Li]

Hi Igor,

Would you please clarify your concern further? I mean why 
ModuleTargetHelper can be put to top level when it use non-public APIs 
e.g. jdk.internal.module.*?


Thank you

-Hamlin

On 2018/10/11 11:08 AM, Igor Ignatyev wrote:

Hi Hamlin,

as ModuleTargetHelper uses non-public API, I'd prefer not to have in a common 
test library, and 8211976 suggests moving it closer to tests. could you please 
explain why you decided to put it into the library instead?

Thanks,
-- Igor

- Original Message -
From: huaming...@oracle.com
To: core-libs-dev@openjdk.java.net
Sent: Wednesday, October 10, 2018 7:40:34 PM GMT -08:00 US/Canada Pacific
Subject: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

Would you please review the following patch?

bug:

    https://bugs.openjdk.java.net/browse/JDK-8186610

    https://bugs.openjdk.java.net/browse/JDK-8211976

webrev: http://cr.openjdk.java.net/~mli/8186610/


Thank you

-Hamlin

Re: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Igor Ignatyev]

Hi Hamlin,

as ModuleTargetHelper uses non-public API, I'd prefer not to have in a common 
test library, and 8211976 suggests moving it closer to tests. could you please 
explain why you decided to put it into the library instead?

Thanks,
-- Igor

- Original Message -
From: huaming...@oracle.com
To: core-libs-dev@openjdk.java.net
Sent: Wednesday, October 10, 2018 7:40:34 PM GMT -08:00 US/Canada Pacific
Subject: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

Would you please review the following patch?

bug:

   https://bugs.openjdk.java.net/browse/JDK-8186610

   https://bugs.openjdk.java.net/browse/JDK-8211976

webrev: http://cr.openjdk.java.net/~mli/8186610/


Thank you

-Hamlin

RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

[Hamlin Li]

Would you please review the following patch?

bug:

  https://bugs.openjdk.java.net/browse/JDK-8186610

  https://bugs.openjdk.java.net/browse/JDK-8211976

webrev: http://cr.openjdk.java.net/~mli/8186610/


Thank you

-Hamlin

Re: 8152910: Get performance improvement with Stable annotation

[Joseph D. Darcy]

Looks fine Brian; thanks,

-Joe

On 10/1/2018 2:31 PM, Brian Burkhalter wrote:

Please review at your convenience:

https://bugs.openjdk.java.net/browse/JDK-8152910

JMH benchmarks do show a slight but measurable performance improvement with the 
below patch applied.

Thanks,

Brian

--- a/src/java.base/share/classes/java/math/BigInteger.java
+++ b/src/java.base/share/classes/java/math/BigInteger.java
@@ -41,6 +41,7 @@
  import jdk.internal.math.DoubleConsts;
  import jdk.internal.math.FloatConsts;
  import jdk.internal.HotSpotIntrinsicCandidate;
+import jdk.internal.vm.annotation.Stable;
  
  /**

   * Immutable arbitrary-precision integers.  All operations behave as if
@@ -1219,8 +1220,10 @@
   * Initialize static constant array when class is loaded.
   */
  private static final int MAX_CONSTANT = 16;
-private static BigInteger posConst[] = new BigInteger[MAX_CONSTANT+1];
-private static BigInteger negConst[] = new BigInteger[MAX_CONSTANT+1];
+@Stable
+private static final BigInteger posConst[] = new 
BigInteger[MAX_CONSTANT+1];
+@Stable
+private static final BigInteger negConst[] = new 
BigInteger[MAX_CONSTANT+1];

Re: generated code and jigsaw modules

[Richard Hillegas]

Thanks, Alan. This is very helpful.

On 10/10/18 9:53 AM, Alan Bateman wrote:

On 10/10/2018 16:37, Richard Hillegas wrote:

:

java.lang.invoke.MethodHandles.lookup().defineClass(generatedClassBytes)

This approach does indeed put the generated class where I want it: 
inside the Derby engine module. Unfortunately, the ClassLoader of the 
generated class is the application class loader. I can't figure out 
how to force the generated class to use the custom ClassLoader instead.
MethodHandles.lookup() creates a Lookup to the caller of the method so 
I assume you must be calling it from Derby code on the class path. If 
you want the class generated in the same runtime package as the code 
loaded from the database then you'll need to get a Lookup object to a 
class in that runtime package, perhaps with privateLookupIn.




Alan's approach is a bit more complicated. It involves following the 
pattern in 
com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl. It 
involves generating a temporary module for each generated class and 
then adding more export directives to the engine module so that the 
generated module can call back into the engine. I have to say I'm a 
little confused about the implications of slow memory leaks with this 
approach. I don't know what happens to these generated modules and 
export directives when the generated class is garbage-collected.


More immediately, however, I am up against the same problem which 
plagues Rémi's approach: how do I get the generated module to resolve 
classes in the custom ClassLoader? More specifically, I am stuck 
trying to get the generated module to require the user-written 
modules, that is, the user-written jar files. What I am missing is 
the ability to retrieve the module names of these jar files so that I 
can craft requires directives. The only way I know to get a module 
name is to use ModuleFinder.of(Path...). Unfortunately, the Path 
interface is an abstraction for file systems and is not a good fit 
for locating a blob of bytes stored inside a database.
My mail wasn't suggesting an approach, I was just pointing out an 
example of code in the JDK that creates a dynamic module to 
encapsulate generated code. It just happens that there is one class in 
that module.


As regards classes in the database then it would require developing 
your own ModuleFinder that can find modules in the database. There was 
an example on jigsaw-dev recently where someone was looking for a 
ModuleFinder to find modules in WAR files [1] which might be useful to 
get an idea on what is involved.


-Alan

[1] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2018-September/013924.html

Re: Review Request: JDK-8211921,,AssertionError in MethodHandles$Lookup.defineClass

[Mandy Chung]

On 10/10/18 1:41 PM, Alan Bateman wrote:



On 10/10/2018 21:38, Mandy Chung wrote:

:

This assertion is not strictly necessary and
test/jdk/java/lang/invoke/DefineClassTest.java verifies these
properties of the resulting Class.  I propose to remove this
assertion.
An alternative is to just drop the PD check from the assertion, the 
check that defining class loader and package should never fail. Either 
is okay with me.


If the loader or package is not the one expected, it's a serious bug and 
also the DefineClassTest test will catch it.  I opt to take this out as 
the assertion does not seem adding much value.


thanks
Mandy

Re: Review Request: JDK-8211921,,AssertionError in MethodHandles$Lookup.defineClass

[Alan Bateman]

On 10/10/2018 21:38, Mandy Chung wrote:

:

This assertion is not strictly necessary and
test/jdk/java/lang/invoke/DefineClassTest.java verifies these
properties of the resulting Class.  I propose to remove this
assertion.
An alternative is to just drop the PD check from the assertion, the 
check that defining class loader and package should never fail. Either 
is okay with me.


-Alan

Review Request: JDK-8211921,,AssertionError in MethodHandles$Lookup.defineClass

[Mandy Chung]

The assertion in Lookup::defineClass ensures that the resulting
Class is defined with the same loader in the same package in
the same protection domain as the lookup class.  When a class
is defined to the boot loader, its protection domain is set to
null which implies AllPermission but Class::getProtectionDomain
however does not guarantee to return the same PD instance.  Hence
the assertion may fail when 2+ threads are getting PD at the same
time.

This assertion is not strictly necessary and
test/jdk/java/lang/invoke/DefineClassTest.java verifies these
properties of the resulting Class.  I propose to remove this
assertion.

diff --git 
a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java

--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
@@ -969,9 +969,6 @@
 ProtectionDomain pd = (loader != null) ? 
lookupClassProtectionDomain() : null;

 String source = "__Lookup_defineClass__";
 Class clazz = 
SharedSecrets.getJavaLangAccess().defineClass(loader, cn, bytes, pd, 
source);

-    assert clazz.getClassLoader() == lookupClass.getClassLoader()
-    && 
clazz.getPackageName().equals(lookupClass.getPackageName())
-    && protectionDomain(clazz) == 
lookupClassProtectionDomain();

 return clazz;
 }

Mandy

Re: RFR (XS) 8211396 : Broken link in javadoc for private java.util.regex.Pattern#normalize()

[Xueming Shen]

+1

On 10/10/18, 10:22 AM, Ivan Gerasimov wrote:

Hello!

The javadoc refers to the enum values as 
java.text.Normalizer.Form.NFC, while it should be 
java.text.Normalizer.Form#NFC.


Also an unpaired parenthesis was removed.

Would you please help review the trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8211396
WEBREV: http://cr.openjdk.java.net/~igerasim/8211396/00/webrev/

Re: RFR (XS) 8211396 : Broken link in javadoc for private java.util.regex.Pattern#normalize()

[Jonathan Gibbons]

Looks good to me.

-- Jon


On 10/10/18 10:22 AM, Ivan Gerasimov wrote:

Hello!

The javadoc refers to the enum values as 
java.text.Normalizer.Form.NFC, while it should be 
java.text.Normalizer.Form#NFC.


Also an unpaired parenthesis was removed.

Would you please help review the trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8211396
WEBREV: http://cr.openjdk.java.net/~igerasim/8211396/00/webrev/

RFR (XS) 8211396 : Broken link in javadoc for private java.util.regex.Pattern#normalize()

[Ivan Gerasimov]

Hello!

The javadoc refers to the enum values as java.text.Normalizer.Form.NFC, 
while it should be java.text.Normalizer.Form#NFC.


Also an unpaired parenthesis was removed.

Would you please help review the trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8211396
WEBREV: http://cr.openjdk.java.net/~igerasim/8211396/00/webrev/

--
With kind regards,
Ivan Gerasimov

Re: RFR [12] - 8211960: broken links in java.util.logging

[Mandy Chung]

On 10/10/18 8:44 AM, Daniel Fuchs wrote:

Hi Mandy,

On 10/10/2018 16:32, Mandy Chung wrote:


Alternatively, this can link to java.util.Formatter class as the text 
is "format string".  It seems that Formatter class fits better with 
the SimpleFormatter class description than String.format.


Right - I was afraid of possible confusion between
java.util.logging.Formatter and java.util.Formatter, and the
spec shows a call to String.format() when explaining how
the LogRecord is formatted...

But if you still think that
{@linkplain java.util.Formatter format string}
is a better link - let me know - I'll make the change.


The javadoc of SimpleFormatter::format(LogRecord) method does describe 
the format argument is a java.util.Formatter format string.   Also the 
class description also links to this method.  So I think these two 
broken links can simply be removed and the spec about the formatting is 
in the SimpleFormatter::format method javadoc.  What do you think?


Mandy

Re: generated code and jigsaw modules

[Alan Bateman]

On 10/10/2018 16:37, Richard Hillegas wrote:

:

java.lang.invoke.MethodHandles.lookup().defineClass(generatedClassBytes)

This approach does indeed put the generated class where I want it: 
inside the Derby engine module. Unfortunately, the ClassLoader of the 
generated class is the application class loader. I can't figure out 
how to force the generated class to use the custom ClassLoader instead.
MethodHandles.lookup() creates a Lookup to the caller of the method so I 
assume you must be calling it from Derby code on the class path. If you 
want the class generated in the same runtime package as the code loaded 
from the database then you'll need to get a Lookup object to a class in 
that runtime package, perhaps with privateLookupIn.




Alan's approach is a bit more complicated. It involves following the 
pattern in com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl. 
It involves generating a temporary module for each generated class and 
then adding more export directives to the engine module so that the 
generated module can call back into the engine. I have to say I'm a 
little confused about the implications of slow memory leaks with this 
approach. I don't know what happens to these generated modules and 
export directives when the generated class is garbage-collected.


More immediately, however, I am up against the same problem which 
plagues Rémi's approach: how do I get the generated module to resolve 
classes in the custom ClassLoader? More specifically, I am stuck 
trying to get the generated module to require the user-written 
modules, that is, the user-written jar files. What I am missing is the 
ability to retrieve the module names of these jar files so that I can 
craft requires directives. The only way I know to get a module name is 
to use ModuleFinder.of(Path...). Unfortunately, the Path interface is 
an abstraction for file systems and is not a good fit for locating a 
blob of bytes stored inside a database.
My mail wasn't suggesting an approach, I was just pointing out an 
example of code in the JDK that creates a dynamic module to encapsulate 
generated code. It just happens that there is one class in that module.


As regards classes in the database then it would require developing your 
own ModuleFinder that can find modules in the database. There was an 
example on jigsaw-dev recently where someone was looking for a 
ModuleFinder to find modules in WAR files [1] which might be useful to 
get an idea on what is involved.


-Alan

[1] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2018-September/013924.html

Re: [12]RFR of JDK-8210403:Refactor java.util.Locale:i18n shell tests to plain java tests

[naoto . sato]

Looks OK to me.

Naoto

On 10/9/18 11:19 PM, Ying Zhou wrote:

Hello,

test/java/util/Locale/LocaleCategory.sh
test/java/util/Locale/LocaleProviders.sh

Please review this patch to refactor above shell script tests to java.

bug: https://bugs.openjdk.java.net/browse/JDK-8210403

webrev: http://cr.openjdk.java.net/~dzhou/8210403/webrev.00/ 



Thanks,

Daisy

Re: RFR [12] - 8211960: broken links in java.util.logging

[Daniel Fuchs]

Hi Mandy,

On 10/10/2018 16:32, Mandy Chung wrote:



On 10/10/18 3:39 AM, Daniel Fuchs wrote:

Hi,

Please find below a doc-only fix for:

8211960: broken links in java.util.logging
https://bugs.openjdk.java.net/browse/JDK-8211960

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8211960/webrev.00/index.html



Alternatively, this can link to java.util.Formatter class as the text is 
"format string".  It seems that Formatter class fits better with the 
SimpleFormatter class description than String.format.


Right - I was afraid of possible confusion between
java.util.logging.Formatter and java.util.Formatter, and the
spec shows a call to String.format() when explaining how
the LogRecord is formatted...

But if you still think that
{@linkplain java.util.Formatter format string}
is a better link - let me know - I'll make the change.

best regards,

-- daniel



Mandy

Re: generated code and jigsaw modules

[Richard Hillegas]

Thanks again to Rémi and Alan for their advice. Unfortunately, I have 
not been able to make either approach work, given another complexity of 
Derby's class loading. Let me explain that additional issue.


Derby lets users load jar files into the database. There they live as 
named blobs of bytes. The jar files contain user-defined data types, 
functions, procedures, and aggregators, which are coded in Java and can 
be used in SQL statements. Derby lets users wire these jar files into a 
custom classpath which drives a custom ClassLoader at query-execution 
time. I have not been able to make this custom ClassLoader work with 
either Rémi or Alan's approach. Note that a Derby engine manages many 
databases and each database can have its own custom ClassLoader.


I like the simplicity of Rémi's approach:

java.lang.invoke.MethodHandles.lookup().defineClass(generatedClassBytes)

This approach does indeed put the generated class where I want it: 
inside the Derby engine module. Unfortunately, the ClassLoader of the 
generated class is the application class loader. I can't figure out how 
to force the generated class to use the custom ClassLoader instead. As a 
consequence,  the generated class cannot resolve user-defined functions 
which live inside jar files in the database. Poking the customer 
ClassLoader into the thread's context class loader before calling 
MethodHandles.lookup() doesn't work.


Alan's approach is a bit more complicated. It involves following the 
pattern in com.sun.org.apache.xalan.internal.xsltc.trax.TemplatesImpl. 
It involves generating a temporary module for each generated class and 
then adding more export directives to the engine module so that the 
generated module can call back into the engine. I have to say I'm a 
little confused about the implications of slow memory leaks with this 
approach. I don't know what happens to these generated modules and 
export directives when the generated class is garbage-collected.


More immediately, however, I am up against the same problem which 
plagues Rémi's approach: how do I get the generated module to resolve 
classes in the custom ClassLoader? More specifically, I am stuck trying 
to get the generated module to require the user-written modules, that 
is, the user-written jar files. What I am missing is the ability to 
retrieve the module names of these jar files so that I can craft 
requires directives. The only way I know to get a module name is to use 
ModuleFinder.of(Path...). Unfortunately, the Path interface is an 
abstraction for file systems and is not a good fit for locating a blob 
of bytes stored inside a database.


I would appreciate any further advice about how to get over these speed 
bumps.


Thanks,
-Rick


On 10/4/18 9:10 AM, Richard Hillegas wrote:
I am looking for advice about how to tighten up module encapsulation 
while generating byte code on the fly. I ask this question on behalf 
of Apache Derby, a pure-Java relational database whose original code 
dates back to Java 1.2. I want to reduce Derby's attack-surface when 
running with a module path.


First a little context: A relational database is an interpreter for 
the SQL language. It converts SQL queries into byte code which then 
runs on a virtual machine embedded in the interpreter. In Derby's 
case, the virtual machine is the Java VM and the byte code is simply 
Java byte code. That is, a Derby query plan is a class whose byte code 
is generated on the fly at run time.


I have converted the Apache Derby codeline into a set of jigsaw 
modules: https://issues.apache.org/jira/browse/DERBY-6945. 
Unfortunately, I had to punch holes in the encapsulation of the main 
Derby module so that the generated query plans could call back into 
the Derby engine. That is because, by default, generated query plans 
load into the catch-all, unnamed module. Note that all of these 
generated classes live in a single package which does not belong to 
any named module.


1) Is it possible to load generated code into a named module?

2) Alternatively, can someone recommend another approach for 
preserving module encapsulation while generating classes on the fly?


I would appreciate any advice or examples which you can recommend.

Thanks,
-Rick

Re: RFR [12] - 8211960: broken links in java.util.logging

[Mandy Chung]

On 10/10/18 3:39 AM, Daniel Fuchs wrote:

Hi,

Please find below a doc-only fix for:

8211960: broken links in java.util.logging
https://bugs.openjdk.java.net/browse/JDK-8211960

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8211960/webrev.00/index.html



Alternatively, this can link to java.util.Formatter class as the text is 
"format string".  It seems that Formatter class fits better with the 
SimpleFormatter class description than String.format.


Mandy

Re: RFR [12] - 8211960: broken links in java.util.logging

[Chris Hegarty]

On 10/10/18 11:39, Daniel Fuchs wrote:

Hi,

Please find below a doc-only fix for:

8211960: broken links in java.util.logging
https://bugs.openjdk.java.net/browse/JDK-8211960

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8211960/webrev.00/index.html


Looks good.

-Chris.

RFR [12] - 8211960: broken links in java.util.logging

[Daniel Fuchs]

Hi,

Please find below a doc-only fix for:

8211960: broken links in java.util.logging
https://bugs.openjdk.java.net/browse/JDK-8211960

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8211960/webrev.00/index.html

The original link was intended to refer to java.util.Formatter#syntax,
which is in another module (and no longer in the parent directory).

Rather than try to navigate the class/module hierarchy to link to
that same anchor, this patch proposes to link to
String#format instead, which has all the appropriate information
and which is what the implementation is specified to use anyway.

best regards,

-- daniel

Re: RFR (Enhancement): 6194856: Zip Files lose ALL ownership and permissions of the files

[Xueming Shen]

Hi Langer,

Here are some of my comments.

ZipEntry:
Optional> getPosixPermissions()
(1) Is "Optional" really necessary here. it's a little inconsistent 
compared to the rest of

methods in the class. a "null" return might be just fine?
(2) Needs a  to separate the first line of the spec from the rest
(3) The wording probably needs more consideration. For example, it 
might be more
 straightforward to use some similar wording from other 
methods, for example


... When read from a ZIP file, this is the posix permission stored in the 
{@codeexternal
file attributes} field of the zip file entry'scentral directory record, if 
there is
one.
  Also, a @ImplNote might be the better place for "it's not available when read 
from ZIS"?


void setPosixPermissions( Set permissions)

I can see the possible use case of using "null" as a special value 
to reset the
permission field (and my proposal of simply returning a null from 
getPosixPermissions()

when there is no one, brings some symmetry here?)

ZipUtil:
(1) those POSIX_... constants don't need to be public?
(2) I like the "stream" style implementation of permsTo/FromFlags 
pair, but have some
 concerns regarding their cost. "stream" is relative expensive 
(when you take a look at
 those supporting class underneath), as these two are supposed 
to be invoked for every
 entry inside a zip file, they can be hundreds and thousands 
... just wonder, given most
 of the entries likely to have the "same" permission set inside 
a "normal" zip/jar file, is it
 worth to put in some cache mechanism here, especially for the 
"get" method, is it designed
 to return a read-only set of permission?  (in which we can use 
some mechanism here).
 That said PosixFileAttributes.getPermissions() actually 
purposely returns a modifiable set
 of permissions. It might be worth some discussion here, as the 
ZipEntry.getPosixPermission()

 needs to specify it, if it's going to return a immutable set.

Test: These days it is discouraged to check in a binary zip file as a 
test target. It is preferred

 to create the testing zip file on the fly.

-Sherman



On 9/25/18, 7:57 AM, Langer, Christoph wrote:


Hi all,

I had asked for opinions regarding adding posix permission support to 
JDK’s zip handling libraries and tools [1]. Since I didn’t get clear 
“no, you can’t do this” answers, I’m now concretely proposing some 
enhancements in the area of java.util.zip, jdk.zipfs and jdk.jartool.


I have reopened the long standing bug report (6194856) which had been 
set to “Won’t fix” quite recently for this purpose.


Here are the technical details:

The ZIP format specifications by infozip and pkware ([2] and [3]) do 
not explicitly specify the handling of posix file permissions. But it 
seems to be common sense that if file attribute compatibility is set 
to “Unix” in the upper byte of CEN field “version made by”, the file 
permissions bits are stored in CEN field “external file attributes”, 
byte 3 and 4. My changes try to honor this in the least obtrusive way. 😊


The following changes are proposed:

java.util.zip.ZipEntry:

it will have an additional field “posixPerms”. A value of -1 means “no 
permission information”, positive values will contain the flag values.


There will be 2 new public methods to read/set the permission information:

public Optional> 
getPosixPermissions()


public void 
setPosixPermissions(Set permissions)


The usage of type “Optional” reflects that posix permissions are not 
necessarily present in a zip file


java.util.zip.ZipFile:

it will have the capability to read the CEN fields and 
set posixPerms if available


java.util.zip.ZipOutputStream:

it will store entries with associated posix 
permissions as unix type in the CEN, together with the bit mask for 
the permissions


jdk.jartool:

I propose to add and option "--preserve-posix" or short "-o" to honor 
the posix bits that may be stored inside zip/jar files. By default the 
option is not set and hence posix permissions are ignored. If the flag 
is set and the file system that the jar tool is running on supports 
posix, posix file permissions that exist in the file system will be 
stored in newly created/update archives or restored to the file system 
if such information is present in the archive.


jdk.zipfs:

I added the capability for posix file permissions in 
the implementation. I decided to support PosixFileAttributes by 
subclassing ZipFileAttributes from this superclass as well as 
subclassing ZipFileAttributeView from PosixFileAttributeView. However, 
as PosixFileAttributes also include groups and owners, I would throw 
UnsupportedOperationExceptions in case of invoking methods to handle 
these attributes. But this approach seems to be most consistent with 
e.g. Files.setPosixFilePermis