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

[Chris Hegarty]

> On 28 Apr 2016, at 21:54, Claes Redestad  wrote:
> 
> 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!

+1.

I’ve seen a few similar, but not the same, issues like this in the core area 
before.

-Chris.

> 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 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Alan Bateman]

On 29/04/2016 03:16, Felix Yang wrote:

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/
Felix - would it be disruptive to you if I asked to hold off on this 
until we get the changes in jake pushed to jdk9/dev (next week). The 
reason is that this we've changed most of these tests in jake to use 
-addmods. We've also replaced the TEST.properties for the 
javax.transaction tests so that they are run with a driver class 
instead. This is also related to the policy for root modules which 
impacts the tests that we have for the EE modules in the jdk repo.


-Alan

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

[Felix Yang]

On 2016/4/29 15:25, Alan Bateman wrote:

On 29/04/2016 03:16, Felix Yang wrote:

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/
Felix - would it be disruptive to you if I asked to hold off on this 
until we get the changes in jake pushed to jdk9/dev (next week). The 
reason is that this we've changed most of these tests in jake to use 
-addmods. We've also replaced the TEST.properties for the 
javax.transaction tests so that they are run with a driver class 
instead. This is also related to the policy for root modules which 
impacts the tests that we have for the EE modules in the jdk repo.

Alan,
 it is OK. Libs SQE will hold off to wait the changes in.

Thanks for the info,
Felix


-Alan

Re: RFR(XXS): 8149519: Investigate implementation of java.specification.version

[Alan Bateman]

On 27/04/2016 12:26, Martin Buchholz wrote:

I think the jdk9 docs have moved (again, g - I complain about this
sort of thing every release) and so a google search for "package class
se 9" only finds the old one +104 at
http://download.java.net/jdk9/docs/api/java/lang/Package.html
Why can't there be a redirect instead of leaving the old docs in place?

The infrastructure that publishes the builds and docs on java.net is 
several steps removed from most of us here but from what I understand, 
the stale docs have been purged and a redirect from the old location to 
the new location is in place.


When I search for "jdk 9 docs" now then Google serves up this link
  http://download.java.net/jdk9/docs/api/index.html?help-doc.html

and I get redirected to the docs at:
  http://download.java.net/java/jdk9/docs/api/index.html

-Alan

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

[Andrej Golovnin]

Hi Stuart,

> I also think it's reasonable, independently of any deprecation/renaming, for 
> IDE vendors to
> add rule checkers that attempt to detect unsafe uses of Optional. I'm a bit
> skeptical of whether it's possible for there to be automatic fixes, but
> detection would be a good step forward.

IntelliJ IDEA already warns you when you use Optional.get()
without Optional.isPresent(). Just use the right IDE! ;-)

I understand that you must use Optional.get() in the bootstrap code of
Java Runtime to avoid usage of lambdas. But in the most other cases
(I would say in 99.9%) you don't need the #get() method at all. At this place
I would like to quote the user raimille1 from the discussion
https://news.ycombinator.com/item?id=11588927 about your change:

"There are many Java 8 developers using streams() and optionals
with an imperative programming mindset still."

And here is an example from JDK. Take look at the code
http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java-.html
:

 397 cf.modules().forEach(m -> {
 398 ModuleReference mref = m.reference();
 399 map.put(mref.descriptor().name(), mref);
 400 });
 401
 402 // set of modules that are observable
 403 Set mrefs = new HashSet<>(map.values());
 404
 405 // add the other modules
 406 for (String mod : otherMods) {
 407 Optional omref = finder.find(mod);
 408 if (omref.isPresent()) {
 409 ModuleReference mref = omref.get();
 410 map.putIfAbsent(mod, mref);
 411 mrefs.add(mref);
 412 } else {
 413 // no need to fail
 414 }
 415 }

It is a mix of imperative and functional programming.
But you can rewrite it in a more functional style.
The lines 397-400 can be rewritten as:

  cf.modules()
 .stream()
 .map(ResolvedModule::reference)
 .forEach(mref -> map.put(mref.descriptor().name(), mref));

The lines 406-415 can be rewritten as:

  otherMods
  .stream()
  .map(finder::find) // After this step you can filter out all
empty Optionals.
  // But I don't think it is really needed.
  .forEach(
  o -> o.ifPresent(mref -> {
  map.putIfAbsent(mref.descriptor().name(), ref);
  mrefs.add(mref);
  }
  );

As you see there is no need for the stupid else-clause in the lines
412-414 and there is no need to call Optional.get().

Scala's Option class has also the #get()-method. But I have never seen
that someone asked to deprecate it and to add a new one. And in my
Scala projects I have never used it.

And like others I have also some wishes:

1. I think Optional should implement Iterable interface. You can use then
Optional everywhere where you can use Iterable today. And you will get
the #forEach()-method which I miss sometimes after using Scala's
Option class.

2. Please add Optional#isEmpty() method. The #isEmpty()-method is useful
when you want to filter out empty Optionals from a stream. Today I
have to write
the lambda o -> !o.isPresen(). With the new Optional#isEmpty()
method I can use
the method reference Optional::isEmpty.

3. Please add Option#orNull() method. It is easier to read than
Optional.orElse(null).

4. I think Optional should implement Serializable interface.
Currently the JEE developers cannot use it as return and parameter types
of methods of the session bean's remote interface. This is what we have
to write today:

  public Whatever getBestMatch(String someCriteria) {
   return someOtherSrvice
 .findAll(someCriteria)
 .stream()
 .map(...)
 .filter(...)
 .sorted(MyComparator::new)
 .findFirst()
 .orElse(null);
  }

Personally I would like to omit orElse(null) and just return the Optional
returned by #findFirst(). But because this method can be called by
a remote client
using RMI and RMI requires that all objects must be Serializable,
I cannot use Optional.
We have now this new cool API in JDK and we cannot use it in JEE
applications.
How stupid is that?

One off-topic wish:

Please integrate Tagir's StreamEx library (or at least some parts of it)
into JDK's Stream API. After adding his library to our product I see that
developers move away from JDK's Stream API because his library is
more useful than the standard Stream API.

   @Tagir: Thank you very much for StreamEx! Great job! :-)


As an average Java developer and an user of the Stream API and
the Optional class, I vote against deprecating of the Optional.get()-method.
(We have a democracy here, haven't we? :-D)

Best regards,
Andrej Golovnin

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

[Alan Bateman]

On 29/04/2016 10:23, Andrej Golovnin wrote:

:

"There are many Java 8 developers using streams() and optionals
with an imperative programming mindset still."

And here is an example from JDK. Take look at the code
http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java-.html
:

  397 cf.modules().forEach(m -> {
  398 ModuleReference mref = m.reference();
  399 map.put(mref.descriptor().name(), mref);
  400 });
  401
  402 // set of modules that are observable
  403 Set mrefs = new HashSet<>(map.values());
  404
  405 // add the other modules
  406 for (String mod : otherMods) {
  407 Optional omref = finder.find(mod);
  408 if (omref.isPresent()) {
  409 ModuleReference mref = omref.get();
  410 map.putIfAbsent(mod, mref);
  411 mrefs.add(mref);
  412 } else {
  413 // no need to fail
  414 }
  415 }

It is a mix of imperative and functional programming.
But you can rewrite it in a more functional style.
The lines 397-400 can be rewritten as:

   cf.modules()
  .stream()
  .map(ResolvedModule::reference)
  .forEach(mref -> map.put(mref.descriptor().name(), mref));


which btw is exactly how it is implemented elsewhere. However it was not 
possible to write it this way before refactoring of the API a few weeks 
ago and it's just that the jlink code wasn't cleaned up much during the 
refactoring.


In general then we have lots of code in the JDK that could be refactored 
and cleaned up. Some of the new code (such as jlink) has had to go 
through many iterations and refactoring and very easy to end up with 
mixes of functional and imperative. I expect there will be a lot of 
churn in this code over the coming months (not features work and just 
improving and iterating on the current implementation) and hopefully in 
better shape then.


-Alan.

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

[Aleksey Shipilev]

On 04/29/2016 02:09 AM, David Holmes wrote:
> bug: https://bugs.openjdk.java.net/browse/JDK-8154710
> webrev: http://cr.openjdk.java.net/~dholmes/8154710/webrev/

Looks good.

Is hrtime_t always integral, so you can "(hrtime_t)now /
NANOSECS_PER_MILLISEC" it?

> 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.

Observable enough to me.

Thanks,
-Aleksey

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

[Daniel Fuchs]

Hi Mandy,

This looks good to me.

Regarding final methods, I believe it might be prudent
to make toStackTraceElement() final - and possibly also
all the getters that call it.

It would be imprudent to try to override any of these
methods in a subclass without looking at the implementation
in the superclass. Making these methods final will ensure
that future maintainers will at least need to look at
the implementation in StackFrameInfo before they
override any of them.

best regards,

-- daniel


On 28/04/16 21:42, Mandy Chung wrote:



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: (S) RFR: 8154710: [Solaris] Investigate use of in-memory low-resolution timestamps for Java and internal time API's

[David Holmes]

On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:

On 04/29/2016 02:09 AM, David Holmes wrote:

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


Looks good.

Is hrtime_t always integral, so you can "(hrtime_t)now /
NANOSECS_PER_MILLISEC" it?


Yes it is a 64-bit (long long) signed integer.


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.


Observable enough to me.


:) Any apps you can think of that might show benefit from this?

Thanks,
David


Thanks,
-Aleksey

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

[Aleksey Shipilev]

On 04/29/2016 01:05 PM, David Holmes wrote:
> On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:
>> On 04/29/2016 02:09 AM, David Holmes wrote:
>>> 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.
>>
>> Observable enough to me.
> 
> :) Any apps you can think of that might show benefit from this?

Theoretically, this might affect heavily logging apps. IIRC, SPECjbb2000
was affected by currentTimeMillis performance. But, I see no reason in
trying to justify the change, apart from the targeted microbenchmark.

-Aleksey

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

[Aleksey Shipilev]

On 04/29/2016 10:09 AM, Chris Hegarty wrote:
>> On 28 Apr 2016, at 21:54, Claes Redestad > > wrote:
>>> 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!
> 
> +1.

Thanks, pushed.

-Aleksey

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

[Peter Levart]

On 04/29/2016 11:23 AM, Andrej Golovnin wrote:

The lines 406-415 can be rewritten as:

   otherMods
   .stream()
   .map(finder::find) // After this step you can filter out all
  // empty Optionals.
  // But I don't think it is really needed.
   .forEach(
   o -> o.ifPresent(mref -> {
   map.putIfAbsent(mref.descriptor().name(), ref);
   mrefs.add(mref);
   }
   );


...even more Stream-y (with JDK 9 changes to Optional):

otherMods
.stream()
.flatMap(mod -> finder.find(mod).stream())
.forEach(mref -> ...);


Yes, it takes some time for people to get used to new tools before they 
use them optimally. This is a normal learning process. So I wonder 
whether it is only the method name that is to blame for the observed 
percentage of wrong or sub-optimal usages of Optional or would it be 
more-or-less the same with a more descriptive name at this time.


But the argument that getWhenPresent() is easier to spot than get() when 
reading code is on spot so at least some bugs would get spotted more 
quickly if the method stood out.


Regards, Peter

Integer/Long reverse bits optimization

[Jaroslav Kameník]

Hello!

I have a small patch to Integer and Long classes, which is speeding up bit
reversion significantly.

Last two/three steps of bit reversion are doing byte reversion, so there is
possibility to use
intrinsified method reverseBytes. Java implementation of reverseBytes is
similar to those
steps, so it should give similar performance when intrinsics are not
available. Here I have
result of few performance tests (thank for hints, Aleksej:) :


old code:

# VM options: -server
ReverseInt.reverse   1  avgt5   8,766 ± 0,214  ns/op
ReverseLong.reverse1  avgt5   9,992 ± 0,165  ns/op

# VM options: -client
ReverseInt.reverse   1  avgt5   9,168 ± 0,268  ns/op
ReverseLong.reverse1  avgt5   9,988 ± 0,123  ns/op


patched:

# VM options: -server
ReverseInt.reverse   1  avgt5  6,411 ± 0,046  ns/op
ReverseLong.reverse1  avgt5  6,299 ± 0,158  ns/op

# VM options: -client
ReverseInt.reverse   1  avgt5  6,430 ± 0,022  ns/op
ReverseLong.reverse1  avgt5  6,301 ± 0,097  ns/op


patched, intrinsics disabled:

# VM options: -server -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l
ReverseInt.reverse   1  avgt5   9,597 ± 0,206  ns/op
ReverseLong.reverse1  avgt5   9,966 ± 0,151  ns/op

# VM options: -client -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l
ReverseInt.reverse   1  avgt5   9,609 ± 0,069  ns/op
ReverseLong.reverse1  avgt5   9,968 ± 0,075  ns/op



You can see, there is little slowdown in integer case with intrinsics
disabled.
It seems to be caused by different 'shape' of byte reverting code in
Integer.reverse and Integer.reverseByte. I tried to replace reverseByte code
with piece of reverse method, and it is as fast as not patched case:


ReverseInt.reverse  1  avgt5  9,184 ± 0,255  ns/op


Diffs from jdk9:

Integer.java:

@@ -1779,9 +1805,8 @@
i = (i & 0x) << 1 | (i >>> 1) & 0x;
i = (i & 0x) << 2 | (i >>> 2) & 0x;
i = (i & 0x0f0f0f0f) << 4 | (i >>> 4) & 0x0f0f0f0f;
-i = (i << 24) | ((i & 0xff00) << 8) |
-((i >>> 8) & 0xff00) | (i >>> 24);
-return i;
+
+return reverseBytes(i);

Long.java:

@@ -1940,10 +1997,8 @@
 i = (i & 0xL) << 1 | (i >>> 1) &
0xL;
i = (i & 0xL) << 2 | (i >>> 2) &
0xL;
i = (i & 0x0f0f0f0f0f0f0f0fL) << 4 | (i >>> 4) &
0x0f0f0f0f0f0f0f0fL;
-i = (i & 0x00ff00ff00ff00ffL) << 8 | (i >>> 8) &
0x00ff00ff00ff00ffL;
-i = (i << 48) | ((i & 0xL) << 16) |
-((i >>> 16) & 0xL) | (i >>> 48);
-return i;
+
+return reverseBytes(i);




Jaroslav

Re: RFR 8154755: Add a VarHandle weakCompareAndSet with volatile semantics

[Vladimir Ivanov]

Looks good.

Best regards,
Vladimir Ivanov

On 4/21/16 7:21 PM, Paul Sandoz wrote:

Hi,

Please review:

   
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8154755-weak-cas-volatile/webrev/

This webrev adds a new VarHandle access mode method, weakCompareAndSetVolatile 
[*]. (I also fixed some doc errors regarding AccessMode enum names that i 
forgot to update in a previous issue).

Aleksey noticed a discrepancy with the current set of weak CAS methods when 
comparing with C++ atomics.

On hardware that supports LL/SC (e.g. ARM) the weak volatile variant may be 
more performant than the strong volatile variant (compareAndSet) when the CAS 
is performed in a loop.

See the thread on the jmm-dev list for more details:

   http://mail.openjdk.java.net/pipermail/jmm-dev/2016-April/000239.html


Implementations currently defer to the strong volatile Unsafe CAS. We will 
follow up with changes to Unsafe, hotspot, and updating the VH implementations 
as separate issues.

Thanks,
Paul.

[*]
/**
  * Possibly atomically sets the value of a variable to the {@code newValue}
  * with the memory semantics of {@link #setVolatile} if the variable's
  * current value, referred to as the witness value, {@code ==} the
  * {@code expectedValue}, as accessed with the memory semantics of
  * {@link #getVolatile}.
  *
  * This operation may fail spuriously (typically, due to memory
  * contention) even if the witness value does match the expected value.
  *
...
  */
public final native
@MethodHandle.PolymorphicSignature
@HotSpotIntrinsicCandidate
boolean weakCompareAndSetVolatile(Object... args);





Paul.

Re: Integer/Long reverse bits optimization

[Claes Redestad]

Hi,

On 2016-04-29 13:36, Jaroslav Kameník wrote:

Hello!

I have a small patch to Integer and Long classes, which is speeding up bit
reversion significantly.

Last two/three steps of bit reversion are doing byte reversion, so there is
possibility to use
intrinsified method reverseBytes. Java implementation of reverseBytes is
similar to those
steps, so it should give similar performance when intrinsics are not
available. Here I have
result of few performance tests (thank for hints, Aleksej:) :


old code:

# VM options: -server
ReverseInt.reverse   1  avgt5   8,766 ± 0,214  ns/op
ReverseLong.reverse1  avgt5   9,992 ± 0,165  ns/op

# VM options: -client
ReverseInt.reverse   1  avgt5   9,168 ± 0,268  ns/op
ReverseLong.reverse1  avgt5   9,988 ± 0,123  ns/op


patched:

# VM options: -server
ReverseInt.reverse   1  avgt5  6,411 ± 0,046  ns/op
ReverseLong.reverse1  avgt5  6,299 ± 0,158  ns/op

# VM options: -client
ReverseInt.reverse   1  avgt5  6,430 ± 0,022  ns/op
ReverseLong.reverse1  avgt5  6,301 ± 0,097  ns/op


patched, intrinsics disabled:

# VM options: -server -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l
ReverseInt.reverse   1  avgt5   9,597 ± 0,206  ns/op
ReverseLong.reverse1  avgt5   9,966 ± 0,151  ns/op

# VM options: -client -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l
ReverseInt.reverse   1  avgt5   9,609 ± 0,069  ns/op
ReverseLong.reverse1  avgt5   9,968 ± 0,075  ns/op



You can see, there is little slowdown in integer case with intrinsics
disabled.
It seems to be caused by different 'shape' of byte reverting code in
Integer.reverse and Integer.reverseByte. I tried to replace reverseByte code
with piece of reverse method, and it is as fast as not patched case:


ReverseInt.reverse  1  avgt5  9,184 ± 0,255  ns/op


Diffs from jdk9:

Integer.java:

@@ -1779,9 +1805,8 @@
 i = (i & 0x) << 1 | (i >>> 1) & 0x;
 i = (i & 0x) << 2 | (i >>> 2) & 0x;
 i = (i & 0x0f0f0f0f) << 4 | (i >>> 4) & 0x0f0f0f0f;
-i = (i << 24) | ((i & 0xff00) << 8) |
-((i >>> 8) & 0xff00) | (i >>> 24);
-return i;
+
+return reverseBytes(i);

Long.java:

@@ -1940,10 +1997,8 @@
  i = (i & 0xL) << 1 | (i >>> 1) &
0xL;
 i = (i & 0xL) << 2 | (i >>> 2) &
0xL;
 i = (i & 0x0f0f0f0f0f0f0f0fL) << 4 | (i >>> 4) &
0x0f0f0f0f0f0f0f0fL;
-i = (i & 0x00ff00ff00ff00ffL) << 8 | (i >>> 8) &
0x00ff00ff00ff00ffL;
-i = (i << 48) | ((i & 0xL) << 16) |
-((i >>> 16) & 0xL) | (i >>> 48);
-return i;
+
+return reverseBytes(i);


reduces code duplication, improves performance...  what's the catch!? :-)

Updating Integer.reverseByte to have the same 'shape' as in 
Integer.reverseByte seems reasonable in this case.


I can sponsor this if you've signed the OCA etc.

Thanks!

/Claes

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

[Tagir F. Valeev]

Thank you for reviews! Here's updated webrev:

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

AS>  *) Does EA break if you make ArrayItr inner class to gain the access to
AS> E[] a?

No, I checked, it still allocates.

AS>  *) next(): Missing braces in throw new NSEE() block;

Fixed

AS>  *) next(): Why loading this.a into local?

Removed local a. Seems that this change does not affect the
performance.

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

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

I tried to mimic java.util.ArrayList.iterator() implementation. Here I
would agree with Peter Levart and leave local variable.

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

Simple test is implemented. I did not find proper class to put it, so
I created new class instead (test/java/util/Arrays/AsList.java).
Please check.

With best regards,
Tagir Valeev.

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

[Aleksey Shipilev]

On 04/29/2016 03:40 PM, Tagir F. Valeev wrote:
> http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r2/

JDK part looks good to me.

Test:
 *) Formatting: "i=0", "i AS>  *) Does EA break if you make ArrayItr inner class to gain the access to
> AS> E[] a?
> 
> No, I checked, it still allocates.

Yeah, this seems to be an issue with EA, investigating... We can push
this iterator in current form meanwhile.

Thanks,
-Aleksey

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

[charlie hunt]

> On Apr 29, 2016, at 5:12 AM, Aleksey Shipilev  
> wrote:
> 
>> On 04/29/2016 01:05 PM, David Holmes wrote:
>>> On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:
 On 04/29/2016 02:09 AM, David Holmes wrote:
 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.
>>> 
>>> Observable enough to me.
>> 
>> :) Any apps you can think of that might show benefit from this?
> 
> Theoretically, this might affect heavily logging apps. IIRC, SPECjbb2000
> was affected by currentTimeMillis performance. But, I see no reason in
> trying to justify the change, apart from the targeted microbenchmark.
> 
> -Aleksey

Fwiw, "back in the day" there was a slight gap in perf between Solaris and 
Windows on SPECjbb2005. That slight gap was attributed to differences in 
currentTimeMillis overhead.

Charlie 

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

[Tagir F. Valeev]

Hello!

Thank you for comments. Update:
http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r3/

AS> Test:
AS>  *) Formatting: "i=0", "i  *) Formatting: "} catch (NSEE ex)" should be on the same line;

Done.

AS>  *) What does the "for (int i=0; i<3; i++) {" thing test?

It tests the idempotence of final iterator state: it returns false
from hasNext() and throws from next() no matter how many times you
launch these methods. Sometimes poorly written iterators throw NSEE
only once, then change state somehow and subsequent hasNext()/next()
calls behave differently which violates the contract. For example,
something like this would fail my test:

if (cursor++ == a.length) throw new NoSuchElementException();

AS>  *) No need for "parallel = true", should be fine with sequential;

Done.

AS>  *) The idiom for exception testing is more self-contained:

AS>  try {
AS>itr.next();
AS>fail("Should have thrown NSEE");
AS>  } catch (NSEE ex) {
AS>// expected
AS>  }

Done.

AS>  *) Probably need to add these cases too, to boundary-test the null
AS> handling:
AS> {null}
AS> {null, 1}
AS> {1, null}
AS> {null, null}
AS> {null, 1, 2}
AS> {1, null, 2}
AS> {1, 2, null}
AS> {null, null, null}

Done.

With best regards,
Tagir Valeev.

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

[Daniel Fuchs]

Hi Aleksey,

On 29/04/16 12:12, Aleksey Shipilev wrote:

On 04/29/2016 01:05 PM, David Holmes wrote:

On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:

On 04/29/2016 02:09 AM, David Holmes wrote:

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.


Observable enough to me.


:) Any apps you can think of that might show benefit from this?


Theoretically, this might affect heavily logging apps. IIRC, SPECjbb2000
was affected by currentTimeMillis performance. But, I see no reason in
trying to justify the change, apart from the targeted microbenchmark.


If by "logging" you mean java.util.logging then this should have no
effect as logging now calls os::javaTimeSystemUTC (through java.time),
to get more precise time stamps.

best regards,

-- daniel



-Aleksey

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

[Claes Redestad]

On 2016-04-29 15:31, Tagir F. Valeev wrote:

Hello!

Thank you for comments. Update:
http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r3/


Looks good.

Thanks!

/Claes

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

[Aleksey Shipilev]

On 04/29/2016 04:38 PM, Claes Redestad wrote:
> On 2016-04-29 15:31, Tagir F. Valeev wrote:
>> Hello!
>>
>> Thank you for comments. Update:
>> http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r3/
> 
> Looks good.

Okay, good! I'll sponsor.

Thanks,
-Aleksey

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

[charlie hunt]

> On Apr 29, 2016, at 8:35 AM, Daniel Fuchs  wrote:
> 
> Hi Aleksey,
> 
> On 29/04/16 12:12, Aleksey Shipilev wrote:
>> On 04/29/2016 01:05 PM, David Holmes wrote:
>>> On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:
 On 04/29/2016 02:09 AM, David Holmes wrote:
> 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.
 
 Observable enough to me.
>>> 
>>> :) Any apps you can think of that might show benefit from this?
>> 
>> Theoretically, this might affect heavily logging apps. IIRC, SPECjbb2000
>> was affected by currentTimeMillis performance. But, I see no reason in
>> trying to justify the change, apart from the targeted microbenchmark.
> 
> If by "logging" you mean java.util.logging then this should have no
> effect as logging now calls os::javaTimeSystemUTC (through java.time),
> to get more precise time stamps.
> 
> best regards,
> 
> — daniel

I think Alexey means getting timestamps via System.currentTimeMillis() and 
internal JVM’s os::javaTimeMillis(), (which could have included logging). That 
was the intention with my comment wrt SPECjbb2005, (of which was of similar 
flavor as SPECjbb2000).  The good news (to me anyway) is SPECjbb2000 and 
SPECjbb2005 have been retired in favor of SPECjbb2015.

hths,

charlie

> 
>> 
>> -Aleksey

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

[Remi Forax]

Hi all,
Just nitpicking,
i wonder if it's not better to use the Doug Lea's convention in next()
i.e. rename i to cursor to shadow the field cursor  

   public E next() {
   int cursor = this.cursor;
if (cursor >= a.length) {
   throw new NoSuchElementException();
   }
   this.cursor = cursor + 1;
   return a[cursor];
   }

also ArrayItr should be marked final.

regards,
Rémi

- Mail original -
> De: "Claes Redestad" 
> À: "Tagir F. Valeev" , "Aleksey Shipilev" 
> 
> Cc: "core-libs-dev" 
> Envoyé: Vendredi 29 Avril 2016 15:38:48
> Objet: Re: RFR: 8155600 - Performance optimization of 
> Arrays.asList().iterator()
> 
> On 2016-04-29 15:31, Tagir F. Valeev wrote:
> > Hello!
> >
> > Thank you for comments. Update:
> > http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r3/
> 
> Looks good.
> 
> Thanks!
> 
> /Claes
> 

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

[Roger Riggs]

Hi,

This change seems fine to me; though barely observable only in a microcosm.

(I was going to make the same comment as Daniel, logging now uses higher 
resolution timestamps).


Roger


On 4/29/2016 9:46 AM, charlie hunt wrote:

On Apr 29, 2016, at 8:35 AM, Daniel Fuchs  wrote:

Hi Aleksey,

On 29/04/16 12:12, Aleksey Shipilev wrote:

On 04/29/2016 01:05 PM, David Holmes wrote:

On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:

On 04/29/2016 02:09 AM, David Holmes wrote:

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.

Observable enough to me.

:) Any apps you can think of that might show benefit from this?

Theoretically, this might affect heavily logging apps. IIRC, SPECjbb2000
was affected by currentTimeMillis performance. But, I see no reason in
trying to justify the change, apart from the targeted microbenchmark.

If by "logging" you mean java.util.logging then this should have no
effect as logging now calls os::javaTimeSystemUTC (through java.time),
to get more precise time stamps.

best regards,

— daniel

I think Alexey means getting timestamps via System.currentTimeMillis() and 
internal JVM’s os::javaTimeMillis(), (which could have included logging). That 
was the intention with my comment wrt SPECjbb2005, (of which was of similar 
flavor as SPECjbb2000).  The good news (to me anyway) is SPECjbb2000 and 
SPECjbb2005 have been retired in favor of SPECjbb2015.

hths,

charlie


-Aleksey

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

[Alan Bateman]

On 28/04/2016 22:18, Steve Drach wrote:
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/ 



The src changes looks okay but did we come to a conclusion on 
URLClassLoader spec? If not, can we revert the change to URLClassPath 
getLoader and deal with it separately?


-Alan

RFR: 8139982 Re-examine java.management dependency on java.util.logging.LoggingMXBean

[Daniel Fuchs]

Hi,

Please find below a patch [2] that eliminates a static
dependency of java.lang.management on java.util.logging.LoggingMXBean.

When JDK-6876135 was fixed, it introduced the PlatformLoggingMXBean
interface, and recommended using PlatformLoggingMXBean over
LoggingMXBean. However, it left a static dependency on 
java.util.logging.LoggingMXBean in java.lang.management:

The MBean registered in the MBeanServer was a private class
in ManagementFactoryHelper implementing a non standard interface
that extended both PlatformLoggingMXBean and LoggingMXBean.
(sun.management.ManagementFactoryHelper$LoggingMXBean)
(see https://bugs.openjdk.java.net/browse/JDK-6876135)

This patch proposes to relax the constraint on the
PlatformLoggingMXBean implementation - by no longer
forcing it to implement LoggingMXBean. This will allow
to get rid of the non standard interface, and make
the MBean declare "java.lang.management.PlatformLoggingMXBean"
as its MXBean interface.

To make it clear that PlatformLoggingMXBean should be
used instead of LoggingMXBean we also propose to deprecate
the java.util.logging.LoggingMXBean interface as well as
the LogManager.getLoggingMXBean() method (since it returns
a deprecated type). As of yet, there no intention of
removing any of these.

Backward Compatibility considerations:
--

1. Local clients which obtain an instance of the logging
MXBean by calling ManagementFactory.getPlatformMXBean(
"java.util.logging:type=Logging",
 PlatformLoggingMXBean.class)
will no longer be able to cast the result on
java.util.logging.LoggingMXBean.
[There should be few, given that PlatformLoggingMXBean
  already has all the methods defined in LoggingMXBean]

2. ManagementFactory.getPlatformMBeanServer().isInstanceOf(
 ObjectName, "java.util.logging.LoggingMXBean")
will now return 'false' instead of 'true'.

3. The Logging MXBean MBeanInfo will now report that its
management interface is "java.lang.management.PlatformLoggingMXBean"
instead of "sun.management.ManagementFactoryHelper$LoggingMXBean".

4. Calls to ManagementFactory.newPlatformMXBeanProxy(
MBeanServerConnection, ObjectName,
java.util.logging.LoggingMXBean.class); and
JMX.newMXBeanProxy(MBeanServerConnection, ObjectName,
java.util.logging.LoggingMXBean.class)
will continue to work as before.

5. Remote clients running previous version of the JDK
should see no changes, except for the interface
name in MBeanInfo, and the change in isInstanceOf
reported in 2.

[1] JBS issue:
https://bugs.openjdk.java.net/browse/JDK-8139982

[2] webrev:
http://cr.openjdk.java.net/~dfuchs/8139982_webrev/webrev.06/index.html

best regards,

-- daniel

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

[Andrej Golovnin]

Hi Peter,

> ...even more Stream-y (with JDK 9 changes to Optional):
> 
> otherMods
>.stream()
>.flatMap(mod -> finder.find(mod).stream())
>.forEach(mref -> ...);
> 
> 
> Yes, it takes some time for people to get used to new tools before they use 
> them optimally. This is a normal learning process. So I wonder whether it is 
> only the method name that is to blame for the observed percentage of wrong or 
> sub-optimal usages of Optional or would it be more-or-less the same with a 
> more descriptive name at this time.
> 
> But the argument that getWhenPresent() is easier to spot than get() when 
> reading code is on spot so at least some bugs would get spotted more quickly 
> if the method stood out.

Even when you call this new method

Optional.dudeCallIsPresentBeforeCallingMe()

it won't help. Against the stupidity, the inability to read JavaDocs and
the laziness helps only a compiler error. Therefore when you really want
to help  developers to avoid bugs due to wrong usage of Optional.get(),
then you must change the compiler to recognize the usage of Optional.get() 
without the isPresent()-check and to raise an error in such cases.
I mean if IntelliJ is able to give you a hint that you use Optional.get() 
without the isPresent()-check, then the Java compiler should be able
to do it too.

Just my two cents.

Best regards,
Andrej Golovnin

Re: 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/ 
>> 
>>  
> The src changes looks okay but did we come to a conclusion on URLClassLoader 
> spec? If not, can we revert the change to URLClassPath getLoader and deal 
> with it separately?

If we revert the change to URLClassPath, then we can’t read multi-release jars 
referenced by the “jar:{file,http}:/some/path/mr.jar!/“ form of a URL.  These 
jars would be treated as non-versioned.  That would mean that a jar referenced 
by the URL jar:file:/some/path/mr.jar!/ and one referenced by the URL 
file:/some/path/mr.jar could supply different classes for the same requested 
entry.  Is that what we want?

Re: Integer/Long reverse bits optimization

[Jaroslav Kameník]

Hi,

thank you:) I have OCA signed and processed.

I have changed Integer.reverseBytes, and added two small checks to
BitTwiddle tests for Integer and Long,
patch is attached.

Jaroslav

2016-04-29 14:33 GMT+02:00 Claes Redestad :

> Hi,
>
>
> On 2016-04-29 13:36, Jaroslav Kameník wrote:
>
>> Hello!
>>
>> I have a small patch to Integer and Long classes, which is speeding up bit
>> reversion significantly.
>>
>> Last two/three steps of bit reversion are doing byte reversion, so there
>> is
>> possibility to use
>> intrinsified method reverseBytes. Java implementation of reverseBytes is
>> similar to those
>> steps, so it should give similar performance when intrinsics are not
>> available. Here I have
>> result of few performance tests (thank for hints, Aleksej:) :
>>
>>
>> old code:
>>
>> # VM options: -server
>> ReverseInt.reverse   1  avgt5   8,766 ± 0,214  ns/op
>> ReverseLong.reverse1  avgt5   9,992 ± 0,165  ns/op
>>
>> # VM options: -client
>> ReverseInt.reverse   1  avgt5   9,168 ± 0,268  ns/op
>> ReverseLong.reverse1  avgt5   9,988 ± 0,123  ns/op
>>
>>
>> patched:
>>
>> # VM options: -server
>> ReverseInt.reverse   1  avgt5  6,411 ± 0,046  ns/op
>> ReverseLong.reverse1  avgt5  6,299 ± 0,158  ns/op
>>
>> # VM options: -client
>> ReverseInt.reverse   1  avgt5  6,430 ± 0,022  ns/op
>> ReverseLong.reverse1  avgt5  6,301 ± 0,097  ns/op
>>
>>
>> patched, intrinsics disabled:
>>
>> # VM options: -server -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l
>> ReverseInt.reverse   1  avgt5   9,597 ± 0,206  ns/op
>> ReverseLong.reverse1  avgt5   9,966 ± 0,151  ns/op
>>
>> # VM options: -client -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l
>> ReverseInt.reverse   1  avgt5   9,609 ± 0,069  ns/op
>> ReverseLong.reverse1  avgt5   9,968 ± 0,075  ns/op
>>
>>
>>
>> You can see, there is little slowdown in integer case with intrinsics
>> disabled.
>> It seems to be caused by different 'shape' of byte reverting code in
>> Integer.reverse and Integer.reverseByte. I tried to replace reverseByte
>> code
>> with piece of reverse method, and it is as fast as not patched case:
>>
>>
>> ReverseInt.reverse  1  avgt5  9,184 ± 0,255  ns/op
>>
>>
>> Diffs from jdk9:
>>
>> Integer.java:
>>
>> @@ -1779,9 +1805,8 @@
>>  i = (i & 0x) << 1 | (i >>> 1) & 0x;
>>  i = (i & 0x) << 2 | (i >>> 2) & 0x;
>>  i = (i & 0x0f0f0f0f) << 4 | (i >>> 4) & 0x0f0f0f0f;
>> -i = (i << 24) | ((i & 0xff00) << 8) |
>> -((i >>> 8) & 0xff00) | (i >>> 24);
>> -return i;
>> +
>> +return reverseBytes(i);
>>
>> Long.java:
>>
>> @@ -1940,10 +1997,8 @@
>>   i = (i & 0xL) << 1 | (i >>> 1) &
>> 0xL;
>>  i = (i & 0xL) << 2 | (i >>> 2) &
>> 0xL;
>>  i = (i & 0x0f0f0f0f0f0f0f0fL) << 4 | (i >>> 4) &
>> 0x0f0f0f0f0f0f0f0fL;
>> -i = (i & 0x00ff00ff00ff00ffL) << 8 | (i >>> 8) &
>> 0x00ff00ff00ff00ffL;
>> -i = (i << 48) | ((i & 0xL) << 16) |
>> -((i >>> 16) & 0xL) | (i >>> 48);
>> -return i;
>> +
>> +return reverseBytes(i);
>>
>
> reduces code duplication, improves performance...  what's the catch!? :-)
>
> Updating Integer.reverseByte to have the same 'shape' as in
> Integer.reverseByte seems reasonable in this case.
>
> I can sponsor this if you've signed the OCA etc.
>
> Thanks!
>
> /Claes
>
--- old/src/java.base/share/classes/java/lang/Integer.java	2016-04-29 17:52:22.407574334 +0200
+++ new/src/java.base/share/classes/java/lang/Integer.java	2016-04-29 17:52:22.295575132 +0200
@@ -1790,9 +1790,8 @@
 i = (i & 0x) << 1 | (i >>> 1) & 0x;
 i = (i & 0x) << 2 | (i >>> 2) & 0x;
 i = (i & 0x0f0f0f0f) << 4 | (i >>> 4) & 0x0f0f0f0f;
-i = (i << 24) | ((i & 0xff00) << 8) |
-((i >>> 8) & 0xff00) | (i >>> 24);
-return i;
+
+return reverseBytes(i);
 }
 
 /**
@@ -1820,10 +1819,10 @@
  */
 @HotSpotIntrinsicCandidate
 public static int reverseBytes(int i) {
-return ((i >>> 24)   ) |
-   ((i >>   8) &   0xFF00) |
-   ((i <<   8) & 0xFF) |
-   ((i << 24));
+return (i << 24)|
+   ((i & 0xff00) << 8)  |
+   ((i >>> 8) & 0xff00) |
+   (i >>> 24);
 }
 
 /**
--- old/src/java.base/share/classes/java/lang/Long.java	2016-04-29 17:52:22.667572486 +0200
+++ new/src/java.base/share/classes/java/lang/Long.java	2016-04-29 17:52:22.73282 +0200
@@ -1952,10 +1952,8 @@
 i = (i & 0xL) << 1 | (i >>> 1) & 0xL;
 i = (i & 0xL) << 2 | (i >>> 2) & 0xL;
 i = (i & 0x0f0f0f0f0f0f0f0fL) << 4 | (i >>> 4) & 0x0f0f0f0f0f0f0f0fL;
-i = (i & 0x00ff00ff

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

[Paul Sandoz]

> On 29 Apr 2016, at 07:32, Alan Bateman  wrote:
> 
> 
> 
> On 28/04/2016 22:18, Steve Drach wrote:
>> 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/
>> 
> The src changes looks okay but did we come to a conclusion on URLClassLoader 
> spec? If not, can we revert the change to URLClassPath getLoader and deal 
> with it separately?
> 

AFAICT this fix does not really change the existing implementation behaviour of 
URLClassLoader and URLClassPath. The patch redirects the processing of 
“jar:…/!” from the more general URLClassPath.Loader to the 
URLClassPath.JarLoader.

But i think the cracking of the URL within the jar URL can be made more robust. 
It probably should use

  file.indexOf(“!/“)

to check there is just one occurrence at the end.

Separately the spec of URLClassLoader could be clarified as to the current 
implementation behaviour.

Paul.

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

[Alan Bateman]

On 29/04/2016 16:55, Steve Drach wrote:
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/ 



The src changes looks okay but did we come to a conclusion on 
URLClassLoader spec? If not, can we revert the change to URLClassPath 
getLoader and deal with it separately?


If we revert the change to URLClassPath, then we can’t read 
multi-release jars referenced by the 
“jar:{file,http}:/some/path/mr.jar!/“ form of a URL.  These jars would 
be treated as non-versioned.  That would mean that a jar referenced by 
the URL jar:file:/some/path/mr.jar!/ and one referenced by the URL 
file:/some/path/mr.jar could supply different classes for the same 
requested entry.  Is that what we want?


So you are planning to eventually change the URLClassLoader spec to 
allow this or not?


-Alan.

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

[Steve Drach]

I put a new webrev out with the change suggested by Paul, using 
file.indexOf(“!/“) instead of file.endsWith(“!/“).

http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html 


> So you are planning to eventually change the URLClassLoader spec to allow 
> this or not?

I think we should push this changeset since URLClassLoader has always accepted 
the jar:…!/ URL and all this changeset does is enable it  to support 
multi-release jars.  And create an issue to track the change to the 
URLClassLoader api.

Re: RFR: 8139982 Re-examine java.management dependency on java.util.logging.LoggingMXBean

[Mandy Chung]

Hi Daniel,

> On Apr 29, 2016, at 8:08 AM, Daniel Fuchs  wrote:
> 
> Hi,
> 
> Please find below a patch [2] that eliminates a static
> dependency of java.lang.management on java.util.logging.LoggingMXBean.
> 
> When JDK-6876135 was fixed, it introduced the PlatformLoggingMXBean
> interface, and recommended using PlatformLoggingMXBean over
> LoggingMXBean.

Adding to this, JDK-6876135 prepared the JDK for modularization and 
PlatformLoggingMXBean was introduced that can be replaced with existing usage 
of LoggingMXBean.

With this change, java.management dependency on java.logging will become 
implementation details for logging purpose and can be eliminated completely in 
the future.

About the deprecation, to be specific, LoggingMXBean will no longer be a 
platform MXBean and an instance of LoggingMXBean will not register in the 
platform MBeanServer.

This is a revised wording for the deprecation note for LoggingMXBean:

@deprecated {@code LoggingMXBean} is no longer a {@link 
java.lang.management.PlatformManagedObject platform MXBean} and replaced with 
{@link java.lang.management.PlatformLoggingMXBean}.
It will not register in the platform MBeanServer. Use 
{@code ManagementFactory.getPlatformMXBean(PlatformLoggingMXBean.class)} 
instead.

One question about: ManagementFactory::getPlatformMXBean(MBeanServerConnection, 
PlatformLoggingMXBean.class)
- what would happen if this method is called from an image with java.logging 
module present and connect to a VM with no java.logging module?  Should the 
ManagementFactory::getPlatformMXBean spec or PlatformLoggingMXBean spec be 
clarified?

ManagementFactoryHelper.java

 191 if (result != null) {
 192 LoggingMXBeanSupport.class.getModule().addReads(m);
 193 }

Reflection access assumes readability now:
  
http://openjdk.java.net/projects/jigsaw/spec/issues/#ReflectionWithoutReadability

So this addReads is not needed.

 252 final Map methods = AccessController.doPrivileged(
 253 new PrivilegedAction<>() {
 254 @Override
 255 public Map run() {
 256 return initMethodMap(impl);
 257 }
 258 });

I believe this doPrivileged is not necessary and so do the other getMethod 
calls.  Probably you are thinking ahead what if java.management may one day be 
defined by a child loader of the defining loader of java.logging.  Then you can 
move doPrivileged to initMethodMap.

 217 // skip
- better to throw InternalError since it expects all methods are used?

 273 throw new SecurityException(ex);
- should not reach here.  SecurityException may cause confusion since this will 
be thrown even without security manager.  could simply throw InternalException

 296 private PlatformLoggingImpl(LoggingMXBeanSupport support) {
- perhaps renaming LoggingMXBeanSupport to LoggingProxy to better represent it.

> 
> Backward Compatibility considerations:
> --
> 
> 1. Local clients which obtain an instance of the logging
> MXBean by calling ManagementFactory.getPlatformMXBean(
>"java.util.logging:type=Logging",
> PlatformLoggingMXBean.class)
> will no longer be able to cast the result on
> java.util.logging.LoggingMXBean.
> [There should be few, given that PlatformLoggingMXBean
>  already has all the methods defined in LoggingMXBean]
> 

I expect this would be really rare too.

> 2. ManagementFactory.getPlatformMBeanServer().isInstanceOf(
> ObjectName, "java.util.logging.LoggingMXBean")
> will now return 'false' instead of 'true'.
> 
> 3. The Logging MXBean MBeanInfo will now report that its
> management interface is "java.lang.management.PlatformLoggingMXBean"
> instead of "sun.management.ManagementFactoryHelper$LoggingMXBean”.

Any impact to permission confiugred in security policy? Should also document 
that.

> 4. Calls to ManagementFactory.newPlatformMXBeanProxy(
>MBeanServerConnection, ObjectName,
>java.util.logging.LoggingMXBean.class); and
> JMX.newMXBeanProxy(MBeanServerConnection, ObjectName,
>java.util.logging.LoggingMXBean.class)
> will continue to work as before.
> 

> 5. Remote clients running previous version of the JDK
>should see no changes, except for the interface
>name in MBeanInfo, and the change in isInstanceOf
>reported in 2.

This is good.

The incompatibility risk for this change is rather low.

Mandy

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

[John Rose]

On Apr 28, 2016, at 11:26 AM, Steve Drach  wrote:
> 
> Keeping with the path precedent, is it acceptable to change “getReifiedName” 
> to “getRealName”?  

For me, in this setting, "true" and "real" are equally good.
(Perhaps they differ metaphysically, but that's too many for me.)
So, yes, nio.Path says "real", and I'm happy with that.
Thanks,
— John

> 
 
 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(XXS): 8149519: Investigate implementation of java.specification.version

[Martin Buchholz]

Today, I tried google-searching for "LinkedList se 9" which sent me to
http://download.java.net/jdk9/docs/api/java/util/LinkedList.html?is-external=true
which gives me 404.  That's an improvement on the stale +104 docs, and
should prod Google's engine into learning about better docs, but it
may still take a while.  Redirection could be improved.
I tried "binging" "LinkedList se 9" and it gave me the same result.

Like I always say, populate the final jdk 9 docs directory from the
very first EA build!

On Fri, Apr 29, 2016 at 1:57 AM, Alan Bateman  wrote:
>
> On 27/04/2016 12:26, Martin Buchholz wrote:
>>
>> I think the jdk9 docs have moved (again, g - I complain about this
>> sort of thing every release) and so a google search for "package class
>> se 9" only finds the old one +104 at
>> http://download.java.net/jdk9/docs/api/java/lang/Package.html
>> Why can't there be a redirect instead of leaving the old docs in place?
>>
> The infrastructure that publishes the builds and docs on java.net is several
> steps removed from most of us here but from what I understand, the stale
> docs have been purged and a redirect from the old location to the new
> location is in place.
>
> When I search for "jdk 9 docs" now then Google serves up this link
>   http://download.java.net/jdk9/docs/api/index.html?help-doc.html
>
> and I get redirected to the docs at:
>   http://download.java.net/java/jdk9/docs/api/index.html
>
> -Alan

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

[Xueming Shen]

JarFile.getRealName() invokes e.realEntry().getName() to get the "realname", 
which
may create a new JarFileEntry() if the names are different. Shouldn't the 
realname
just be the "super.getName()" ?

sherman

On 04/29/2016 11:36 AM, Steve Drach wrote:

I put a new webrev out with the change suggested by Paul, using 
file.indexOf(“!/“) instead of file.endsWith(“!/“).

http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html


So you are planning to eventually change the URLClassLoader spec to allow this 
or not?

I think we should push this changeset since URLClassLoader has always accepted 
the jar:…!/ URL and all this changeset does is enable it  to support 
multi-release jars.  And create an issue to track the change to the 
URLClassLoader api.

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

[Alan Bateman]

On 29/04/2016 19:36, Steve Drach wrote:
I put a new webrev out with the change suggested by Paul, using 
file.indexOf(“!/“) instead of file.endsWith(“!/“).


http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html 

This still needs to be fixed to compare the URL protocol without regard 
to case (yes, some of the old code isn't right but best not to make it 
worss).




So you are planning to eventually change the URLClassLoader spec to 
allow this or not?


I think we should push this changeset since URLClassLoader has always 
accepted the jar:…!/ URL and all this changeset does is enable it  to 
support multi-release jars.  And create an issue to track the change 
to the URLClassLoader api.


There shouldn't be any urgency with this change going in but if you are 
doing it this way then please create a high priority bug to get the spec 
and implementation aligned.


-Alan

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

[Steve Drach]

>> I put a new webrev out with the change suggested by Paul, using 
>> file.indexOf(“!/“) instead of file.endsWith(“!/“).
>> 
>> http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html 
>> This 
>> still needs to be fixed to compare the URL protocol without regard to case 
>> (yes, some of the old code isn't right but best not to make it worss).

Okay.

> 
>> 
>>> So you are planning to eventually change the URLClassLoader spec to allow 
>>> this or not?
>> 
>> I think we should push this changeset since URLClassLoader has always 
>> accepted the jar:…!/ URL and all this changeset does is enable it  to 
>> support multi-release jars.  And create an issue to track the change to the 
>> URLClassLoader api.
>> 
> There shouldn't be any urgency with this change going in but if you are doing 
> it this way then please create a high priority bug to get the spec and 
> implementation aligned.

Remember that I’ve not really changed anything, just enhanced what already 
exists to support multi-release jar files.  I submitted a bug, JDK-8155770, but 
I marked it as P4 according to the standard classifications:

P1 - Blocks development and/or testing work, production could not run.
P2 - Crashes, loss of data, severe memory leak.
P3 - Major loss of function.
P4 - Minor loss of function, or other problem where easy workaround is present.

I just didn’t see it as any higher than P4.  What would you like it to be?

 
 

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

[Alan Bateman]

On 29/04/2016 22:12, Steve Drach wrote:


I just didn’t see it as any higher than P4.  What would you like it to be?

We've stumbled on an issue where the spec and implementation are in 
conflict. The right thing to do it to fix it while we have a handle on it.


-Alan

Re: RFR(XXS): 8149519: Investigate implementation of java.specification.version

[Alan Bateman]

On 29/04/2016 21:03, Martin Buchholz wrote:

Today, I tried google-searching for "LinkedList se 9" which sent me to
http://download.java.net/jdk9/docs/api/java/util/LinkedList.html?is-external=true
which gives me 404.  That's an improvement on the stale +104 docs, and
should prod Google's engine into learning about better docs, but it
may still take a while.  Redirection could be improved.
I tried "binging" "LinkedList se 9" and it gave me the same result.

Like I always say, populate the final jdk 9 docs directory from the
very first EA build!

I completely agree that is poor form to change location. From what I can 
tell, they've put a redirect from the top level directory to the new 
location. This should at least keep bookmarks working. Also the stale 
docs are removed so I assume the search engines will eventually discard 
their memory of the old locations.


-Alan

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

[Steve Drach]

>> I just didn’t see it as any higher than P4.  What would you like it to be?
>> 
>>
> We've stumbled on an issue where the spec and implementation are in conflict. 
> The right thing to do it to fix it while we have a handle on it.

I bumped the priority to P2.

Re: RFR 8147984: WindowsTerminal should support function keys

[Stuart Marks]

Hi Jan,

I finally got a chance to take a look at this. The change looks fine.

It would be nice to have a reference to where the escape sequences are 
documented. There are links to the Windows VK_ codes there, which is great. But 
there's no reference for the escape sequences that each keypress is mapped to, 
e.g. F4 is "ESC O S", and F5 is "ESC [ 1 5 ~" (and what happened to "ESC [ 1 6 ~"??)


I did some searching, and it seems really hard to find a definitive reference. 
Perhaps the best reference is "XTerm Control Sequences" [1] which seems to 
document xterm pretty thoroughly, which is what everybody seems to follow 
nowadays. It even looks like it's being kept up to date (last modified 2016-02-21).


Anyway I'd suggest adding a comment with a reference to this document.

As a cross-check, these sequences match what my Mac's Terminal.app emits, at 
least for unshifted F1-F12. (The Terminal app was probably copied from xterm.)


Thanks,

s'marks


[1] http://invisible-island.net/xterm/ctlseqs/ctlseqs.html


On 1/22/16 3:41 AM, Jan Lahoda wrote:

Hello,

I'd like to enhance the WindowsTerminal in jdk.internal.le with function keys
handling. The intent is so that jshell can bind actions for shortcuts including
function keys.

The patch for adding the function keys support is here:
http://cr.openjdk.java.net/~jlahoda/8147984/webrev.00/

An example of a feature that uses/may use this support is here:
http://mail.openjdk.java.net/pipermail/kulla-dev/2016-January/001226.html

Any comments are welcome!

Thanks,
   Jan

RFR (M) 8155739: [TESTBUG] VarHandles/Unsafe tests for weakCAS should allow spurious failures

[Aleksey Shipilev]

Hi,

I would like to fix a simple testbug in our weakCompareAndSet VarHandles
and Unsafe intrinsics tests. weakCompareAndSet is spec-ed to allow
spurious failures, but current tests do not allow that. This blocks
development and testing on non-x86 platforms.

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

Webrevs:
  http://cr.openjdk.java.net/~shade/8155739/webrev.hs.00/
  http://cr.openjdk.java.net/~shade/8155739/webrev.jdk.00/

The tests are auto-generated, and the substantiative changes are in
*.template files. I also removed obsolete generate-unsafe-tests.sh. I
would like to push through hs-comp to expose this to Power and AArch64
folks early.

Testing: x86_64, jdk:java/lang/invoke/VarHandle, hotspot:compiler/unsafe

Thanks,
-Aleksey

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

[Steve Drach]

I updated the webrev with changes that Alan suggested

still needs to be fixed to compare the URL protocol without regard to 
case

And Sherman suggested

Shouldn't the realname just be the "super.getName()” ?

The webrev is at 
http://cr.openjdk.java.net/~sdrach/8151542/webrev.03/index.html 

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

[David Holmes]

(adding back hotspot-dev - still heed a hs/runtime reviewer)

Hi Roger,

On 30/04/2016 12:19 AM, Roger Riggs wrote:

Hi,

This change seems fine to me; though barely observable only in a microcosm.


Thanks for the review.


(I was going to make the same comment as Daniel, logging now uses higher
resolution timestamps).


Good to know.

Thanks,
David


Roger


On 4/29/2016 9:46 AM, charlie hunt wrote:

On Apr 29, 2016, at 8:35 AM, Daniel Fuchs 
wrote:

Hi Aleksey,

On 29/04/16 12:12, Aleksey Shipilev wrote:

On 04/29/2016 01:05 PM, David Holmes wrote:

On 29/04/2016 7:50 PM, Aleksey Shipilev wrote:

On 04/29/2016 02:09 AM, David Holmes wrote:

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.

Observable enough to me.

:) Any apps you can think of that might show benefit from this?

Theoretically, this might affect heavily logging apps. IIRC,
SPECjbb2000
was affected by currentTimeMillis performance. But, I see no reason in
trying to justify the change, apart from the targeted microbenchmark.

If by "logging" you mean java.util.logging then this should have no
effect as logging now calls os::javaTimeSystemUTC (through java.time),
to get more precise time stamps.

best regards,

— daniel

I think Alexey means getting timestamps via System.currentTimeMillis()
and internal JVM’s os::javaTimeMillis(), (which could have included
logging). That was the intention with my comment wrt SPECjbb2005, (of
which was of similar flavor as SPECjbb2000).  The good news (to me
anyway) is SPECjbb2000 and SPECjbb2005 have been retired in favor of
SPECjbb2015.

hths,

charlie


-Aleksey

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

[Paul Sandoz]

> On 29 Apr 2016, at 13:46, Alan Bateman  wrote:
> 
> 
> 
> On 29/04/2016 19:36, Steve Drach wrote:
>> I put a new webrev out with the change suggested by Paul, using 
>> file.indexOf(“!/“) instead of file.endsWith(“!/“).
>> 
>> http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html
> This still needs to be fixed to compare the URL protocol without regard to 
> case (yes, some of the old code isn't right but best not to make it worss).
> 

That triggered me to look at this much more closely.

AFAICT URL instances do canonicalize the protocol to lower case, so it’s only 
where the raw URL characters are checked that this would need to be done.

Steve is there a case whereby when you extract the nested URL from a jar URL it 
would ever get routed to Loader or FileLoader? It seems it would always route 
to JarLoader. If so, off the top of my head, could you do:

String protocol = url.getProtocol();
String file = url.getFile();
if ("jar".equals(protocol) && file != null && file.indexOf("!/") == 
file.length() - 2) {
// jar: protocols
URL nestedUrlToJarFile = new URL(file.substring(0, file.length() - 2)); 
 // extract the nested URL
return new JarLoader(nestedUrlToJarFile, jarHandler, lmap);
} else if ("file".equals(protocol)) {
// file: protocols
if (file != null && file.endsWith("/“)) {
// Exploded from base directory
return new FileLoader(url);
} else {
   // Assumed to be a jar file
   return new JarLoader(url, jarHandler, lmap);
}
} else {
// Fallback to URL connection
return new Loader(url);
}

That makes all the cases much clearer.

?

Paul.

>> 
>>> So you are planning to eventually change the URLClassLoader spec to allow 
>>> this or not?
>> 
>> I think we should push this changeset since URLClassLoader has always 
>> accepted the jar:…!/ URL and all this changeset does is enable it  to 
>> support multi-release jars.  And create an issue to track the change to the 
>> URLClassLoader api.
>> 
> There shouldn't be any urgency with this change going in but if you are doing 
> it this way then please create a high priority bug to get the spec and 
> implementation aligned.
> 
> -Alan

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

[Daniel D. Daugherty]

On 4/28/16 5:09 PM, David Holmes wrote:

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


src/os/solaris/vm/os_solaris.cpp
L1356: static _get_nsec_fromepoch_func_t  _get_nsec_fromepoch = NULL;
nit: two spaced between the type and the var name.
Not sure why since you aren't lining up with anything.

L: Solaris::_pthread_setname_np =  // from 11.3
Thanks for documenting the release.

L4450:
nit: why add a blank line?

Thumbs up!  Nits only so feel free to fix or ignore, but don't
need another webrev.

Dan




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: RFR (M) 8155739: [TESTBUG] VarHandles/Unsafe tests for weakCAS should allow spurious failures

[Paul Sandoz]

> On 29 Apr 2016, at 15:12, Aleksey Shipilev  
> wrote:
> 
> Hi,
> 
> I would like to fix a simple testbug in our weakCompareAndSet VarHandles
> and Unsafe intrinsics tests. weakCompareAndSet is spec-ed to allow
> spurious failures, but current tests do not allow that. This blocks
> development and testing on non-x86 platforms.
> 
> Bug:
>  https://bugs.openjdk.java.net/browse/JDK-8155739
> 
> Webrevs:
>  http://cr.openjdk.java.net/~shade/8155739/webrev.hs.00/
>  http://cr.openjdk.java.net/~shade/8155739/webrev.jdk.00/
> 

Looks good.

Small tweak if you so wish to do so:

#if[JdkInternalMisc]
static final int WEAK_ATTEMPTS = Integer.getInteger("weakAttempts", 10);
#end[JdkInternalMisc]

which avoids changes to the SunMiscUnsafe* tests.

Paul.

> The tests are auto-generated, and the substantiative changes are in
> *.template files. I also removed obsolete generate-unsafe-tests.sh. I
> would like to push through hs-comp to expose this to Power and AArch64
> folks early.
> 
> Testing: x86_64, jdk:java/lang/invoke/VarHandle, hotspot:compiler/unsafe
> 
> Thanks,
> -Aleksey
> 

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

[Xueming Shen]

On 4/29/16 3:30 PM, Steve Drach wrote:

I updated the webrev with changes that Alan suggested

still needs to be fixed to compare the URL protocol without regard to case


-> toLowerCase(Locale.ROOT).



And Sherman suggested

Shouldn't the realname just be the "super.getName()” ?

The webrev is at 
http://cr.openjdk.java.net/~sdrach/8151542/webrev.03/index.html 

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

[Steve Drach]

> On Apr 29, 2016, at 4:47 PM, Xueming Shen  wrote:
> 
> On 4/29/16 3:30 PM, Steve Drach wrote:
>> I updated the webrev with changes that Alan suggested
>> 
>>  still needs to be fixed to compare the URL protocol without regard to 
>> case
> 
> -> toLowerCase(Locale.ROOT).

Actually it’s already canonicalized in URL, so I don’t need to do it.

> 
>> 
>> And Sherman suggested
>> 
>>  Shouldn't the realname just be the "super.getName()” ?
>> 
>> The webrev is at 
>> http://cr.openjdk.java.net/~sdrach/8151542/webrev.03/index.html 
>> 
>> 
> 

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

[Steve Drach]

Hopefully the last one ;-)  This webrev removes the lowercase of protocol, and 
incorporates better (in my mind) seperation of choices for choosing the loader, 
similar to what Paul suggested.  Everything else remains the same.  Only 
URLClassPath changed from previous webrev.

http://cr.openjdk.java.net/~sdrach/8151542/webrev.04/index.html 

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

[David Holmes]

On 30/04/2016 9:28 AM, Daniel D. Daugherty wrote:

On 4/28/16 5:09 PM, David Holmes wrote:

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


src/os/solaris/vm/os_solaris.cpp
L1356: static _get_nsec_fromepoch_func_t  _get_nsec_fromepoch = NULL;
nit: two spaced between the type and the var name.
Not sure why since you aren't lining up with anything.

L: Solaris::_pthread_setname_np =  // from 11.3
Thanks for documenting the release.

L4450:
nit: why add a blank line?

Thumbs up!  Nits only so feel free to fix or ignore, but don't
need another webrev.


Thanks for the review Dan - will nit fix. :)

David


Dan




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

Review request: 8154190 & 8155513: Deprivilege java.compiler and jdk.charsets

[Mandy Chung]

JDK-8154190: Deprivilege java.compiler module
Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8154190/webrev.00/

JDK-8155513: Deprivilege jdk.charsets module
Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8155513/webrev.00/

Very simple change.

These patches move java.compiler and jdk.charsets module to be defined by the 
platform class loader and grant with AllPermissions initially.  We could grant 
finer-grained permissions in the future.

Mandy

Re: Review request: 8154190 & 8155513: Deprivilege java.compiler and jdk.charsets

[Alan Bateman]

On 30/04/2016 06:02, Mandy Chung wrote:

JDK-8154190: Deprivilege java.compiler module
Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8154190/webrev.00/

JDK-8155513: Deprivilege jdk.charsets module
Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8155513/webrev.00/

Very simple change.

These patches move java.compiler and jdk.charsets module to be defined by the 
platform class loader and grant with AllPermissions initially.  We could grant 
finer-grained permissions in the future.

This looks okay to me. For jdk.charsets then I expect it should be easy 
to identify the permission it needs as there are only a few cases where 
it needs os.name or to read a resource file in the image.


-Alan.