Re: RFR 8199616: Fix @module declarations in tier1 tests

[Alexandre (Shura) Iline]

> On Mar 15, 2018, at 7:14 PM, Martin Buchholz  wrote:
> 
> Interesting.
> 
> Is the end goal to have jtreg itself validate the @modules tag or to execute 
> each test as its own module?  It seems like the sort of checking your tool is 
> doing belongs in jtreg itself.

That is indeed one of the options we are discussing.

Shura

> 
> 
> On Thu, Mar 15, 2018 at 5:12 PM Alexandre (Shura) Iline 
> > wrote:
> Martin, 
> 
> 
>> On Mar 15, 2018, at 4:30 PM, Martin Buchholz > > wrote:
>> 
>> How did you find the missing module dependencies?
> 
> I have a tool which runs tests one by one like this (I am simplifying):
>  - run test with -javaoptions:”—limit-modules …” with only the modules listed 
> through @modules
>  - if fails, run test with no —limit-modules
>  - if passes, that is a potential problem.
> We are thinking about making the tool open-source in code-tools project in 
> one form or another. 
> 
> Tool aside, everybody is free to use —limit-modules on a newly created test.
> 
>> Why was it only noticed now?
> 
> It was not - I have just gotten around fixing it. There are more known 
> @module deficiencies right now.
> 
>> Can we automatically prevent backsliding?
> 
> That would be a job for a continuous integration system, I imagine.
> 
> Shura
> 
> 
>> 
>> 
>> On Thu, Mar 15, 2018 at 3:40 PM Alexandre (Shura) Iline 
>> > wrote:
>> Hi,
>> 
>> Please take a quick look on fix adding missing module dependencies to tests 
>> in :tier1 jdk tests.
>> 
>> Webrev: http://cr.openjdk.java.net/~shurailine/8199616/webrev.00 
>> 
>> 
>> Shura.
>> 
> 

Re: RFR 8199616: Fix @module declarations in tier1 tests

[mandy chung]

On 3/15/18 4:35 PM, Alexandre (Shura) Iline wrote:


Actually java/lang/ProcessHandle/JavaChild.java has a dependency on 
com.sun.management.OperatingSystemMXBean.



I see.  Thanks

Mandy

Re: RFR 8199616: Fix @module declarations in tier1 tests

[Martin Buchholz]

Interesting.

Is the end goal to have jtreg itself validate the @modules tag or to
execute each test as its own module?  It seems like the sort of checking
your tool is doing belongs in jtreg itself.


On Thu, Mar 15, 2018 at 5:12 PM Alexandre (Shura) Iline <
alexandre.il...@oracle.com> wrote:

> Martin,
>
>
> On Mar 15, 2018, at 4:30 PM, Martin Buchholz  wrote:
>
> How did you find the missing module dependencies?
>
>
> I have a tool which runs tests one by one like this (I am simplifying):
>  - run test with -javaoptions:”—limit-modules …” with only the modules
> listed through @modules
>  - if fails, run test with no —limit-modules
>  - if passes, that is a potential problem.
> We are thinking about making the tool open-source in code-tools project in
> one form or another.
>
> Tool aside, everybody is free to use —limit-modules on a newly created
> test.
>
> Why was it only noticed now?
>
>
> It was not - I have just gotten around fixing it. There are more known
> @module deficiencies right now.
>
> Can we automatically prevent backsliding?
>
>
> That would be a job for a continuous integration system, I imagine.
>
> Shura
>
>
>
>
> On Thu, Mar 15, 2018 at 3:40 PM Alexandre (Shura) Iline <
> alexandre.il...@oracle.com> wrote:
>
>> Hi,
>>
>> Please take a quick look on fix adding missing module dependencies to
>> tests in :tier1 jdk tests.
>>
>> Webrev: http://cr.openjdk.java.net/~shurailine/8199616/webrev.00
>>
>> Shura.
>>
>>
>

Re: RFR 8199616: Fix @module declarations in tier1 tests

[Alexandre (Shura) Iline]

Martin, 


> On Mar 15, 2018, at 4:30 PM, Martin Buchholz  wrote:
> 
> How did you find the missing module dependencies?

I have a tool which runs tests one by one like this (I am simplifying):
 - run test with -javaoptions:”—limit-modules …” with only the modules listed 
through @modules
 - if fails, run test with no —limit-modules
 - if passes, that is a potential problem.
We are thinking about making the tool open-source in code-tools project in one 
form or another. 

Tool aside, everybody is free to use —limit-modules on a newly created test.

> Why was it only noticed now?

It was not - I have just gotten around fixing it. There are more known @module 
deficiencies right now.

> Can we automatically prevent backsliding?

That would be a job for a continuous integration system, I imagine.

Shura


> 
> 
> On Thu, Mar 15, 2018 at 3:40 PM Alexandre (Shura) Iline 
> > wrote:
> Hi,
> 
> Please take a quick look on fix adding missing module dependencies to tests 
> in :tier1 jdk tests.
> 
> Webrev: http://cr.openjdk.java.net/~shurailine/8199616/webrev.00 
> 
> 
> Shura.
> 

Re: RFR 8199616: Fix @module declarations in tier1 tests

[Alexandre (Shura) Iline]

> On Mar 15, 2018, at 4:15 PM, mandy chung  wrote:
> 
> 
> 
> On 3/15/18 3:40 PM, Alexandre (Shura) Iline wrote:
>> Hi,
>> 
>> Please take a quick look on fix adding missing module dependencies to tests 
>> in :tier1 jdk tests.
>> 
>> Webrev: 
>> http://cr.openjdk.java.net/~shurailine/8199616/webrev.00
> 
> Looks okay.   But I'm surprised that 
> test/jdk/java/lang/ProcessHandle/OnExitTest.java requires
>44  * @modules jdk.management
> 
> Looks like jdk.management get pulled by the test library. 

Actually java/lang/ProcessHandle/JavaChild.java has a dependency on 
com.sun.management.OperatingSystemMXBean.

Shura


> If so, we should file JBS issue to fix the test library to eliminate such 
> dependency.
> 
> Mandy

Re: RFR 8199616: Fix @module declarations in tier1 tests

[Martin Buchholz]

How did you find the missing module dependencies?
Why was it only noticed now?
Can we automatically prevent backsliding?


On Thu, Mar 15, 2018 at 3:40 PM Alexandre (Shura) Iline <
alexandre.il...@oracle.com> wrote:

> Hi,
>
> Please take a quick look on fix adding missing module dependencies to
> tests in :tier1 jdk tests.
>
> Webrev: http://cr.openjdk.java.net/~shurailine/8199616/webrev.00
>
> Shura.
>
>

Re: RFR 8199616: Fix @module declarations in tier1 tests

[mandy chung]

On 3/15/18 3:40 PM, Alexandre (Shura) Iline wrote:

Hi,

Please take a quick look on fix adding missing module dependencies to tests in 
:tier1 jdk tests.

Webrev: http://cr.openjdk.java.net/~shurailine/8199616/webrev.00


Looks okay.   But I'm surprised that 
test/jdk/java/lang/ProcessHandle/OnExitTest.java requires

   44  * @modules jdk.management

Looks like jdk.management get pulled by the test library.  If so, we 
should file JBS issue to fix the test library to eliminate such dependency.


Mandy

RFR 8199616: Fix @module declarations in tier1 tests

[Alexandre (Shura) Iline]

Hi,

Please take a quick look on fix adding missing module dependencies to tests in 
:tier1 jdk tests.

Webrev: http://cr.openjdk.java.net/~shurailine/8199616/webrev.00

Shura.

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

On Mar 15, 2018, at 8:56 AM, Brian Burkhalter  
wrote:

> On Mar 15, 2018, at 7:33 AM, Roger Riggs  wrote:
> 
>> BAOS.java:
>> 
>> line 163: ok but I don't think I would keep using the expanded 'byte array 
>> output stream" phrase
>> instead of the proper noun: ByteArrayOutputStream. (except for consistency)
> 
> I don’t like it either: I was just maintaining consistency (as you observed 
> the other usages).

Changed “byte array output stream” globally to {@code ByteArratOutputStream}."

>> In the Write.java test:
>> 
>> line 60:  Probably "-" in the message should be "--" for consistency
>> 
>> line 68:  Add the "e" Throwable to the thrown RuntimeException so it gets 
>> printed in a stack trace/message
>> 
>> Line 100:..  If this was a testng test, the Assert.assertEquals messages 
>> would conveniently
>> print expected and actual values.  I'd suggest converting it to use @run 
>> testng.
>> 
>> (Though I expect after these are debugged, they will never fail).
>> 
>> 73: some formatting cleanup of the old code might be useful. (spaces around 
>> "=")
> 
> Will update.


Converted to TestNG and fixed all the above. Also replaced initialization of 
the local Random variable to use RandomFactory.

Updated patch: http://cr.openjdk.java.net/~bpb/8180410/webrev.01/.

Thanks,

Brian

Re: RFR 8193033 remove terminally deprecated sun.misc.Unsafe.defineClass

[Paul Sandoz]

> On Mar 15, 2018, at 10:39 AM, Alan Bateman  wrote:
> 
> On 15/03/2018 17:06, Paul Sandoz wrote:
>> Hi,
>> 
>> Please review this patch to remove sun.misc.Unsafe.defineClass in 11.
>> 
>> There has been much outreach, by Alan and the Jigsaw team, about its public 
>> replacement MethodHandles.Lookup.defineClass.
>> 
>> CSR is here:
>> 
>>   https://bugs.openjdk.java.net/browse/JDK-8199699
>> 
> Looks good, I assume the import of java.security.ProtectionDomain can be 
> removed too.
> 

Yes, thanks,
Paul.

Re: RFR 8193033 remove terminally deprecated sun.misc.Unsafe.defineClass

[Alan Bateman]

On 15/03/2018 17:06, Paul Sandoz wrote:

Hi,

Please review this patch to remove sun.misc.Unsafe.defineClass in 11.

There has been much outreach, by Alan and the Jigsaw team, about its public 
replacement MethodHandles.Lookup.defineClass.

CSR is here:

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

Looks good, I assume the import of java.security.ProtectionDomain can be 
removed too.


-Alan

Re: RFR 8193033 remove terminally deprecated sun.misc.Unsafe.defineClass

[mandy chung]

+1

Mandy

On 3/15/18 10:06 AM, Paul Sandoz wrote:

Hi,

Please review this patch to remove sun.misc.Unsafe.defineClass in 11.

There has been much outreach, by Alan and the Jigsaw team, about its public 
replacement MethodHandles.Lookup.defineClass.

CSR is here:

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

Thanks,
Paul.


diff -r 3c0a12972165 src/jdk.unsupported/share/classes/sun/misc/Unsafe.java
--- a/src/jdk.unsupported/share/classes/sun/misc/Unsafe.javaThu Mar 15 
08:11:01 2018 -0700
+++ b/src/jdk.unsupported/share/classes/sun/misc/Unsafe.javaThu Mar 15 
09:51:00 2018 -0700
@@ -811,25 +811,6 @@
  /// random trusted operations from JNI:
  
  /**

- * Tells the VM to define a class, without security checks.  By default, 
the
- * class loader and protection domain come from the caller's class.
- *
- * @deprecated Use {@link 
java.lang.invoke.MethodHandles.Lookup#defineClass 
MethodHandles.Lookup#defineClass}
- * to define a class to the same class loader and in the same runtime 
package
- * and {@linkplain java.security.ProtectionDomain protection domain} of a
- * given {@code Lookup}'s {@linkplain 
java.lang.invoke.MethodHandles.Lookup#lookupClass() lookup class}.
- *
- * @see java.lang.invoke.MethodHandles.Lookup#defineClass(byte[])
- */
-@Deprecated(since="9", forRemoval=true)
-@ForceInline
-public Class defineClass(String name, byte[] b, int off, int len,
-ClassLoader loader,
-ProtectionDomain protectionDomain) {
-return theInternalUnsafe.defineClass(name, b, off, len, loader, 
protectionDomain);
-}
-
-/**
   * Defines a class but does not make it known to the class loader or 
system dictionary.
   * 
   * For each CP entry, the corresponding CP patch must either be null or 
have

Re: RFR 8193033 remove terminally deprecated sun.misc.Unsafe.defineClass

[Chris Hegarty]

> On 15 Mar 2018, at 17:06, Paul Sandoz  wrote:
> 
> Hi,
> 
> Please review this patch to remove sun.misc.Unsafe.defineClass in 11.
> 
> There has been much outreach, by Alan and the Jigsaw team, about its public 
> replacement MethodHandles.Lookup.defineClass.
> 
> CSR is here:
> 
>  https://bugs.openjdk.java.net/browse/JDK-8199699

Looks good Paul.   I don’t think that a CSR is strictly needed, but does no 
harm.

-Chris.

> Thanks,
> Paul.
> 
> 
> diff -r 3c0a12972165 src/jdk.unsupported/share/classes/sun/misc/Unsafe.java
> --- a/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java  Thu Mar 15 
> 08:11:01 2018 -0700
> +++ b/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java  Thu Mar 15 
> 09:51:00 2018 -0700
> @@ -811,25 +811,6 @@
> /// random trusted operations from JNI:
> 
> /**
> - * Tells the VM to define a class, without security checks.  By default, 
> the
> - * class loader and protection domain come from the caller's class.
> - *
> - * @deprecated Use {@link 
> java.lang.invoke.MethodHandles.Lookup#defineClass 
> MethodHandles.Lookup#defineClass}
> - * to define a class to the same class loader and in the same runtime 
> package
> - * and {@linkplain java.security.ProtectionDomain protection domain} of a
> - * given {@code Lookup}'s {@linkplain 
> java.lang.invoke.MethodHandles.Lookup#lookupClass() lookup class}.
> - *
> - * @see java.lang.invoke.MethodHandles.Lookup#defineClass(byte[])
> - */
> -@Deprecated(since="9", forRemoval=true)
> -@ForceInline
> -public Class defineClass(String name, byte[] b, int off, int len,
> -ClassLoader loader,
> -ProtectionDomain protectionDomain) {
> -return theInternalUnsafe.defineClass(name, b, off, len, loader, 
> protectionDomain);
> -}
> -
> -/**
>  * Defines a class but does not make it known to the class loader or 
> system dictionary.
>  * 
>  * For each CP entry, the corresponding CP patch must either be null or 
> have

RFR 8193033 remove terminally deprecated sun.misc.Unsafe.defineClass

[Paul Sandoz]

Hi,

Please review this patch to remove sun.misc.Unsafe.defineClass in 11.

There has been much outreach, by Alan and the Jigsaw team, about its public 
replacement MethodHandles.Lookup.defineClass.

CSR is here:

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

Thanks,
Paul.


diff -r 3c0a12972165 src/jdk.unsupported/share/classes/sun/misc/Unsafe.java
--- a/src/jdk.unsupported/share/classes/sun/misc/Unsafe.javaThu Mar 15 
08:11:01 2018 -0700
+++ b/src/jdk.unsupported/share/classes/sun/misc/Unsafe.javaThu Mar 15 
09:51:00 2018 -0700
@@ -811,25 +811,6 @@
 /// random trusted operations from JNI:
 
 /**
- * Tells the VM to define a class, without security checks.  By default, 
the
- * class loader and protection domain come from the caller's class.
- *
- * @deprecated Use {@link 
java.lang.invoke.MethodHandles.Lookup#defineClass 
MethodHandles.Lookup#defineClass}
- * to define a class to the same class loader and in the same runtime 
package
- * and {@linkplain java.security.ProtectionDomain protection domain} of a
- * given {@code Lookup}'s {@linkplain 
java.lang.invoke.MethodHandles.Lookup#lookupClass() lookup class}.
- *
- * @see java.lang.invoke.MethodHandles.Lookup#defineClass(byte[])
- */
-@Deprecated(since="9", forRemoval=true)
-@ForceInline
-public Class defineClass(String name, byte[] b, int off, int len,
-ClassLoader loader,
-ProtectionDomain protectionDomain) {
-return theInternalUnsafe.defineClass(name, b, off, len, loader, 
protectionDomain);
-}
-
-/**
  * Defines a class but does not make it known to the class loader or 
system dictionary.
  * 
  * For each CP entry, the corresponding CP patch must either be null or 
have

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/15/2018 05:26 PM, mandy chung wrote:
> On 3/15/18 3:58 AM, Aleksey Shipilev wrote:
>> On 03/15/2018 10:38 AM, David Holmes wrote:
>>> To be pedantic, talking about "overriding static methods" is as wrong as 
>>> talking about "inheriting
>>> static methods. They can't be inherited and so can't be overridden.
>>>
>>> In this context perhaps "intercept" would be a better choice? Or even 
>>> "proxy"?
>> All right, I made it "intercept". Submit-jdk tests returned fine.
>> I am pushing, unless there other pedantic remarks :)
> 
> "intercept" is a better choice.   Good to go.

Already in, thanks everyone!

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[mandy chung]

On 3/15/18 3:58 AM, Aleksey Shipilev wrote:

On 03/15/2018 10:38 AM, David Holmes wrote:

To be pedantic, talking about "overriding static methods" is as wrong as talking 
about "inheriting
static methods. They can't be inherited and so can't be overridden.

In this context perhaps "intercept" would be a better choice? Or even "proxy"?

All right, I made it "intercept". Submit-jdk tests returned fine.
I am pushing, unless there other pedantic remarks :)


"intercept" is a better choice.   Good to go.

Mandy

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

Hi Roger,

On Mar 15, 2018, at 7:33 AM, Roger Riggs  wrote:

> BAOS.java:
> 
> line 163: ok but I don't think I would keep using the expanded 'byte array 
> output stream" phrase
> instead of the proper noun: ByteArrayOutputStream. (except for consistency)

I don’t like it either: I was just maintaining consistency (as you observed the 
other usages).

> In the Write.java test:
> 
> line 60:  Probably "-" in the message should be "--" for consistency
> 
> line 68:  Add the "e" Throwable to the thrown RuntimeException so it gets 
> printed in a stack trace/message
> 
> Line 100:..  If this was a testng test, the Assert.assertEquals messages 
> would conveniently
> print expected and actual values.  I'd suggest converting it to use @run 
> testng.
> 
> (Though I expect after these are debugged, they will never fail).
> 
> 73: some formatting cleanup of the old code might be useful. (spaces around 
> "=")

Will update.

Thanks,

Brian

Re: Raw String Literal Library Support

[Alan Bateman]

On 13/03/2018 13:47, Jim Laskey wrote:

:

We are recommending that trimLeft and trimRight use UWS, leave trim alone to 
avoid breaking the world and then possibly introduce trimWhitespace that uses 
UWS.
Right, it would too risky to change rim() as it goes all the way back to 
JDK 1.0.


If you introduce a method named "trimWhitespace" then I think it would 
be a surprising if were not aligned with "isWhitespace". I also share 
Stuart's concerns about the handling of control characters in legacy 
trim (or TWS in the proposal). Can you expand a bit on why UWS was 
recommended?


-Alan

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Roger Riggs]

Hi Brian,

BAOS.java:

line 163: ok but I don't think I would keep using the expanded 'byte 
array output stream" phrase

instead of the proper noun: ByteArrayOutputStream. (except for consistency)

In the Write.java test:

line 60:  Probably "-" in the message should be "--" for consistency

line 68:  Add the "e" Throwable to the thrown RuntimeException so it 
gets printed in a stack trace/message


Line 100:..  If this was a testng test, the Assert.assertEquals messages 
would conveniently
print expected and actual values.  I'd suggest converting it to use @run 
testng.


(Though I expect after these are debugged, they will never fail).

73: some formatting cleanup of the old code might be useful. (spaces 
around "=")


Regards, Roger

On 3/14/2018 8:48 PM, Brian Burkhalter wrote:

https://bugs.openjdk.java.net/browse/JDK-8180410
http://cr.openjdk.java.net/~bpb/8180410/webrev.00/

This proposed patch would make the following changes:

1. Add a new method writeBytes(byte[]) which writes all supplied bytes but does 
not throw IOE.
2. Document some previously undocumented exceptions which can be thrown by 
write(byte[],int,int) and writeTo(OutputStream).
3. s/@exception/@throws/.

The test is renamed from WriteBounds to Write with the addition of a test of 
write() and writeBytes().

A CSR will be filed later.

Thanks,

Brian

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Peter Levart]

On 03/15/2018 09:57 AM, Aleksey Shipilev wrote:

On 03/15/2018 08:56 AM, Peter Levart wrote:

Hi Aleksey,

In test, the following comment:

   26  * @summary This is a test to ensure that proxies do not inherit static 
methods.

I think the word "inherit" is not correct here. Interface static methods can 
not be inherited. VM
already ensures that. Perhaps the comment should be:

     "This is a test to ensure that proxies do not try to override interface static 
methods."

I think word "inherited" there is in "proxy-inherited" sense. "Override" does 
not seems fitting
either: proxy does not override.


It does some of them:
- the default interface methods are overridden and their implementation 
wired to InvocationHandler.
- the abstract interface methods are implemented and their 
implementation wired to InvocationHandler.


Static interface methods can't be overridden. That's why I wrote that 
test checks that proxies do not "try to override" and not "override". 
But maybe "trying an impossible thing" is not the right way to express 
something. David Holmes might have better words for it.


Regards, Peter


  Maybe this:

  This is a test to ensure that proxies do not try to capture interface static 
methods.

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/15/2018 10:38 AM, David Holmes wrote:
> On 15/03/2018 5:56 PM, Peter Levart wrote:
>> Hi Aleksey,
>>
>> In test, the following comment:
>>
>>    26  * @summary This is a test to ensure that proxies do not inherit 
>> static methods.
>>
>> I think the word "inherit" is not correct here. Interface static methods can 
>> not be inherited. VM
>> already ensures that. Perhaps the comment should be:
>>
>>  "This is a test to ensure that proxies do not try to override interface 
>> static methods."
>>
>> Nothing can actually override an interface static method even if it tries to 
>> (in the sense class
>> static methods may be overridden). So I think this test checks that proxy 
>> class doesn't try to do
>> this, right?
> 
> To be pedantic, talking about "overriding static methods" is as wrong as 
> talking about "inheriting
> static methods. They can't be inherited and so can't be overridden.
> 
> In this context perhaps "intercept" would be a better choice? Or even "proxy"?

All right, I made it "intercept". Submit-jdk tests returned fine.
I am pushing, unless there other pedantic remarks :)

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[David Holmes]

On 15/03/2018 5:56 PM, Peter Levart wrote:

Hi Aleksey,

In test, the following comment:

   26  * @summary This is a test to ensure that proxies do not inherit 
static methods.


I think the word "inherit" is not correct here. Interface static methods 
can not be inherited. VM already ensures that. Perhaps the comment 
should be:


     "This is a test to ensure that proxies do not try to override 
interface static methods."


Nothing can actually override an interface static method even if it 
tries to (in the sense class static methods may be overridden). So I 
think this test checks that proxy class doesn't try to do this, right?


To be pedantic, talking about "overriding static methods" is as wrong as 
talking about "inheriting static methods. They can't be inherited and so 
can't be overridden.


In this context perhaps "intercept" would be a better choice? Or even 
"proxy"?


David


Regards, Peter

On 03/14/2018 11:04 PM, Aleksey Shipilev wrote:

On 03/14/2018 10:44 PM, mandy chung wrote:

David - I think the test fails even in your first version.

It should use ProxyClashTest.class.getClassLoader() to define the 
proxy class as the test is running

in agent vm mode.

Right. This passes local testing:
  http://cr.openjdk.java.net/~shade/8188240/webrev.02/

...and I am going to redo jdk/submit.

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/15/2018 08:56 AM, Peter Levart wrote:
> Hi Aleksey,
> 
> In test, the following comment:
> 
>   26  * @summary This is a test to ensure that proxies do not inherit static 
> methods.
> 
> I think the word "inherit" is not correct here. Interface static methods can 
> not be inherited. VM
> already ensures that. Perhaps the comment should be:
> 
>     "This is a test to ensure that proxies do not try to override interface 
> static methods."

I think word "inherited" there is in "proxy-inherited" sense. "Override" does 
not seems fitting
either: proxy does not override. Maybe this:

 This is a test to ensure that proxies do not try to capture interface static 
methods.

-Aleksey

Re: Raw String Literal Library Support

[Stephen Colebourne]

On 14 March 2018 at 23:55, Stuart Marks  wrote:
> So, how about we define trimLeft, trimRight, and trimWhitespace
> all in terms of Character.isWhitespace?

This seems like a reasonable approach. I'd expect tab to be trimmed for example.

Commons-Lang is a good source to consider when looking at naming.
https://commons.apache.org/proper/commons-lang/javadocs/api-release/org/apache/commons/lang3/StringUtils.html

Rather than re-using "trim", commons-lang uses "strip". So you have
strip(), stripLeft() and stripRight().

If you want to stick with "trim", how about trimAll() instead of
trimWhitespace(). Shorter and more obvious I think. Otherwise, I think
you'd need trimWhitespaceLeft() and trimWhitespaceRight() to match.

In line with these whitespace methods, I'd like to see isBlank()
added, could also be named isWhitespace(). The existing isEmpty()
method is fine, but a lot of the time user input validation routines
want to base their decision on "empty once trimmed". str.isBlank()
would be the same as str.trimAll().isEmpty() but without the object
creation.

Finally, a constant for EMPTY has always been missing from
java.lang.String. It would be great to add it.

Stephen

Re: GetPrimitiveArrayCritical vs GetByteArrayRegion: 140x slow-down using -Xcheck:jni and java.util.zip.DeflaterOutputStream

[Thomas Schatzl]

Hi,

On Thu, 2018-03-15 at 01:00 +, Ian Rogers wrote:
> An old data point on how large a critical region should be comes from
> java.nio.Bits. In JDK 9 the code migrated into unsafe, but in JDK 8
> the copies within a critical region were bound at most copying 1MB:
> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/
> native/java/nio/Bits.c#l88 This is inconsistent with Deflater and
> ObjectOutputStream which both allow unlimited arrays and thereby
> critical region sizes.
> 
> In JDK 9 the copies starve the garbage collector in nio Bits too as
> there is no 1MB limit to the copy sizes:
> http://hg.openjdk.java.net/jdk/jdk/rev/f70e100d3195
> which came from:
> https://bugs.openjdk.java.net/browse/JDK-8149596
> 
> Perhaps this is a regression not demonstrated due to the testing
> challenge.
> [...]
> It doesn't seem unreasonable to have the loops for the copies occur
> in 1MB chunks but JDK-8149596 lost this and so I'm confused on what
> the HotSpot stand point is.

Please file a bug (seems to be a core-libs/java.nio regression?),
preferably with some kind of regression test. Also file enhancements (I
would guess) for the other cases allowing unlimited arrays.

Long TTSP is a performance bug as any other.

> In a way criticals are better than unsafe as they may
> pin the memory and not starve GC, which shenandoah does.

(Region based) Object pinning has its own share of problems:

 - only (relatively) easily implemented in region based collectors

 - may slow down pause a bit in presence of pinned regions/objects (for
non-concurrent copying collectors)

 - excessive use of pinning may cause OOME and VM exit probably earlier
than the gc locker. GC locker seems to provide a more gradual
degradation. E.g. pinning regions typically makes these regions
unavailable for allocation.
I.e. you still should not use it for many, very long living objects.
Of course this somewhat depends on the sophistication of the
implementation.

I think region based pinning would be a good addition to other
collectors than Shenandoah too. It has been on our minds for a long
time, but there are so many other more important issues :), so of
course we are eager to see contributions in this area. ;)

If you are interested on working on this, please ping us on hotspot-gc-
dev for implementation ideas to get you jump-started.

Thanks,
  Thomas

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Peter Levart]

Hi Aleksey,

In test, the following comment:

  26  * @summary This is a test to ensure that proxies do not inherit 
static methods.


I think the word "inherit" is not correct here. Interface static methods 
can not be inherited. VM already ensures that. Perhaps the comment 
should be:


    "This is a test to ensure that proxies do not try to override 
interface static methods."


Nothing can actually override an interface static method even if it 
tries to (in the sense class static methods may be overridden). So I 
think this test checks that proxy class doesn't try to do this, right?


Regards, Peter

On 03/14/2018 11:04 PM, Aleksey Shipilev wrote:

On 03/14/2018 10:44 PM, mandy chung wrote:

David - I think the test fails even in your first version.

It should use ProxyClashTest.class.getClassLoader() to define the proxy class 
as the test is running
in agent vm mode.

Right. This passes local testing:
  http://cr.openjdk.java.net/~shade/8188240/webrev.02/

...and I am going to redo jdk/submit.

-Aleksey