Re: RFR(m): 8140281 deprecate Optional.get()

[Martin Buchholz]

No opinion on Optional.get() itself, but here's an opinion on the high
cost of deprecation.

If you deprecate the API today, and I have a library that is already
using the API, then I should add a @SuppressWarning("deprecation")
today and keep it there until the number of people using pre-jdk9 is
insignificant, when I can finally rename it.  How long is that?  These
days, I estimate at least 10 years.  But even 20 years is quite
possible.

RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Felix Yang]

Hi there,

please review the changes to explicitly declare module dependencies 
for "java/sql/*" and "javax/*" tests;


Bug:  https://bugs.openjdk.java.net/browse/JDK-8155088

Webrev: http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.00/

Thanks,

Felix

RE: RFR: 8155600: jaxp.library.TestPolicy should extend the default security policy

[Frank Yuan]

Thank you! Pushed.

Best Regards
Frank

-Original Message-
From: Mandy Chung [mailto:mandy.ch...@oracle.com] 
Sent: Friday, April 29, 2016 4:00 AM
To: Frank Yuan 
Cc: core-libs-dev ; huizhe wang 
; Xueming Shen 
Subject: Re: RFR: 8155600: jaxp.library.TestPolicy should extend the default 
security policy

Hi Frank,

The fix looks fine.

Thanks for verifying it with my patch to deprivilege jdk.charasets.

Mandy

> On Apr 28, 2016, at 1:11 AM, Frank Yuan  wrote:
>
> Hi  Mandy, Joe and all
>
> Would you like to review the fix for bug 
> https://bugs.openjdk.java.net/browse/JDK-8155514?
>
> The webrev is at: http://cr.openjdk.java.net/~fyuan/8155514/webrev.00/.
>
> It’s verified with the source bundle in 
> http://scaaa637.us.oracle.com/archive/2016/04/2016-04-26-045641.mlchun
> g.jdk9-deprivilege
>
>
> Thanks,
>
> Frank

Re: RFR(m): 8140281 deprecate Optional.get()

[Stuart Marks]

On 4/28/16 4:05 AM, Stanislav Baiduzhyi wrote:

Those examples are amazing. Would it be possible to expand Optional
class javadoc with some of those examples? First, second, and the last
one are the most descriptive in my opinion. I frowned at Optional for
so long, you just showed me that I was not using it properly.


Yes, this is a good point, independent of deprecation of get() and addition of 
new/replacement methods such as getWhenPresent(). It does seem really hard to 
look at the API docs for Optional and to figure out how they're intended to be 
used. While the javadocs aren't the right place for a tutorial, it's certainly 
reasonable to have a few small examples in key places.


I've filed JDK-8155693 to track this.

s'marks

Re: RFR(m): 8140281 deprecate Optional.get()

[Stuart Marks]

On 4/28/16 2:26 AM, Victor Polischuk wrote:

I am sorry if my comment would be inappropriate, but is there a reason not to
introduce "Optional ifAbsent(Runnable emptyAction)" and change signature
of ifPresent to "Optional ifPresent(Consumer consumer)"?

In that case method chaining would be more natural and it will give a decent
way to have more than one simple action assigned to Optional without creating
messy java block.


Yes, this is starting to veer off topic from Optional.get().

I'll just say that a few methods have been added to Optional in Java 9: 
ifPresentOrElse(), or(), and stream(). It seems pretty complete now. Please see 
the Java 9 early access docs for details:


http://download.java.net/java/jdk9/docs/api/java/util/Optional.html

s'marks

(S) RFR: 8154710: [Solaris] Investigate use of in-memory low-resolution timestamps for Java and internal time API's

[David Holmes]

bug: https://bugs.openjdk.java.net/browse/JDK-8154710
webrev: http://cr.openjdk.java.net/~dholmes/8154710/webrev/

This change is small in nature but somewhat broad in scope. It "affects" 
the implementation of System.currentTimeMillis() in the Java space, and 
os::javaTimeMillis() in the VM. But on Solaris only.


I say "affects" but the change will be unobservable other than in terms 
of performance.


As of Solaris 11.3.6 a new in-memory timestamp has been made available 
(not unlike what has always existed on Windows). There are actually 3 
different timestamps exported but the one we are interested in is 
get_nsecs_fromepoch - which is of course elapsed nanoseconds since the 
epoch - which is exactly what javaTimeMillis() is, but expressed in 
milliseconds. The in-memory timestamps have an update accuracy of 1ms, 
so are not suitable for any other API's that want the time-of-day, but 
at a greater accuracy.


Microbenchmark shows the in-memory access is approx 45% faster (19ns on 
my test system) compared to the gettimeofday call (35ns).


Thanks,
David

Re: JDK 9 pre-review of JDK-6850612: Deprecate Class.newInstance since it violates the checked exception language contract

[Peter Levart]

On 04/28/2016 09:06 PM, Jason Mehrens wrote:

Hi Peter,


As mentioned, Class.newInstance() has a special cache for constructor
and caller that speeds up repeated invocations from the same caller by
skipping access checks.

I'm sure I'm missing something obvious related to performance or security but, 
couldn't the exact same 'cachedConstructor' field be used for caching  
Class.getConstructor(new Class[0]) as long as a constructor copy is returned?
That way the recommended workaround is near the same performance.


This would only speed up the lookup part (optimization for looking up 
no-arg constructor - perhaps this could be a special field in 
Class.ReflectionData), but returning a copy would force re-evaluation of 
access checks which can now be skipped if the caller remains the same in 
consecutive invocations to Class.newInstance().


Regards, Peter

RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Steve Drach]

I’ve updated the webrev to change all instances of the word “reified” to “real” 
as in getRealName().

Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 


Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev.01/ 

 

Re: RFR (XS) 8155090: String concatenation fails with a custom SecurityManager that uses concatenation

[Claes Redestad]

Hi Aleksey,

On 2016-04-28 22:10, Aleksey Shipilev wrote:

Hi,

Please review the fix for a shady bootstrapping issue, when a custom
SecurityManager is using string concatenation:
   https://bugs.openjdk.java.net/browse/JDK-8155090

The essence of the issue is that during StringConcatFactory::,
we are reading the system properties via the privileged calls. When
user SecurityManager that uses string concatenation is set, we are
trying to produce a string concatenation stub in order to proceed, and
double-back on SCF. There, we try to run SCF methods without fully
complete : the existing test fails with uninitialized static
final Strategy field.

The cleanest (yet subtle) solution here is to make sure the default SCF
settings are good to run with, which allows transient 
operations to complete normally:
   http://cr.openjdk.java.net/~shade/8155090/webrev.00/


looks good to me!

While a subtle fix indeed, the comment well explains the need
for doing this, and alternatives like ensuring there are no calls
back into the SecurityManager from SCF would be very fragile
in comparison.

Nits: the the -> the, (onto -> into?) no need for a re-review if
you choose to fix these.

Thanks!

/Claes



Testing: offending test; java/lang/String jtregs

Thanks,
-Aleksey

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Paul Sandoz]

> On 28 Apr 2016, at 12:20, Steve Drach  wrote:
> 
> 
>> On Apr 28, 2016, at 12:03 PM, Alan Bateman  wrote:
>> 
>> 
>> 
>> On 28/04/2016 19:53, Steve Drach wrote:
>>> :
>>> Yes, and for regular jar files, that worked fine, but when we tried it with 
>>> a multi-release jar we found it by passed the part of JarLoader where we 
>>> open the jar file as a runtime jar, so, for example, this code fails to 
>>> retrieve the correct versioned entry, returning instead the base entry.
>>> 
>>> URL[] urls = { new URL(“jar:file:/foo/multi-release.jar!/“) };
>>> URLClassLoader cldr = new URLClassLoader(urls);
>>> Class vcls = cldr.loadClass("version.Version”);
>>> 
>>> The change just corrects the logic when working with a “jar:…..!/“ URL.
>>> 
>>> 
>> Can you double check the URLClassLoader spec?
> 
> We discussed it.  It seems the spec might be deficit with respect to 
> "jar:file:/foo/multi-release.jar!/“ type URLs, but it didn’t matter when it 
> referred to a legacy jar file.
> 

It appears to be an undocumented “feature" that URLClassLoader can accept base 
jar-scheme URLs such as:

  jar:file/….!/
  jar:http/….!/

by virtue of those URLs being passed to URLClassPath, which is contrary to what 
is stated on URLClassLoader:

"Any URL that ends with a '/' is assumed to refer to a directory. Otherwise, 
the URL is assumed to refer to a JAR file which will be opened as needed.”

The above only reliably applies to file-scheme URLs.

Here is the original logic:

String file = url.getFile();
if (file != null && file.endsWith("/")) {
if ("file".equals(url.getProtocol())) {
return new FileLoader(url);  <—— exploded class loading
} else {
return new Loader(url); <—— uses URL connection, can process the jar 
file referenced by jar:file:/…!/
}
} else {
return new JarLoader(url, jarHandler, lmap);  <—— fall back, always the URL 
references a jar file
}

Paul.

Re: RFR (XS) 8155215: java.lang.String concatenation spec is unnecessarily strong

[Aleksey Shipilev]

On 04/27/2016 01:33 PM, Aleksey Shipilev wrote:
> Please review this tiny spec improvement that syncs up JLS and
> java.lang.String Javadoc. java.lang.String Javadoc unnecessarily locks
> down the implementation details for String concat (e.g. usage of
> StringBuilder):
>   http://cr.openjdk.java.net/~shade/8155215/webrev.00/
> 
> I'll submit CCC as soon as we agree on wording.

Thanks for on-list and off-list reviews! Under Alex Buckley's guidance,
we have arrived to this version, moving more to @implNote:
  http://cr.openjdk.java.net/~shade/8155215/webrev.02/

Cheers,
-Aleksey

RFR (XS) 8155090: String concatenation fails with a custom SecurityManager that uses concatenation

[Aleksey Shipilev]

Hi,

Please review the fix for a shady bootstrapping issue, when a custom
SecurityManager is using string concatenation:
  https://bugs.openjdk.java.net/browse/JDK-8155090

The essence of the issue is that during StringConcatFactory::,
we are reading the system properties via the privileged calls. When
user SecurityManager that uses string concatenation is set, we are
trying to produce a string concatenation stub in order to proceed, and
double-back on SCF. There, we try to run SCF methods without fully
complete : the existing test fails with uninitialized static
final Strategy field.

The cleanest (yet subtle) solution here is to make sure the default SCF
settings are good to run with, which allows transient 
operations to complete normally:
  http://cr.openjdk.java.net/~shade/8155090/webrev.00/

Testing: offending test; java/lang/String jtregs

Thanks,
-Aleksey

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

[nadeesh tv]

Hi all,
Thanks Stephen for the comments.
Please see the updated webrev 
http://cr.openjdk.java.net/~ntv/8148949/webrev.02/


Regards,
Nadeesh


On 4/28/2016 7:58 PM, Stephen Colebourne wrote:

I'd like to see the test cases in test_secondsPattern() check the
result of the parse (by passing more arguments from
data_secondsPattern)

Otherwise looks good.
Stephen

On 28 April 2016 at 14:12, nadeesh tv  wrote:

Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8148949/webrev.01/

Regards,
Nadeesh TV

On 4/25/2016 8:08 PM, nadeesh tv wrote:

HI all,
Please  review a fix for
Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949

Issue - Pattern letters 'A'  does not match the intent of LDML/CLDR

Solution -  Changed the definition of pattern letters 'A','n','N'

Webrev -  http://cr.openjdk.java.net/~ntv/8148949/webrev.00/



--
Thanks and Regards,
Nadeesh TV



--
Thanks and Regards,
Nadeesh TV

Re: RFR: 8155600: jaxp.library.TestPolicy should extend the default security policy

[Mandy Chung]

Hi Frank,

The fix looks fine.

Thanks for verifying it with my patch to deprivilege jdk.charasets.  

Mandy

> On Apr 28, 2016, at 1:11 AM, Frank Yuan  wrote:
> 
> Hi  Mandy, Joe and all
>  
> Would you like to review the fix for bug 
> https://bugs.openjdk.java.net/browse/JDK-8155514?
>  
> The webrev is at: http://cr.openjdk.java.net/~fyuan/8155514/webrev.00/.
>  
> It’s verified with the source bundle in 
> http://scaaa637.us.oracle.com/archive/2016/04/2016-04-26-045641.mlchung.jdk9-deprivilege
>  
>  
> Thanks,
>  
> Frank

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

[Mandy Chung]

> On Apr 28, 2016, at 11:37 AM, Brent Christian  
> wrote:
> 
> Hi, Mandy
> 
> It looks good to me.  A few minor things:
> 
> StackFrameInfo.java:
> 
> Should getByteCodeIndex() be final ?
> 

It’s a package-private class and so no subclass outside this package anyway.  
So it doesn’t really matter. I didn’t catch the inconsistency there - some 
methods in this class are final and some are not. I may clean them up.


> 
> StackWalker.java, line 136:
> * change ';' to ‘,'
> 

ok.

> 
> JavaLangInvokeAccess.java, line 40:
> 
> For the isNative() docs, I would prefer "Returns true if..." to "Tests if..."
> 
> 

Both conventions are used.  I can change that.

> Some test code for getByteCodeIndex() would be good - sounds like you plan to 
> add that.

yes.  Will send out for review next.

thanks for the review.
Mandy

> 
> Thanks,
> -Brent
> On 04/27/2016 11:31 AM, Mandy Chung wrote:
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.01/index.html
>> 
>> I added a new StackFrame::getByteCodeIndex method to return bci and 
>> updatedgetFileName and getLineNumber to have the same returned type as in 
>> StackTraceElement.  From usage perspective, the caller has to prepare and 
>> handle the information is unavailable and I think it would typically log 
>> some token to represent the unavailable case and might well use null and 
>> negative value. This patch would save the creation of short-lived Optional 
>> object that might help logging filename/linenumber case.
>> 
>> I’m working on a new test to verify bci and line number to be included in 
>> the next revision.
>> 
>> Mandy
>> 
>>> On Apr 11, 2016, at 2:22 PM, Mandy Chung  wrote:
>>> 
>>> Webrev at:
>>>   
>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.00/index.html
>>> 
>>> StackFrame::getFileName and StackFrame::getLineNumber are originally 
>>> proposed with the view of any stack walking code can migrate to the 
>>> StackWalker API without the use of StackTraceElement.
>>> 
>>> File name and line number are useful for debugging and troubleshooting 
>>> purpose. It has additional overhead to map from a method and BCI to look up 
>>> the file name and line number.
>>> 
>>> StackFrame::toStackTraceElement method returns StackTraceElement that 
>>> includes the file name and line number. There is no particular benefit to 
>>> duplicate getFileName and getLineNumber methods in StackFrame. It is 
>>> equivalently convenient to call 
>>> StackFrame.toStackTraceElement().getFileName() (or getLineNumber).
>>> 
>>> This patch proposes to remove StackFrame::getFileName and 
>>> StackFrame::getLineNumber methods since such information can be obtained 
>>> from StackFrame.toStackTraceElement().
>>> 
>>> Mandy
>> 
> 

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Steve Drach]

> On Apr 28, 2016, at 12:03 PM, Alan Bateman  wrote:
> 
> 
> 
> On 28/04/2016 19:53, Steve Drach wrote:
>> :
>> Yes, and for regular jar files, that worked fine, but when we tried it with 
>> a multi-release jar we found it by passed the part of JarLoader where we 
>> open the jar file as a runtime jar, so, for example, this code fails to 
>> retrieve the correct versioned entry, returning instead the base entry.
>> 
>> URL[] urls = { new URL(“jar:file:/foo/multi-release.jar!/“) };
>> URLClassLoader cldr = new URLClassLoader(urls);
>> Class vcls = cldr.loadClass("version.Version”);
>> 
>> The change just corrects the logic when working with a “jar:…..!/“ URL.
>> 
>> 
> Can you double check the URLClassLoader spec?

We discussed it.  It seems the spec might be deficit with respect to 
"jar:file:/foo/multi-release.jar!/“ type URLs, but it didn’t matter when it 
referred to a legacy jar file.

> 
> Also I assume the URL you are want here is "file:/foo/multi-release.jar”

No, that one is correct and still works as it did before.

> as "jar:file:/foo/multi-release.jar!/" is the URL to the top-level directory 
> in the JAR file.

The spec for JarURLconnections says

"If the entry name is omitted, the URL refers to the whole JAR file: 
jar:http://www.foo.com/bar/baz.jar!/“ 


that’s the way we are using it, as the location of a jar file.

I’ve ran this through jprt with test core and found no errors/failures with 
this change.  

> 
> -Alan

Re: JDK 9 pre-review of JDK-6850612: Deprecate Class.newInstance since it violates the checked exception language contract

[Jason Mehrens]

Hi Peter,

>As mentioned, Class.newInstance() has a special cache for constructor
>and caller that speeds up repeated invocations from the same caller by
>skipping access checks.

I'm sure I'm missing something obvious related to performance or security but, 
couldn't the exact same 'cachedConstructor' field be used for caching  
Class.getConstructor(new Class[0]) as long as a constructor copy is returned?
That way the recommended workaround is near the same performance.

Jason

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Steve Drach]

>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 
>>> 
>>> 
>>> Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ 
>>> 
>>> 
>>> This changeset causes the URL returned from a ClassLoader.getResource(name) 
>>> invocation to be reified, that is the URL is a direct pointer to either a 
>>> versioned or unversioned entry in the jar file. The patch also assures that 
>>> jar URL’s are always processed by the URLClassPath.JarLoader.  The 
>>> MultiReleaseJarURLConnection test was enhanced to demonstrate that reified 
>>> URLs are returned.  The SimpleHttpServer test helper class was moved into 
>>> it’s own file.
>> I was happy to see John's note on diction so I assume the method will be 
>> renamed. Have you considered making it public so that tools and libraries 
>> outside of the JDK can use this?
> 
> It sounds reasonable for this to be a public API,

I opened JDK-8155657 to track that.

> and if so, does it make sense to
> move it to JarEntry ( rather than JarFile )? So it would be 
> JarEntry::getTrueName,
> or similar.

The part of JarFile that knows all about versioning is a subclass of JarEntry, 
JarFileEntry.  JarEntry knows nothing about  versioning, nor about 
JarFileEntry.  Only JarFile knows about JarFileEntry.

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Alan Bateman]

On 28/04/2016 19:53, Steve Drach wrote:

:
Yes, and for regular jar files, that worked fine, but when we tried it with a 
multi-release jar we found it by passed the part of JarLoader where we open the 
jar file as a runtime jar, so, for example, this code fails to retrieve the 
correct versioned entry, returning instead the base entry.

URL[] urls = { new URL(“jar:file:/foo/multi-release.jar!/“) };
URLClassLoader cldr = new URLClassLoader(urls);
Class vcls = cldr.loadClass("version.Version”);

The change just corrects the logic when working with a “jar:…..!/“ URL.



Can you double check the URLClassLoader spec?

Also I assume the URL you are want here is "file:/foo/multi-release.jar" 
as "jar:file:/foo/multi-release.jar!/" is the URL to the top-level 
directory in the JAR file.


-Alan

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Steve Drach]

>> Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 
>> 
>> 
>> Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ 
>> 
>> 
>> This changeset causes the URL returned from a ClassLoader.getResource(name) 
>> invocation to be reified, that is the URL is a direct pointer to either a 
>> versioned or unversioned entry in the jar file. The patch also assures that 
>> jar URL’s are always processed by the URLClassPath.JarLoader.  The 
>> MultiReleaseJarURLConnection test was enhanced to demonstrate that reified 
>> URLs are returned.  The SimpleHttpServer test helper class was moved into 
>> it’s own file.
> I was happy to see John's note on diction so I assume the method will be 
> renamed. Have you considered making it public so that tools and libraries 
> outside of the JDK can use this?

I opened an issue to track this — JDK-8155657.  We’d like to get this 
changeset, including the name change for getReifieidName, integrated as soon as 
we can.

> 
> One question on URLClassPath getLoader as it's not immediately obvious that 
> this is needed. URLClassLoader specifies that any URL ending with "/" is 
> assumed to be a directory.

Yes, and for regular jar files, that worked fine, but when we tried it with a 
multi-release jar we found it by passed the part of JarLoader where we open the 
jar file as a runtime jar, so, for example, this code fails to retrieve the 
correct versioned entry, returning instead the base entry.

URL[] urls = { new URL(“jar:file:/foo/multi-release.jar!/“) };
URLClassLoader cldr = new URLClassLoader(urls);
Class vcls = cldr.loadClass("version.Version”);

The change just corrects the logic when working with a “jar:…..!/“ URL.


> Are you planning to update java.net.JarURLConnection as that has special 
> handling for #runtime that will need to be replaced.

No, we are not.  Appending the #runtime fragment in URLClassPath is the way we 
communicate to URLJarFile that the jar should be opened as a runtime versioned 
JarFile.  All this changeset does is assure that the internal implementation 
details are not leaked to the outside world.

> 
> -Alan

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

[Brent Christian]

Hi, Mandy

It looks good to me.  A few minor things:

StackFrameInfo.java:

Should getByteCodeIndex() be final ?


StackWalker.java, line 136:
* change ';' to ','


JavaLangInvokeAccess.java, line 40:

For the isNative() docs, I would prefer "Returns true if..." to "Tests 
if..."



Some test code for getByteCodeIndex() would be good - sounds like you 
plan to add that.


Thanks,
-Brent
On 04/27/2016 11:31 AM, Mandy Chung wrote:

Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.01/index.html

I added a new StackFrame::getByteCodeIndex method to return bci and 
updatedgetFileName and getLineNumber to have the same returned type as in 
StackTraceElement.  From usage perspective, the caller has to prepare and 
handle the information is unavailable and I think it would typically log some 
token to represent the unavailable case and might well use null and negative 
value. This patch would save the creation of short-lived Optional object that 
might help logging filename/linenumber case.

I’m working on a new test to verify bci and line number to be included in the 
next revision.

Mandy


On Apr 11, 2016, at 2:22 PM, Mandy Chung  wrote:

Webrev at:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.00/index.html

StackFrame::getFileName and StackFrame::getLineNumber are originally proposed 
with the view of any stack walking code can migrate to the StackWalker API 
without the use of StackTraceElement.

File name and line number are useful for debugging and troubleshooting purpose. 
It has additional overhead to map from a method and BCI to look up the file 
name and line number.

StackFrame::toStackTraceElement method returns StackTraceElement that includes 
the file name and line number. There is no particular benefit to duplicate 
getFileName and getLineNumber methods in StackFrame. It is equivalently 
convenient to call StackFrame.toStackTraceElement().getFileName() (or 
getLineNumber).

This patch proposes to remove StackFrame::getFileName and 
StackFrame::getLineNumber methods since such information can be obtained from 
StackFrame.toStackTraceElement().

Mandy

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Steve Drach]

Keeping with the path precedent, is it acceptable to change “getReifiedName” to 
“getRealName”?  

>>> 
>>> Diction Note: Reified X means X wasn't real (in some sense) until now. As 
>>> in non-reified types, which are not real at runtime because the static 
>>> compiler discarded them.
>>> 
>> 
>> I suggested reified because i thought it fit with the notion of making 
>> something abstract more concrete, but perhaps this just confuses matters?
> 
> It's the real name, but since it already exists (because that's how it is 
> stored) it isn't really reified, it's just revealed.
> 
> This API uses the term "real name" for an almost identical phenomenon (target 
> of a sym-link in a file system):
> 
> https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#toRealPath-java.nio.file.LinkOption...-
> 
> In a versioned jar, the logical names are sometimes mapped to other names 
> under which the elements are actually stored.
> Thus, they behave like sym-links.  (But with a bit of control context thrown 
> in, the active version.)
> 
> On old-fashioned file systems with real version numbers, the Common Lisp 
> function TRUENAME does exactly what you are trying to do here.
> 
> http://www.mathcs.duq.edu/simon/Gcl/gcl_1141.html
> 
> (And in some way, we are sliding down the slope toward re-inventing those 
> file systems, aren't we?)
> 
> The older pre-nio API for File calls it "getCanonicalPath", but I think "true 
> name" is better than
> "canonical name", since "canonical" means "follows the rules", rather than 
> what we need in this case,
> which is "where it really is stored".
> 
> http://docs.oracle.com/javase/8/docs/api/java/io/File.html#getCanonicalPath--
> 
>> 
>>> In this case it appears you are simply exposing a translated name, not 
>>> making it real for the first time.
>>> 
>>> If this is true, I think you want to say "true name" or "real name" or 
>>> "translated name", not "reified name”.
>> 
>> or “versioned name" would work for me.
> 
> I'm just whinging about the term "reified" which doesn't seem to work, 
> logically.
> 
> "Versioned name" would work for me too.  But "true name" has the two good 
> precedents cited above.
> 
> — John

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

[nadeesh tv]

Hi all,

Please see the updated webrev 
http://cr.openjdk.java.net/~ntv/8079628/webrev.02/


Thanks and Regards,
Nadeesh TV

On 4/28/2016 8:17 PM, Stephen Colebourne wrote:

My mistake on the spec for "DD". It should be SignStyle.NOT_NEGATIVE,
not NORMAL.

I'd like to see a test that checks adjacent value parsing works
correctly for "DDD". ie. DDDHHmm should be able to parse into a
LocalDateTime where the date-time value can be checked against the
input string.

I think this will be a good fix once complete.
thanks
Stephen

On 28 April 2016 at 14:10, nadeesh tv  wrote:

Hi all,

Thanks Stephen for the comments.

Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8079628/webrev.01/

Regards,
Nadeesh TV

On 4/26/2016 5:42 PM, Stephen Colebourne wrote:

This change introduces inconsistencies and problems. For example, it
will parse up to 19 digits for "D" but only up to 2 digits for "d".
The following would be better:

   *D   1  appendValue(ChronoField.DAY_OF_YEAR)
   *DD  2  appendValue(ChronoField.DAY_OF_YEAR, 2, 3,
SignStyle.NORMAL)
   *DDD 3  appendValue(ChronoField.DAY_OF_YEAR, 3)

This limits the accepted input to 3 digits, which is quite sufficient
here. It is also clearer for the common "D" and "DDD" cases.

Note that a test case needs to cover adjacent value parsing for
day-of-year. This pattern "DDD" should be tested and work. With
the webrev changes, it will not work as adjacent value parsing mode
will not be triggered.

(The change will alter the meaning of "DD" but no-one should be
using that anyway as it is broken, only working for the first 99 days
of the year.)

The code in TCKDateTimeFormatterBuilder needs a blank line after the
new methods.

Stephen


On 26 April 2016 at 12:48, nadeesh tv  wrote:

Hi all,

Please  review a fix for

Bug ID -   https://bugs.openjdk.java.net/browse/JDK-8079628

Issue -  Pattern 'D' is not implemented as intended by CLDR

Solution - Changed the definition of 'D' to D..D 1..3
appendValue(ChronoField.DAY_OF_YEAR, n, 19, SignStyle.NORMAL)

Webrev -  http://cr.openjdk.java.net/~ntv/8079628/webrev.00/

--
Thanks and Regards,
Nadeesh TV


--
Thanks and Regards,
Nadeesh TV



--
Thanks and Regards,
Nadeesh TV

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Paul Sandoz]

> On 27 Apr 2016, at 17:10, John Rose  wrote:
> 
> On Apr 27, 2016, at 4:20 PM, Paul Sandoz  wrote:
>> 
>> 
>>> On 27 Apr 2016, at 15:26, John Rose  wrote:
>>> 
>>> Diction Note: Reified X means X wasn't real (in some sense) until now. As 
>>> in non-reified types, which are not real at runtime because the static 
>>> compiler discarded them.
>>> 
>> 
>> I suggested reified because i thought it fit with the notion of making 
>> something abstract more concrete, but perhaps this just confuses matters?
> 
> It's the real name, but since it already exists (because that's how it is 
> stored) it isn't really reified, it's just revealed.
> 
> This API uses the term "real name" for an almost identical phenomenon (target 
> of a sym-link in a file system):
> 
> https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#toRealPath-java.nio.file.LinkOption...-
> 
> In a versioned jar, the logical names are sometimes mapped to other names 
> under which the elements are actually stored.
> Thus, they behave like sym-links.  (But with a bit of control context thrown 
> in, the active version.)
> 

Yes, they behave like sym links in a virtual overlay (with a version) but there 
is no directly explicit information encoded in the zip file to express those 
links.


> On old-fashioned file systems with real version numbers, the Common Lisp 
> function TRUENAME does exactly what you are trying to do here.
> 
> http://www.mathcs.duq.edu/simon/Gcl/gcl_1141.html
> 

Touche, i should know better than to spar with you on language, and i know when 
i am beat when you play the Lisp card :-)


> (And in some way, we are sliding down the slope toward re-inventing those 
> file systems, aren't we?)
> 
> The older pre-nio API for File calls it "getCanonicalPath", but I think "true 
> name" is better than
> "canonical name", since "canonical" means "follows the rules", rather than 
> what we need in this case,
> which is "where it really is stored".
> 
> http://docs.oracle.com/javase/8/docs/api/java/io/File.html#getCanonicalPath--
> 
>> 
>>> In this case it appears you are simply exposing a translated name, not 
>>> making it real for the first time.
>>> 
>>> If this is true, I think you want to say "true name" or "real name" or 
>>> "translated name", not "reified name”.
>> 
>> or “versioned name" would work for me.
> 
> I'm just whinging about the term "reified" which doesn't seem to work, 
> logically.
> 
> "Versioned name" would work for me too.  But "true name" has the two good 
> precedents cited above.

Agreed.

Paul.

Re: RFR: 8155600: jaxp.library.TestPolicy should extend the default security policy

[huizhe wang]

Hi Frank,

The change looks good.

Thanks!

Joe

On 4/28/2016 1:11 AM, Frank Yuan wrote:


Hi  Mandy, Joe and all

Would you like to review the fix for bug 
https://bugs.openjdk.java.net/browse/JDK-8155514?


The webrev is at: http://cr.openjdk.java.net/~fyuan/8155514/webrev.00/ 
.


It's verified with the source bundle in 
http://scaaa637.us.oracle.com/archive/2016/04/2016-04-26-045641.mlchung.jdk9-deprivilege 



Thanks,

Frank

Re: RFR: 8155600: jaxp.library.TestPolicy should extend the default security policy

[huizhe wang]

Hi Frank,

Thanks for the quick fix.  The change looks good.

-Joe

On 4/28/2016 1:11 AM, Frank Yuan wrote:


Hi  Mandy, Joe and all

Would you like to review the fix for bug 
https://bugs.openjdk.java.net/browse/JDK-8155514?


The webrev is at: http://cr.openjdk.java.net/~fyuan/8155514/webrev.00/ 
.


It's verified with the source bundle in 
http://scaaa637.us.oracle.com/archive/2016/04/2016-04-26-045641.mlchung.jdk9-deprivilege 



Thanks,

Frank

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

[Stephen Colebourne]

I'd like to see the test cases in test_secondsPattern() check the
result of the parse (by passing more arguments from
data_secondsPattern)

Otherwise looks good.
Stephen

On 28 April 2016 at 14:12, nadeesh tv  wrote:
> Hi all,
> Please see the updated webrev
> http://cr.openjdk.java.net/~ntv/8148949/webrev.01/
>
> Regards,
> Nadeesh TV
>
> On 4/25/2016 8:08 PM, nadeesh tv wrote:
>>
>> HI all,
>> Please  review a fix for
>> Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949
>>
>> Issue - Pattern letters 'A'  does not match the intent of LDML/CLDR
>>
>> Solution -  Changed the definition of pattern letters 'A','n','N'
>>
>> Webrev -  http://cr.openjdk.java.net/~ntv/8148949/webrev.00/
>>
>>
>
> --
> Thanks and Regards,
> Nadeesh TV
>

Re: RFR: 8155600 - Performance optimization of Arrays.asList().iterator()

[Peter Levart]

On 04/28/2016 04:08 PM, Attila Szegedi wrote:

On Thu, Apr 28, 2016 at 3:52 PM, Aleksey Shipilev <
aleksey.shipi...@oracle.com> wrote:


  *) next(): Missing braces in throw new NSEE() block;
  *) next(): Why loading this.a into local?


I agree with Aleksey but I'd go as far as to say that not only this.a is
unnecessary, but i as well. The body of next() looks too elaborate. This
seems equivalent and concise:

   if (cursor >= a.length) {
   throw new NoSuchElementException();
   }
   return a[cursor++];

Attila.


...except that your variant can throw ArrayIndexOutOfBoundsException (if 
used inappropriately) and Tagir's can't.


(this.a is final and doesn't need to be loaded into local).

Regards, Peter

Re: RFR: 8155600 - Performance optimization of Arrays.asList().iterator()

[Attila Szegedi]

On Thu, Apr 28, 2016 at 3:52 PM, Aleksey Shipilev <
aleksey.shipi...@oracle.com> wrote:

>  *) next(): Missing braces in throw new NSEE() block;
>  *) next(): Why loading this.a into local?


I agree with Aleksey but I'd go as far as to say that not only this.a is
unnecessary, but i as well. The body of next() looks too elaborate. This
seems equivalent and concise:

  if (cursor >= a.length) {
  throw new NoSuchElementException();
  }
  return a[cursor++];

Attila.

Re: RFR: 8155600 - Performance optimization of Arrays.asList().iterator()

[Aleksey Shipilev]

On 04/28/2016 10:37 AM, Tagir F. Valeev wrote:
> Hello!
> 
> Please review and sponsor the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8155600
> http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r1/

I like the motivation and the patch.

Nitpicks:
 *) Does EA break if you make ArrayItr inner class to gain the access to
E[] a?
 *) next(): Missing braces in throw new NSEE() block;
 *) next(): Why loading this.a into local?


> What do you think? Is it reasonable change? Should I check some tests
> or add new ones for this optimization?

I think Arrays.asList is frequently used in tests already. But, a quick
boundary jtreg test would be nice to have, given now we have a special
iterator.

Thanks,
-Aleksey

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

[nadeesh tv]

Hi all,
Please see the updated webrev 
http://cr.openjdk.java.net/~ntv/8148949/webrev.01/


Regards,
Nadeesh TV
On 4/25/2016 8:08 PM, nadeesh tv wrote:

HI all,
Please  review a fix for
Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949

Issue - Pattern letters 'A'  does not match the intent of LDML/CLDR

Solution -  Changed the definition of pattern letters 'A','n','N'

Webrev -  http://cr.openjdk.java.net/~ntv/8148949/webrev.00/




--
Thanks and Regards,
Nadeesh TV

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

[nadeesh tv]

Hi all,

Thanks Stephen for the comments.

Please see the updated webrev 
http://cr.openjdk.java.net/~ntv/8079628/webrev.01/


Regards,
Nadeesh TV
On 4/26/2016 5:42 PM, Stephen Colebourne wrote:

This change introduces inconsistencies and problems. For example, it
will parse up to 19 digits for "D" but only up to 2 digits for "d".
The following would be better:

  *D   1  appendValue(ChronoField.DAY_OF_YEAR)
  *DD  2  appendValue(ChronoField.DAY_OF_YEAR, 2, 3,
SignStyle.NORMAL)
  *DDD 3  appendValue(ChronoField.DAY_OF_YEAR, 3)

This limits the accepted input to 3 digits, which is quite sufficient
here. It is also clearer for the common "D" and "DDD" cases.

Note that a test case needs to cover adjacent value parsing for
day-of-year. This pattern "DDD" should be tested and work. With
the webrev changes, it will not work as adjacent value parsing mode
will not be triggered.

(The change will alter the meaning of "DD" but no-one should be
using that anyway as it is broken, only working for the first 99 days
of the year.)

The code in TCKDateTimeFormatterBuilder needs a blank line after the
new methods.

Stephen


On 26 April 2016 at 12:48, nadeesh tv  wrote:

Hi all,

Please  review a fix for

Bug ID -   https://bugs.openjdk.java.net/browse/JDK-8079628

Issue -  Pattern 'D' is not implemented as intended by CLDR

Solution - Changed the definition of 'D' to D..D 1..3
appendValue(ChronoField.DAY_OF_YEAR, n, 19, SignStyle.NORMAL)

Webrev -  http://cr.openjdk.java.net/~ntv/8079628/webrev.00/

--
Thanks and Regards,
Nadeesh TV



--
Thanks and Regards,
Nadeesh TV

Re: RFR(m): 8140281 deprecate Optional.get()

[Stanislav Baiduzhyi]

Stuart,

Those examples are amazing. Would it be possible to expand Optional
class javadoc with some of those examples? First, second, and the last
one are the most descriptive in my opinion. I frowned at Optional for
so long, you just showed me that I was not using it properly.

On 26 April 2016 at 08:06, Stuart Marks  wrote:
>
>
> On 4/25/16 5:46 PM, Vitaly Davidovich wrote:
>>
>> We've introduced Optional to avoid this. The problem is that the
>> obvious
>> thing to do is to get() the value out of the Optional, but this buys
>> us
>> right back into the what-if-the-value-is-missing case that we had with
>> nulls.
>>
>> Why is this the "obvious" thing? Because a few stackoverflow threads went
>> like
>> that?
>
>
> Well, the Stack Overflow exchange was a caricature. But without looking too
> hard, I found this code: [2]
>
> return stringList.stream().filter(...).findFirst().get() != null;
>
> [2] http://stackoverflow.com/q/36346409/1441122
>
> The person, presumably new to Java 8, was able to determine that findFirst()
> returned an Optional instead of the desired value, and was able to figure
> out enough to call get(), and even thought enough about the possibly-absent
> case to check it against null. But the person had to ask on Stack Overflow
> about how to solve the problem.
>
> Now I obviously don't know the exact thought process that went on, but get()
> has among the shortest of name of the Optional methods, and has no
> arguments, so it seems like it ought to be called more more frequently
> compared to more specialized methods like ifPresentOrElse() or
> orElseThrow().
>
> There are other items on Stack Overflow such as these: [3] [4]. But these
> probably aren't what you're looking for. :-)
>
> [3] http://stackoverflow.com/a/26328555/1441122
>
> [4] http://stackoverflow.com/a/23464794/1441122
>
>
>> It's not clear it returns an Optional - it could be returning some other
>> type
>> that happens to have a getWhenPresent.  What would be clear it's an
>> Optional is
>> to have a local of that type and assign find(pk) to it.
>
>
> Of course there's always a possibility that there's some other API that has
> getWhenPresent() on it. There isn't one in the JDK, and I've been unable to
> find one elsewhere. So "getWhenPresent" should be different enough to
> trigger a different thought pattern, as opposed to a plain "get" which fades
> into the background.
>
> The Optional API was designed to be chained, so you don't want to pull it
> into a local variable.
>
>> Descriptive names are good, but like Jon, I'm not buying this change.  Is
>> it
>> really wrong to tell people to read the javadoc? And is that worth the
>> deprecation churn? It's going to be needlessly verbose (esp in existing
>> code
>> that already checked isPresent) for no strong benefit.  If someone is
>> mindlessly
>> having their IDE write code for them, this isn't going to prevent poor
>> code.
>> For code review purposes, if it's important to call out that something is
>> Optional then use the type somewhere explicitly.
>
>
> Personally I'm generally skeptical of simple name changes, but after having
> gone through a bunch of examples of the use of get(), my skepticism melted
> away. It really does seem to be misused a significant fraction of the time,
> possibly even a majority of the time.
>
> Beginners and Java 8 learners will probably fall into the trap of forgetting
> to check. But JDK programmers, who are decidedly not beginners, are writing
> code that uses get() unnecessarily:
>
> 
>
> http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/DependencyFinder.java.sdiff.html
>
>  206 if (source.isPresent()) {
>  207 executor.runTask(source.get(), deque);
>  208 }
>
> could be rewritten as
>
> source.ifPresent(archive -> executor.runTask(archive, deque));
>
> 
>
> http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java.sdiff.html
>
>  476 Optional req = options.requires.stream()
>  477 .filter(mn -> !modules.containsKey(mn))
>  478 .findFirst();
>  479 if (req.isPresent()) {
>  480 throw new BadArgs("err.module.not.found", req.get());
>  481 }
>
> could be rewritten as
>
> options.requires.stream()
>.filter(mn -> !modules.containsKey(mn))
>.findFirst()
>.ifPresent(s -> throw new BadArgs("err.module.not.found",
> s));
>
> 
>
> http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java.sdiff.html
>
> 1203 String hist = replayableHistory
> 

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Chris Hegarty]

On 28 Apr 2016, at 09:11, Alan Bateman  wrote:
> 
> On 27/04/2016 22:58, Steve Drach wrote:
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 
>> 
>> 
>> Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ 
>> 
>> 
>> This changeset causes the URL returned from a ClassLoader.getResource(name) 
>> invocation to be reified, that is the URL is a direct pointer to either a 
>> versioned or unversioned entry in the jar file. The patch also assures that 
>> jar URL’s are always processed by the URLClassPath.JarLoader.  The 
>> MultiReleaseJarURLConnection test was enhanced to demonstrate that reified 
>> URLs are returned.  The SimpleHttpServer test helper class was moved into 
>> it’s own file.
> I was happy to see John's note on diction so I assume the method will be 
> renamed. Have you considered making it public so that tools and libraries 
> outside of the JDK can use this?

It sounds reasonable for this to be a public API, and if so, does it make sense 
to
move it to JarEntry ( rather than JarFile )? So it would be 
JarEntry::getTrueName,
or similar.

-Chris.

Re[2]: RFR(m): 8140281 deprecate Optional.get()

[Victor Polischuk]

I am sorry if my comment would be inappropriate, but is there a reason not to 
introduce "Optional ifAbsent(Runnable emptyAction)" and change signature of 
ifPresent to "Optional ifPresent(Consumer consumer)"?

In that case method chaining would be more natural and it will give a decent 
way to have more than one simple action assigned to Optional without creating 
messy java block.

--- Original message ---
From: "Stuart Marks" 
Date: 28 April 2016, 05:50:45
> > Is there a better way of doing this?  We'd almost need a 
> > ifPresentElse(Consumer consumer, Runnable action) method I guess.
> Close. In JDK 9 we added
> 
> ifPresentOrElse(Consumer action, Runnable emptyAction)
> 
> s'marks

 
 

Re: RFR: 8155600 - Performance optimization of Arrays.asList().iterator()

[Peter Levart]

Hi Tagir,

I think this is a nice optimization. I like the part that allows EA to 
optimize-away the Arrays.ArrayList allocation when used in foreach loop.


Regards, Peter

On 04/28/2016 09:37 AM, Tagir F. Valeev wrote:

Hello!

Please review and sponsor the following patch:
https://bugs.openjdk.java.net/browse/JDK-8155600
http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r1/

It appears that custom implementation of Arrays.asList().iterator() is
noticeably more performant than the AbstractList implementation used
currently. Here's JMH test which illustrates this:

http://cr.openjdk.java.net/~tvaleev/webrev/8155600/jmh/

 @Benchmark
 public int sumList() {
 return sum(list);
 }

 @Benchmark
 public int sumArray() {
 return sum(Arrays.asList(arr));
 }

 public static int sum(Iterable data) {
 int sum = 0;
 for (int item : data) sum+=item;
 return sum;
 }

Here's summary result on my Intel x64 machine:

JDK 9ea+115:
Benchmark   (limit)  Mode  Cnt ScoreError  Units
ArrayTest.sumArray0  avgt   50 6,371 ±  0,009  ns/op
ArrayTest.sumArray1  avgt   50 8,020 ±  0,100  ns/op
ArrayTest.sumArray   10  avgt   5014,424 ±  0,060  ns/op
ArrayTest.sumArray  100  avgt   5088,684 ±  3,261  ns/op
ArrayTest.sumArray 1000  avgt   50   677,048 ±  1,050  ns/op
ArrayTest.sumArray1  avgt   50  8038,224 ± 59,678  ns/op
ArrayTest.sumList 0  avgt   50 3,395 ±  0,003  ns/op
ArrayTest.sumList 1  avgt   50 5,613 ±  0,061  ns/op
ArrayTest.sumList10  avgt   5013,448 ±  0,076  ns/op
ArrayTest.sumList   100  avgt   5097,242 ±  0,105  ns/op
ArrayTest.sumList  1000  avgt   50   664,342 ±  0,704  ns/op
ArrayTest.sumList 1  avgt   50  7971,230 ± 61,344  ns/op

JDK 9ea+115 patched:
Benchmark   (limit)  Mode  Cnt ScoreError  Units
ArrayTest.sumArray0  avgt   50 2,932 ±  0,011  ns/op
ArrayTest.sumArray1  avgt   50 5,000 ±  0,062  ns/op
ArrayTest.sumArray   10  avgt   50 9,421 ±  0,219  ns/op
ArrayTest.sumArray  100  avgt   5043,848 ±  0,187  ns/op
ArrayTest.sumArray 1000  avgt   50   383,619 ±  1,019  ns/op
ArrayTest.sumArray1  avgt   50  6860,087 ± 13,022  ns/op
ArrayTest.sumList 0  avgt   50 3,400 ±  0,008  ns/op
ArrayTest.sumList 1  avgt   50 6,142 ±  0,047  ns/op
ArrayTest.sumList10  avgt   5012,488 ±  0,084  ns/op
ArrayTest.sumList   100  avgt   5083,029 ±  0,078  ns/op
ArrayTest.sumList  1000  avgt   50   605,651 ±  0,992  ns/op
ArrayTest.sumList 1  avgt   50  7649,681 ± 16,355  ns/op

AbstractList.iterator() makes unnecessary bookkeeping like modCount
tracking which reduces the performance. Also it seems that having
implicit this$0 field in AbstractList.iterator() makes impossible to
avoid allocation of short-lived Arrays.ArrayList object in sumArray()
test. Launching JMH -prof gc confirms that non-patched sumArray() test
allocates 24 bytes per invocation while patched sumArray() does not
allocate anything.

What do you think? Is it reasonable change? Should I check some tests
or add new ones for this optimization?

With best regards,
Tagir Valeev.

Re: RFR [9] 8155578: OpenJDK build failed after JDK-8044773

[Alan Bateman]

On 28/04/2016 09:17, Chris Hegarty wrote:

This is a review for a trivial, but build-fatal, addition of a qualified export
to the jdk.net module, that was mistakenly omitted from the changes
for 8044773.



This looks fine.

RFR [9] 8155578: OpenJDK build failed after JDK-8044773

[Chris Hegarty]

This is a review for a trivial, but build-fatal, addition of a qualified export
to the jdk.net module, that was mistakenly omitted from the changes
for 8044773.

$ hg diff -U 11
diff --git a/src/java.base/share/classes/module-info.java 
b/src/java.base/share/classes/module-info.java
--- a/src/java.base/share/classes/module-info.java
+++ b/src/java.base/share/classes/module-info.java
@@ -158,22 +158,23 @@
 exports jdk.internal.misc to
 java.corba,
 java.desktop,
 java.logging,
 java.management,
 java.naming,
 java.rmi,
 java.security.jgss,
 java.sql,
 java.xml,
 jdk.charsets,
+jdk.net,
 jdk.scripting.nashorn,
 jdk.unsupported,
 jdk.vm.ci;
 exports jdk.internal.perf to
 java.desktop,
 java.management,
 jdk.jvmstat;
 exports jdk.internal.ref to
 java.desktop;
 exports jdk.internal.reflect to
 java.corba,

-Chris.

RFR: 8155600: jaxp.library.TestPolicy should extend the default security policy

[Frank Yuan]

Hi  Mandy, Joe and all

 

Would you like to review the fix for bug 
https://bugs.openjdk.java.net/browse/JDK-8155514?

 

The webrev is at: http://cr.openjdk.java.net/~fyuan/8155514/webrev.00/.

 

It's verified with the source bundle in 
http://scaaa637.us.oracle.com/archive/2016/04/2016-04-26-045641.mlchung.jdk9-deprivilege
 

 

 

Thanks,

 

Frank

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

[Alan Bateman]

On 27/04/2016 22:58, Steve Drach wrote:

Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 


Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ 


This changeset causes the URL returned from a ClassLoader.getResource(name) 
invocation to be reified, that is the URL is a direct pointer to either a 
versioned or unversioned entry in the jar file. The patch also assures that jar 
URL’s are always processed by the URLClassPath.JarLoader.  The 
MultiReleaseJarURLConnection test was enhanced to demonstrate that reified URLs 
are returned.  The SimpleHttpServer test helper class was moved into it’s own 
file.
I was happy to see John's note on diction so I assume the method will be 
renamed. Have you considered making it public so that tools and 
libraries outside of the JDK can use this?


One question on URLClassPath getLoader as it's not immediately obvious 
that this is needed. URLClassLoader specifies that any URL ending with 
"/" is assumed to be a directory.


Are you planning to update java.net.JarURLConnection as that has special 
handling for #runtime that will need to be replaced.


-Alan

RFR: 8155600 - Performance optimization of Arrays.asList().iterator()

[Tagir F. Valeev]

Hello!

Please review and sponsor the following patch:
https://bugs.openjdk.java.net/browse/JDK-8155600
http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r1/

It appears that custom implementation of Arrays.asList().iterator() is
noticeably more performant than the AbstractList implementation used
currently. Here's JMH test which illustrates this:

http://cr.openjdk.java.net/~tvaleev/webrev/8155600/jmh/

@Benchmark
public int sumList() {
return sum(list);
}

@Benchmark
public int sumArray() {
return sum(Arrays.asList(arr));
}

public static int sum(Iterable data) {
int sum = 0;
for (int item : data) sum+=item;
return sum;
}

Here's summary result on my Intel x64 machine:

JDK 9ea+115:
Benchmark   (limit)  Mode  Cnt ScoreError  Units
ArrayTest.sumArray0  avgt   50 6,371 ±  0,009  ns/op
ArrayTest.sumArray1  avgt   50 8,020 ±  0,100  ns/op
ArrayTest.sumArray   10  avgt   5014,424 ±  0,060  ns/op
ArrayTest.sumArray  100  avgt   5088,684 ±  3,261  ns/op
ArrayTest.sumArray 1000  avgt   50   677,048 ±  1,050  ns/op
ArrayTest.sumArray1  avgt   50  8038,224 ± 59,678  ns/op
ArrayTest.sumList 0  avgt   50 3,395 ±  0,003  ns/op
ArrayTest.sumList 1  avgt   50 5,613 ±  0,061  ns/op
ArrayTest.sumList10  avgt   5013,448 ±  0,076  ns/op
ArrayTest.sumList   100  avgt   5097,242 ±  0,105  ns/op
ArrayTest.sumList  1000  avgt   50   664,342 ±  0,704  ns/op
ArrayTest.sumList 1  avgt   50  7971,230 ± 61,344  ns/op

JDK 9ea+115 patched:
Benchmark   (limit)  Mode  Cnt ScoreError  Units
ArrayTest.sumArray0  avgt   50 2,932 ±  0,011  ns/op
ArrayTest.sumArray1  avgt   50 5,000 ±  0,062  ns/op
ArrayTest.sumArray   10  avgt   50 9,421 ±  0,219  ns/op
ArrayTest.sumArray  100  avgt   5043,848 ±  0,187  ns/op
ArrayTest.sumArray 1000  avgt   50   383,619 ±  1,019  ns/op
ArrayTest.sumArray1  avgt   50  6860,087 ± 13,022  ns/op
ArrayTest.sumList 0  avgt   50 3,400 ±  0,008  ns/op
ArrayTest.sumList 1  avgt   50 6,142 ±  0,047  ns/op
ArrayTest.sumList10  avgt   5012,488 ±  0,084  ns/op
ArrayTest.sumList   100  avgt   5083,029 ±  0,078  ns/op
ArrayTest.sumList  1000  avgt   50   605,651 ±  0,992  ns/op
ArrayTest.sumList 1  avgt   50  7649,681 ± 16,355  ns/op

AbstractList.iterator() makes unnecessary bookkeeping like modCount
tracking which reduces the performance. Also it seems that having
implicit this$0 field in AbstractList.iterator() makes impossible to
avoid allocation of short-lived Arrays.ArrayList object in sumArray()
test. Launching JMH -prof gc confirms that non-patched sumArray() test
allocates 24 bytes per invocation while patched sumArray() does not
allocate anything.

What do you think? Is it reasonable change? Should I check some tests
or add new ones for this optimization?

With best regards,
Tagir Valeev.