Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

[Tony Printezis]

Good idea, as long as it re-uses the existing ThreadLocal infrastructure
and doesn’t introduce an extra per-thread map. Making it a ThreadLocal
subclass would be an excellent start.

Tony

—
Tony Printezis | @TonyPrintezis | tprinte...@twitter.com


On June 19, 2018 at 9:07:07 AM, David Lloyd (david.ll...@redhat.com) wrote:

It may be confusing (to some, I guess) but it is consistent: the
ThreadLocal subclass author has absolute control over how the value is
presented to the user. Adding compute() or many of the other
suggested variants will break that guarantee, which seems like kind of
a big deal to me.

What about introducing a different thread local API that has more
modern behavior? Maybe a new subclass of ThreadLocal?

On Mon, Jun 18, 2018 at 5:28 PM Martin Buchholz 
wrote:
>
> yes, the proposed new methods would use nulls differently. The original
get() behavior of creating a mapping was a mistake, inconsistent with Map.
Yes, it will cause confusion. But it's already confusing.
>
> On Mon, Jun 18, 2018 at 1:45 PM, David Lloyd 
wrote:
>>
>> On Mon, Jun 18, 2018 at 12:53 PM, Martin Buchholz 
wrote:
>> > On Mon, Jun 18, 2018 at 10:21 AM, Tony Printezis <
tprinte...@twitter.com>
>> > wrote:
>> >
>> >> Martin,
>> >>
>> >> You are forgiven. :-)
>> >>
>> >> So, the (valid, I think) issue with getIfPresent(), as originally
proposed
>> >> by Peter, was that if get() was overridden in the subclass to somehow
>> >> transform the returned value, getIfPresent() wouldn’t apply the same
>> >> transformation.
>> >>
>> >> Doesn’t your compute() method have essentially the same issue? Apart
from
>> >> that, I personally like this proposal as I agree: one look-up is
always
>> >> better than two.
>> >>
>> >>
>> > A non-prototype implementation would delegate compute into
ThreadLocalMap
>> > itself, where there is no risk of overriding.
>>
>> I don't think overriding is the risk; the risk would be compute*
>> behaving inconsistently with regards to an overridden get*.
>>
>>
>> --
>> - DML
>
>


-- 
- DML

Re: BiCollector

[Victor Williams Stafusa da Silva]

What about forking?

2018-06-19 13:42 GMT-03:00 Paul Sandoz :

> Gosh, this is a tricky one to name.
>
> collectingTo seems the best so far, although collect(collectingTo(…)) ...
>
> One last suggestion from me, “expanding”, as in the collector expands the
> number of collectors the input elements are applied to.
>
> Paul.
>
>
>
> > On Jun 19, 2018, at 7:47 AM, Brian Goetz  wrote:
> >
> > It is "distributing" in the same sense as the distributive law:
> >
> > c*(a+b) = c*a + c*b
> >
> > (Think of the two collectors as the "sum" of a collector, and
> "distributing" says that you can send the elements to the sum by sending
> all of the elements to each.)
> >
> > That said, I agree that the less mathematically-inclined might be drawn
> to the plain-english meaning, which is more like an (imprecise) bisection.
> >
> > On 6/19/2018 10:14 AM, Zheka Kozlov wrote:
> >> I don't like `distributing` for the same reason as `bisecting`: for me,
> it sounds like a Stream is giving each collector only a part of elements.
> >>
> >> 2018-06-19 19:44 GMT+07:00 Brian Goetz  brian.go...@oracle.com>>:
> >>
> >>
> >>
> >>collectingToBoth
> >>
> >>
> >>This one is actually both evocative of what the method does, and
> >>in the spirit of the existing naming conventions (collectingAndThen.)
> >>
> >>An n-ary version could just be called `collectingTo`, where it is
> >>passed a varargs of Collector.  Could we get away with
> >>collectingTo for a binary version as well?  The existence of the
> >>"combiner" function might make that a stretch, but I prefer
> >>`collectingTo` to `collectingToBoth`.
> >>
> >>
> >>(I still like `distributing` too.)
> >>
> >>
> >
>
>

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[David Holmes]

Sorry another update is imminent ... stay tuned.

David

On 19/06/2018 2:41 PM, David Holmes wrote:
Discussions on the CSR request have led to further changes to the 
documentation involving nests and the new nest-related method. There are 
no semantic changes here just clearer explanations.


Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v7-incr/


(don't worry if you don't see a v6, it didn't really exist).

Full webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v7/


Specdiffs updated in place at: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/specs/


Summary of changes:

- The definition of a nest etc is moved to the class-level javadoc of 
java.lang.Class, along with some other edits provided by Alex Buckley to 
pave the way for future updates

- The nest-related methods are written in a more clear and consistent way

Thanks,
David
-

On 12/06/2018 3:16 PM, David Holmes wrote:

Here is one further minor update from the CSR discussions:

http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v5-incr/src/java.base/share/classes/java/lang/Class.java.cdiff.html 



Thanks,
David

On 25/05/2018 3:52 PM, David Holmes wrote:
Here are the further minor updates so far in response to all the 
review comments.


Incremental corelibs webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3-incr/ 



Full corelibs webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3/

Change summary:

src/java.base/share/classes/jdk/internal/reflect/Reflection.java
- remove inaccurate pseudo-assertion comment

test/jdk/java/lang/reflect/Nestmates/SampleNest.java
- code cleanup: <> operator

test/jdk/java/lang/reflect/Nestmates/TestReflectionAPI.java
- code cleanup: streamify duplicate removals

test/jdk/java/lang/invoke/PrivateInterfaceCall.java
- use consistent @bug number

Thanks,
David

On 22/05/2018 8:15 PM, David Holmes wrote:

Here are the updates so far in response to all the review comments.

Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2-incr/ 



Full webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2/


Change summary:

src/java.base/share/classes/java/lang/Class.java
- getNesthost:
   - change "any error" -> "any linkage error" as runtime errors 
will propagate. [This needs ratifying by EG]
   - add clarification that primitive and array classes are not 
explicitly members of any nest and so form singleton nests

   - add clarification that all nestmates are in the same package
   - re-word @return text to exclude the "royal 'we'"
- fix javadoc cross references

---

Moved reflection API tests from 
test/hotspot/jtreg/runtime/Nestmates/reflectionAPI/ to 
test/jdk/java/lang/reflect/Nestmates/


---

java/lang/reflect/Nestmates/TestReflectionAPI.java

Run tests twice to show that failure reasons remain the same.

---

test/jdk/jdk/lambda/vm/InterfaceAccessFlagsTest.java

Disable test via annotation rather than commenting out.

---

src/java.base/share/classes/jdk/internal/reflect/Reflection.java

- Fix indent for nestmate access check.
- Remove unnecessary local variable

---

src/java.base/share/classes/sun/invoke/util/VerifyAccess.java

- Replace myassert with proper assert

---

Thanks,
David

On 15/05/2018 10:52 AM, David Holmes wrote:
This review is being spread across four groups: langtools, 
core-libs, hotspot and serviceability. This is the specific review 
thread for core-libs - webrev:


http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/

See below for full details - including annotated full webrev 
guiding the review.


The intent is to have JEP-181 targeted and integrated by the end of 
this month.


Thanks,
David
-

The nestmates project (JEP-181) introduces new classfile attributes 
to identify classes and interfaces in the same nest, so that the VM 
can perform access control based on those attributes and so allow 
direct private access between nestmates without requiring javac to 
generate synthetic accessor methods. These access control changes 
also extend to core reflection and the MethodHandle.Lookup contexts.


Direct private calls between nestmates requires a more general 
calling context than is permitted by invokespecial, and so the JVMS 
is updated to allow, and javac updated to use, invokevirtual and 
invokeinterface for private class and interface method calls 
respectively. These changed semantics also extend to MethodHandle 
findXXX operations.


At this time we are only concerned with static nest definitions, 
which map to a top-level class/interface as the nest-host and all 
its nested types as nest-members.


Please see the JEP for further details.

JEP: https://bugs.openjdk.java.net/browse/JDK-8046171
Bug: https://bugs.openjdk.java.net/browse/JDK-8010319
CSR: https://bugs.openjdk.java.net/browse/JDK-8197445

All of the specification changes have been previously 

Re: RFR JDK-8066709 Make some JDK system properties read only

[Brent Christian]

On 6/19/18 8:08 AM, Roger Riggs wrote:


* src/java.base/share/classes/java/lang/System.java :

Should the @implNote with the list of cached properties be added 
everywhere the @apiNote is being added ?  Right now the @implNote is 
only added to getProperties().


The repetition was getting tiresome and the base of all the 
xxxProperties methods is getProperties.
  Joe suggested having one copy of the full information  and referring 
to that from the individual @apiNotes.


Fair enough.


* src/java.base/share/classes/jdk/internal/util/StaticProperty.java :

  45 private StaticProperty() {
  46
  47 }

Maybe put this all on one line?


Will do


Thanks,
-Brent

RE: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.

[Deshpande, Vivek R]

Thanks David.
Sending it to core-libs-dev.

 I would like to contribute a patch which improves the array comparison when 
there is a mismatch for the first element.
 This avoids call to vectorizedMismatch method and gives ~80x speed up.
 Could you please review and sponsor the patch.
 Link to bug:
 https://bugs.openjdk.java.net/browse/JDK-8205194
 webrev:
 http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.00/
 
Regards,
Vivek

-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com] 
Sent: Monday, June 18, 2018 10:32 PM
To: Deshpande, Vivek R ; jdk-...@openjdk.java.net
Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a 
mismatch at first element.

Hi Vivek,

Reviews should take place on the appropriate mailing list for the code being 
changed, not on the jdk-dev list. Please takes this to core-libs-dev.

Thanks,
David

On 19/06/2018 9:52 AM, Deshpande, Vivek R wrote:
> Hi All
> 
> Forgot to add the links:
> https://bugs.openjdk.java.net/browse/JDK-8205194
> webrev:
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
> 0/
> 
> Regards.
> Vivek
> 
> From: Deshpande, Vivek R
> Sent: Monday, June 18, 2018 4:50 PM
> To: 'jdk-...@openjdk.java.net' 
> Cc: 'Paul Sandoz' ; Viswanathan, Sandhya 
> 
> Subject: RFR(S): 8205194: Improve the Array Comparison when there is a 
> mismatch at first element.
> 
> Hi All
> 
> I would like to contribute a patch which improves the array comparison when 
> there is a mismatch for the first element.
> This avoids call to vectorizedMismatch method and gives ~80x speed up.
> Please review and sponsor the patch.
> 
> Regards,
> Vivek
> 

Re: BiCollector

[Paul Sandoz]

Gosh, this is a tricky one to name.

collectingTo seems the best so far, although collect(collectingTo(…)) ...

One last suggestion from me, “expanding”, as in the collector expands the 
number of collectors the input elements are applied to.

Paul.



> On Jun 19, 2018, at 7:47 AM, Brian Goetz  wrote:
> 
> It is "distributing" in the same sense as the distributive law:
> 
> c*(a+b) = c*a + c*b
> 
> (Think of the two collectors as the "sum" of a collector, and "distributing" 
> says that you can send the elements to the sum by sending all of the elements 
> to each.)
> 
> That said, I agree that the less mathematically-inclined might be drawn to 
> the plain-english meaning, which is more like an (imprecise) bisection.
> 
> On 6/19/2018 10:14 AM, Zheka Kozlov wrote:
>> I don't like `distributing` for the same reason as `bisecting`: for me, it 
>> sounds like a Stream is giving each collector only a part of elements.
>> 
>> 2018-06-19 19:44 GMT+07:00 Brian Goetz > >:
>> 
>> 
>> 
>>collectingToBoth
>> 
>> 
>>This one is actually both evocative of what the method does, and
>>in the spirit of the existing naming conventions (collectingAndThen.)
>> 
>>An n-ary version could just be called `collectingTo`, where it is
>>passed a varargs of Collector.  Could we get away with
>>collectingTo for a binary version as well?  The existence of the
>>"combiner" function might make that a stretch, but I prefer
>>`collectingTo` to `collectingToBoth`.
>> 
>> 
>>(I still like `distributing` too.)
>> 
>> 
> 

Re: BiCollector

[Peter Levart]

On 06/19/18 02:38, John Rose wrote:

unzipping(
Function f1,  // defaults to identity
Collector c1,
Function f2,  // defaults to identity
Collector c2,
BiFunction finisher) {
  return toBoth(mapping(f1, c1), mapping(f2, c2));
   }


You don't need these f1 and f2 as arguments, because we already have a 
Collectors.mapping(f1, c1) combinator. So you can write:


unzipping(
    mapping(f1, c1),
    mapping(f2, c2)
    finisher
)

But then unzipping is not really "unzipping".

What's wrong with my initial proposal of Collectors.toBoth()?

We already have some toXxx() methods (toList, toSet, toCollection, 
toMap, ...), so toBoth seems to me as a natural extension of that naming 
principle:


toBoth(
    toMap()
    toList()
    combiner
)

But I'm open to other naming suggestions.

I would also call the 3rd parameter 'combiner' rather than 'finisher', 
because finisher is known as particular function that is bound to the 
particular Collector. And this 3rd argument is not the resulting 
collector's finisher - it is just a part of it. The real finisher of the 
resulting Collector is composed of that function and finishers of 
underlying collectors.


Regards, Peter

Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.

[Paul Sandoz]

Hi Vivek,

Thanks for investigating this.

 164 public static int mismatch(boolean[] a,
 165boolean[] b,
 166int length) {
 167 int i = 0;
 168 if (a[i] != b[i])
 169 return i;
You might as well replace the use if i with 0 and i think you can move this to 
be performed when the length is greater than the threshold, that way you don’t 
impact small arrays below the threshold.

 186 public static int mismatch(boolean[] a, int aFromIndex,
 187boolean[] b, int bFromIndex,
 188int length) {
 189 int i = 0;
 190 if (a[i] != b[i])
 191 return i;
This is incorrect. You need to use aFromIndex and bFromIndex.

Do you run the test test/jdk/java/util/Arrays/ArraysEqCmpTest.java? If that 
passed then we need to strengthen for the case of a mismatch on the first 
relative element in each array.

Paul.

> On Jun 19, 2018, at 9:36 AM, Deshpande, Vivek R  
> wrote:
> 
> Thanks David.
> Sending it to core-libs-dev.
> 
> I would like to contribute a patch which improves the array comparison when 
> there is a mismatch for the first element.
> This avoids call to vectorizedMismatch method and gives ~80x speed up.
> Could you please review and sponsor the patch.
> Link to bug:
> https://bugs.openjdk.java.net/browse/JDK-8205194
> webrev:
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.00/
> 
> Regards,
> Vivek
> 
> -Original Message-
> From: David Holmes [mailto:david.hol...@oracle.com] 
> Sent: Monday, June 18, 2018 10:32 PM
> To: Deshpande, Vivek R ; jdk-...@openjdk.java.net
> Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a 
> mismatch at first element.
> 
> Hi Vivek,
> 
> Reviews should take place on the appropriate mailing list for the code being 
> changed, not on the jdk-dev list. Please takes this to core-libs-dev.
> 
> Thanks,
> David
> 
> On 19/06/2018 9:52 AM, Deshpande, Vivek R wrote:
>> Hi All
>> 
>> Forgot to add the links:
>> https://bugs.openjdk.java.net/browse/JDK-8205194
>> webrev:
>> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
>> 0/
>> 
>> Regards.
>> Vivek
>> 
>> From: Deshpande, Vivek R
>> Sent: Monday, June 18, 2018 4:50 PM
>> To: 'jdk-...@openjdk.java.net' 
>> Cc: 'Paul Sandoz' ; Viswanathan, Sandhya 
>> 
>> Subject: RFR(S): 8205194: Improve the Array Comparison when there is a 
>> mismatch at first element.
>> 
>> Hi All
>> 
>> I would like to contribute a patch which improves the array comparison when 
>> there is a mismatch for the first element.
>> This avoids call to vectorizedMismatch method and gives ~80x speed up.
>> Please review and sponsor the patch.
>> 
>> Regards,
>> Vivek
>> 

RE: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.

[Deshpande, Vivek R]

Hi Roger

I will also test with zero length arrays and let you know.
Thanks for the input.

Regards,
Vivek

From: Deshpande, Vivek R
Sent: Tuesday, June 19, 2018 10:17 AM
To: 'Paul Sandoz' 
Cc: David Holmes ; core-libs-dev@openjdk.java.net; 
Viswanathan, Sandhya 
Subject: RE: RFR(S): 8205194: Improve the Array Comparison when there is a 
mismatch at first element.

Thanks Paul for quick review. I will work on the things you have mentioned and 
get back soon.
I will also test this with test/jdk/java/util/Arrays/ArraysEqCmpTest.java.

Regards,
Vivek

From: Paul Sandoz [mailto:paul.san...@oracle.com]
Sent: Tuesday, June 19, 2018 9:55 AM
To: Deshpande, Vivek R 
mailto:vivek.r.deshpa...@intel.com>>
Cc: David Holmes mailto:david.hol...@oracle.com>>; 
core-libs-dev@openjdk.java.net; 
Viswanathan, Sandhya 
mailto:sandhya.viswanat...@intel.com>>
Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a 
mismatch at first element.

Hi Vivek,

Thanks for investigating this.


 164 public static int mismatch(boolean[] a,

 165boolean[] b,

 166int length) {

 167 int i = 0;

 168 if (a[i] != b[i])

 169 return i;
You might as well replace the use if i with 0 and i think you can move this to 
be performed when the length is greater than the threshold, that way you don’t 
impact small arrays below the threshold.


 186 public static int mismatch(boolean[] a, int aFromIndex,

 187boolean[] b, int bFromIndex,

 188int length) {

 189 int i = 0;

 190 if (a[i] != b[i])

 191 return i;
This is incorrect. You need to use aFromIndex and bFromIndex.

Do you run the test test/jdk/java/util/Arrays/ArraysEqCmpTest.java? If that 
passed then we need to strengthen for the case of a mismatch on the first 
relative element in each array.

Paul.

On Jun 19, 2018, at 9:36 AM, Deshpande, Vivek R 
mailto:vivek.r.deshpa...@intel.com>> wrote:

Thanks David.
Sending it to core-libs-dev.

I would like to contribute a patch which improves the array comparison when 
there is a mismatch for the first element.
This avoids call to vectorizedMismatch method and gives ~80x speed up.
Could you please review and sponsor the patch.
Link to bug:
https://bugs.openjdk.java.net/browse/JDK-8205194
webrev:
http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.00/

Regards,
Vivek

-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com]
Sent: Monday, June 18, 2018 10:32 PM
To: Deshpande, Vivek R 
mailto:vivek.r.deshpa...@intel.com>>; 
jdk-...@openjdk.java.net
Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a 
mismatch at first element.

Hi Vivek,

Reviews should take place on the appropriate mailing list for the code being 
changed, not on the jdk-dev list. Please takes this to core-libs-dev.

Thanks,
David

On 19/06/2018 9:52 AM, Deshpande, Vivek R wrote:
Hi All

Forgot to add the links:
https://bugs.openjdk.java.net/browse/JDK-8205194
webrev:
http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
0/

Regards.
Vivek

From: Deshpande, Vivek R
Sent: Monday, June 18, 2018 4:50 PM
To: 'jdk-...@openjdk.java.net' 
mailto:jdk-...@openjdk.java.net>>
Cc: 'Paul Sandoz' mailto:paul.san...@oracle.com>>; 
Viswanathan, Sandhya
mailto:sandhya.viswanat...@intel.com>>
Subject: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch 
at first element.

Hi All

I would like to contribute a patch which improves the array comparison when 
there is a mismatch for the first element.
This avoids call to vectorizedMismatch method and gives ~80x speed up.
Please review and sponsor the patch.

Regards,
Vivek

Re: Proposal: optimization of Map.copyOf and Collectors.toUnmodifiableMap

[Peter Levart]

Hi Stuart,

On 06/19/2018 04:09 AM, Stuart Marks wrote:



On 6/17/18 4:23 PM, Peter Levart wrote:
My attempt to optimize the Map.copyOf() was also using just public 
APIs (with the addition of an internal class).


Well, it does speed-up Map.copyOf() by 30%. But I agree there are 
other approaches that could go even further. In particular when all 
intermediary copies could be avoided.


Here's one approach (which still uses just public APIs) and avoids 
intermediary copying:


You mentioned "using just public APIs" a couple times. Are you viewing 
that as a constraint? I don't think it is.


More a desire than a constraint. If there really is a solution that is 
better but needs SharedSecrets, then I'm all for it. OTOH, it surely is 
less maintenance when only public APIs are used, isn't it?


I'd like to use Map.forEach() which is already optimized in most JDK 
internal implementations while the default implementation is not worse 
than alternatives.




http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/webrev.02/ 



Why choosing Map.forEach for dumping the content of the map instead 
of iteration over entrySet? Because in some Map implementations this 
is the most direct iteration without producing garbage (for example 
in IdentityHashMap), but in the worst case (default Map method for 
example) it is delegated to iteration over entrySet.


Sure, entrySet().toArray() does a lot of allocation and copying. Using 
forEach() can avoid this, but it doesn't necessarily preserve the 
right invariants. In particular, if the source is a concurrent map, 
the number of times forEach's BiConsumer is called might not match 
size(), if the map has changed size. So, the forEach approach has to 
deal with the possibility of it not being called exactly size() times, 
whereas we know that the array from toArray() can't change size (and 
that its contents are consistent, for some definition of consistent).


Right, and my last approach detected that and has thrown 
ConcurrentModificationException. Which might not be desired in a 
scenario where a live CHM is copied into an unmodifiable Map for example.




This change also publishes a reference to the Map under construction 
before its constructor has returned. I'm not sure of all the memory 
model implications of this.


It publishes a Builder object that has a controlled access to the 
constructing map. You can't get a reference to the constructing map from 
it and it becomes useless before the constructor of the map returns.




This change also hands out an object that has access to the new Map 
instance's internals.


Yes, this is the Builder object.

You're aware of this, of course, because you snapshot the current 
thread and null it out later, saying


    // prevent naughty Map implementations from modifying MapN after
    // it is fully constructed

So there are potential security vulnerabilities here, which requires 
some serious thought. What you have *seems* reasonable, but my 
experience is that things that are subtle but that seem reasonable 
actually end up having security holes.


This is not my invention. The same technique is used in 
ObjectInputStream / ObjectOutputStream.




I'm trying to think of some alternatives.

The issue with the forEach() approach on an arbitrary map is that you 
have to hand out a BiConsumer, and malicious code can call it even 
after forEach() has returned. What's necessary is to have a way to 
shut off the BiConsumer before reading out what it's accumulated. I 
don't know of a simple, fast, and correct way to do this. (Choose any 
two!) The "enabled" state (whether a Thread instance or just a 
boolean) will probably have to be a volatile that must be checked 
every time. What are the performance implications? Anyway, while it 
seems promising, the forEach() approach seems like it leads off into 
the weeds.


The technique used in my last webrev and taken from ObjectInputStream / 
ObjectOutputStream is simple, simple to prove correctness and fast as it 
needs zero synchronization. It could be called "a single-threaded 
object" idiom:


class SingleThreadedObject {
    private Thread thread;

    SingleThreadedObject() {
        thread = Thread.currentThread();
    }

    void use() {
        if (Thread.currentThread() != thread) throw new 
IllegalStateException();


        ... anything ...
    }

    void close() {
        if (Thread.currentThread() != thread) throw new 
IllegalStateException();

        thread = null;
    }
}


Claim: The thread that constructs the object is the only thread that can 
use() or close() the object and only until close() is called by the 
constructing thread. To prove the correctness of this claim, we have 
consider two scenarios:


- object is use()d and finally close()d in the same thread that 
constructed it - no need to prove anything here as it is obvious that 
this works by inspecting the code.


- object is attempted to be use()d or close()d in some other thread 

RE: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.

[Deshpande, Vivek R]

Thanks Paul for quick review. I will work on the things you have mentioned and 
get back soon.
I will also test this with test/jdk/java/util/Arrays/ArraysEqCmpTest.java.

Regards,
Vivek

From: Paul Sandoz [mailto:paul.san...@oracle.com]
Sent: Tuesday, June 19, 2018 9:55 AM
To: Deshpande, Vivek R 
Cc: David Holmes ; core-libs-dev@openjdk.java.net; 
Viswanathan, Sandhya 
Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a 
mismatch at first element.

Hi Vivek,

Thanks for investigating this.


 164 public static int mismatch(boolean[] a,

 165boolean[] b,

 166int length) {

 167 int i = 0;

 168 if (a[i] != b[i])

 169 return i;
You might as well replace the use if i with 0 and i think you can move this to 
be performed when the length is greater than the threshold, that way you don’t 
impact small arrays below the threshold.


 186 public static int mismatch(boolean[] a, int aFromIndex,

 187boolean[] b, int bFromIndex,

 188int length) {

 189 int i = 0;

 190 if (a[i] != b[i])

 191 return i;
This is incorrect. You need to use aFromIndex and bFromIndex.

Do you run the test test/jdk/java/util/Arrays/ArraysEqCmpTest.java? If that 
passed then we need to strengthen for the case of a mismatch on the first 
relative element in each array.

Paul.


On Jun 19, 2018, at 9:36 AM, Deshpande, Vivek R 
mailto:vivek.r.deshpa...@intel.com>> wrote:

Thanks David.
Sending it to core-libs-dev.

I would like to contribute a patch which improves the array comparison when 
there is a mismatch for the first element.
This avoids call to vectorizedMismatch method and gives ~80x speed up.
Could you please review and sponsor the patch.
Link to bug:
https://bugs.openjdk.java.net/browse/JDK-8205194
webrev:
http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.00/

Regards,
Vivek

-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com]
Sent: Monday, June 18, 2018 10:32 PM
To: Deshpande, Vivek R 
mailto:vivek.r.deshpa...@intel.com>>; 
jdk-...@openjdk.java.net
Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a 
mismatch at first element.

Hi Vivek,

Reviews should take place on the appropriate mailing list for the code being 
changed, not on the jdk-dev list. Please takes this to core-libs-dev.

Thanks,
David

On 19/06/2018 9:52 AM, Deshpande, Vivek R wrote:

Hi All

Forgot to add the links:
https://bugs.openjdk.java.net/browse/JDK-8205194
webrev:
http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
0/

Regards.
Vivek

From: Deshpande, Vivek R
Sent: Monday, June 18, 2018 4:50 PM
To: 'jdk-...@openjdk.java.net' 
mailto:jdk-...@openjdk.java.net>>
Cc: 'Paul Sandoz' mailto:paul.san...@oracle.com>>; 
Viswanathan, Sandhya
mailto:sandhya.viswanat...@intel.com>>
Subject: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch 
at first element.

Hi All

I would like to contribute a patch which improves the array comparison when 
there is a mismatch for the first element.
This avoids call to vectorizedMismatch method and gives ~80x speed up.
Please review and sponsor the patch.

Regards,
Vivek

Re: BiCollector

[Zheka Kozlov]

The function you propose is just a binary variant of mapping:

Collector mapping(
  Function mapper,
  Collector downstream);

(omitted '? super' for readability)

So, it is logical to use the name biMapping:

Collector biMapping(
  Function mapper1,
  Function mapper2,
  Collector downstream1,
  Collector downstream2,
  BiFunction finisher);


2018-06-19 7:38 GMT+07:00 John Rose :

> On Jun 18, 2018, at 2:29 PM, Brian Goetz  wrote:
> >
> > "bisecting" sounds like it sends half the elements to one collector and
> half to the other …
>
> The main bisection or splitting operation that's relevant to a stream is
> what
> a spliterator does, so this is a concern.
>
> Nobody has mentioned "unzipping" yet; this is a term of art which applies
> to streams
> of tuples.  The image of a zipper is relatively clear and unambiguous, and
> the tradition
> is pretty strong.  https://en.wikipedia.org/wiki/
> Convolution_(computer_science)
>
> The thing we are looking at differs in two ways from classic "unzipping":
> First, the
> two collectors themselves convert the same T elements to whatever internal
> value
> (T1, T2) is relevant.  Second, we are looking at a new terminal operation
> (a collector) which
> consolidates the results from both of streams (a notional Stream and
> Stream,
> if you like), rather than delivering the streams as a pair of outputs.
>
> The classic "unzip" operation applies "fst" and "snd" (or some other
> conventional
> set of access functions) to each T-element of the input stream.  Since we
> don't
> have a privileged 2-tuple type (like Pair) in Java, the user would
> need
> to nominate those two functions explicitly, either by folding them into a
> "mapping"
> on each collector, or as a utility overloading like this:
>
>unzipping(
> Function f1,  // defaults to identity
> Collector c1,
> Function f2,  // defaults to identity
> Collector c2,
> BiFunction finisher) {
>  return toBoth(mapping(f1, c1), mapping(f2, c2));
>   }
>
>
> > "tee" might be a candidate, though it doesn't follow the `ing
> convention.  "teeing" sounds dumb.
>
>
> "tee" sounds asymmetrical.  "diverting" or "detouring" are "*ing" words
> that might
> express asymmetrical disposition of derivative streams.
>
> An asymmetrical operation might be interesting if it could fork off a
> stream of
> its own.  It would have to have a side-effecting void-producing terminal
> operation,
> so the main (undiverted) stream could continue to progress at the top
> level of
> the expression.
>
> interface Stream {
>   default Stream diverting(Consumer> tee) { … }
> }
>
> values.stream().diverting(s2->s2.forEach(System.out::
> println)).filter(…).collect(…);
>
> Or (and this might be a sweet spot) a symmetric stream-tee operation could
> materialize two sibling streams and rejoin their results with a bifunction:
>
> class Collectors {
>   static  Stream unzipping(
> Function, R1> f1,
> Function, R2> f2,
> BiFunction finisher)
> { … }
> }
>
> values.stream().unzipping(
> s1->s1.forEach(System.out::println),
> s2->s2.filter(…).collect(…),
> (void1, r2)->r2
> );
>
> This would allow each "fork child" of the stream to continue to use the
> Stream API instead of the more restrictive Collector operators.
>
> Optimal code generation for forked/unzipped/teed streams would be tricky,
> requiring simultaneous loop control logic for each stream.
> To me that's a feature, not a bug, since hand-writing ad hoc
> simultaneous loops is a pain.
>
> My $0.02.
>
> — John

RFR 8195650 Method references to VarHandle accessors

[Paul Sandoz]

Hi,

Please review the following fix to ensure method references to VarHandle 
signature polymorphic methods are supported at runtime (specifically the method 
handle to a signature polymorphic method can be loaded from the constant pool):

  http://cr.openjdk.java.net/~psandoz/jdk/JDK-8195650-varhandle-mref/webrev/ 


I also added a “belts and braces” test to ensure a constant method handle to 
MethodHandle::invokeBasic cannot be loaded if outside of the j.l.invoke package.

Paul.

RFC: Add new JCA provider to support hardware RNGs

[Gustavo Romero]

Hi,

Please, could I get comments on the following change?

Since it's related to security, I would be glad if security
experts could also comment on that.

webrev: http://cr.openjdk.java.net/~gromero/POWER9/darn/v6_rebased/

It introduces a way to get benefits from hardware instructions in userspace
that can delivery a random number and so be used to speed up and avoid
blocking in some SecureRandom methods, notably generateSeed() and
nextBytes(), without loosing the random number quality. Particularly on
Power, the new POWER9 CPUs introduced a hardware instruction called 'darn'
that can be used for that purpose.

The main idea is to add a new JCA provider called "HWTRNG" (if no better
name is suggested), adding new helper methods that can then be intrinsified
and that will be used  by generateSeed() and nextBytes(). In that sense,
this change also adds the proper mechanisms in the Interpreter,
C1 Compiler, and C2 compiler (for PPC64, but also paving the way for other
platforms) to intrinsify these helper methods that will be used in the end
by generateSeed() and nextBytes() methods. The added helpers are:

  byte[] getRandomSequence0(int)
  byte[] getRandomSequence1(byte[])
  long validRandomLong(void)
  long randomLong(void)

The helpers also take care of checking if the returned random number is
valid and attempt 10 times (as recommended by ISA) get a valid random
number before falling back to a software alternative (HWTRNG is based
mostly on JCA provider NativePRNG and so the fall back mechanism will use
the exactly same methods found in NativePRNG and hence behave similarly.
Nonetheless, in my experience such a hardware failures (which are the main
cause of a returned invalid random number) are quite rare: in my tests I
was never able to exhaust the HW RNG and force it to generate an invalid
random number).

The user, besides having to specify explicitly the use of a non-default
provider (HWTRNG), will have to unlock the VM experimental options to
effectively use the hardware RNG as an unique random source by passing
options "-XX:+UnlockExperimentalVMOptions -XX:+UseRANDOMIntrinsics".

On Power, for the Interpreter and C1 Compiler, besides avoiding the
blocking effect of a low entropy on /dev/random, I was able to get about
126 Mbps (3x higher than the version without 'darn') on generaSeed() and
nextBytes(). The maximum theoretical value on a POWER9 machine is usually
128 Mbps.

I'll address the details about the file headers (Copyright, dates, authors,
versions, etc) after I get some feedback about this change.


Thanks in advance.

Best regards,
Gustavo

RFC: Add new JCA provider to support hardware RNGs

[Gustavo Romero]

Sorry for resending it. I missed a few MLs.

--

Hi,

Please, could I get comments on the following change?

Since it's related to security, I would be glad if security
experts could also comment on that.

webrev: http://cr.openjdk.java.net/~gromero/POWER9/darn/v6_rebased/

It introduces a way to get benefits from hardware instructions in userspace
that can delivery a random number and so be used to speed up and avoid
blocking in some SecureRandom methods, notably generateSeed() and
nextBytes(), without loosing the random number quality. Particularly on
Power, the new POWER9 CPUs introduced a hardware instruction called 'darn'
that can be used for that purpose.

The main idea is to add a new JCA provider called "HWTRNG" (if no better
name is suggested), adding new helper methods that can then be intrinsified
and that will be used  by generateSeed() and nextBytes(). In that sense,
this change also adds the proper mechanisms in the Interpreter,
C1 Compiler, and C2 compiler (for PPC64, but also paving the way for other
platforms) to intrinsify these helper methods that will be used in the end
by generateSeed() and nextBytes() methods. The added helpers are:

   byte[] getRandomSequence0(int)
   byte[] getRandomSequence1(byte[])
   long validRandomLong(void)
   long randomLong(void)

The helpers also take care of checking if the returned random number is
valid and attempt 10 times (as recommended by ISA) get a valid random
number before falling back to a software alternative (HWTRNG is based
mostly on JCA provider NativePRNG and so the fall back mechanism will use
the exactly same methods found in NativePRNG and hence behave similarly.
Nonetheless, in my experience such a hardware failures (which are the main
cause of a returned invalid random number) are quite rare: in my tests I
was never able to exhaust the HW RNG and force it to generate an invalid
random number).

The user, besides having to specify explicitly the use of a non-default
provider (HWTRNG), will have to unlock the VM experimental options to
effectively use the hardware RNG as an unique random source by passing
options "-XX:+UnlockExperimentalVMOptions -XX:+UseRANDOMIntrinsics".

On Power, for the Interpreter and C1 Compiler, besides avoiding the
blocking effect of a low entropy on /dev/random, I was able to get about
126 Mbps (3x higher than the version without 'darn') on generaSeed() and
nextBytes(). The maximum theoretical value on a POWER9 machine is usually
128 Mbps.

I'll address the details about the file headers (Copyright, dates, authors,
versions, etc) after I get some feedback about this change.


Thanks in advance.

Best regards,
Gustavo

Re: RFR: JDK-8202788: Explicitly reclaim cached thread-local direct buffers at thread exit

[David Lloyd]

It may be confusing (to some, I guess) but it is consistent: the
ThreadLocal subclass author has absolute control over how the value is
presented to the user.  Adding compute() or many of the other
suggested variants will break that guarantee, which seems like kind of
a big deal to me.

What about introducing a different thread local API that has more
modern behavior?  Maybe a new subclass of ThreadLocal?

On Mon, Jun 18, 2018 at 5:28 PM Martin Buchholz  wrote:
>
> yes, the proposed new methods would use nulls differently.  The original 
> get() behavior of creating a mapping was a mistake, inconsistent with Map.  
> Yes, it will cause confusion.  But it's already confusing.
>
> On Mon, Jun 18, 2018 at 1:45 PM, David Lloyd  wrote:
>>
>> On Mon, Jun 18, 2018 at 12:53 PM, Martin Buchholz  
>> wrote:
>> > On Mon, Jun 18, 2018 at 10:21 AM, Tony Printezis 
>> > wrote:
>> >
>> >> Martin,
>> >>
>> >> You are forgiven. :-)
>> >>
>> >> So, the (valid, I think) issue with getIfPresent(), as originally proposed
>> >> by Peter, was that if get() was overridden in the subclass to somehow
>> >> transform the returned value, getIfPresent() wouldn’t apply the same
>> >> transformation.
>> >>
>> >> Doesn’t your compute() method have essentially the same issue? Apart from
>> >> that, I personally like this proposal as I agree: one look-up is always
>> >> better than two.
>> >>
>> >>
>> > A non-prototype implementation would delegate compute into ThreadLocalMap
>> > itself, where there is no risk of overriding.
>>
>> I don't think overriding is the risk; the risk would be compute*
>> behaving inconsistently with regards to an overridden get*.
>>
>>
>> --
>> - DML
>
>


-- 
- DML

Re: BiCollector

[Zheka Kozlov]

I don't like `distributing` for the same reason as `bisecting`: for me, it
sounds like a Stream is giving each collector only a part of elements.

2018-06-19 19:44 GMT+07:00 Brian Goetz :

>
>
> collectingToBoth
>>
>
> This one is actually both evocative of what the method does, and in the
> spirit of the existing naming conventions (collectingAndThen.)
>
> An n-ary version could just be called `collectingTo`, where it is passed a
> varargs of Collector.  Could we get away with collectingTo for a binary
> version as well?  The existence of the "combiner" function might make that
> a stretch, but I prefer `collectingTo` to `collectingToBoth`.
>
>
> (I still like `distributing` too.)
>

Re: RFR JDK-8066709 Make some JDK system properties read only

[Roger Riggs]

Hi Brent,

On 6/18/2018 6:53 PM, Brent Christian wrote:

Hi, Roger

On 6/18/18 7:31 AM, Roger Riggs wrote:


The CSR and Webrev are updated.

webrev:
http://cr.openjdk.java.net/~rriggs/webrev-static-property-8066709


* src/java.base/share/classes/java/lang/System.java :

Should the @implNote with the list of cached properties be added 
everywhere the @apiNote is being added ?  Right now the @implNote is 
only added to getProperties().


The repetition was getting tiresome and the base of all the 
xxxProperties methods is getProperties.
 Joe suggested having one copy of the full information  and referring 
to that from the individual @apiNotes.


* src/java.base/share/classes/jdk/internal/util/StaticProperty.java :

Nit:

  45 private StaticProperty() {
  46
  47 }

Maybe put this all on one line?


Will do

Thanks, Roger



Otherwise, the change looks good to me.

-Brent

Re: BiCollector

[Brian Goetz]

It is "distributing" in the same sense as the distributive law:

    c*(a+b) = c*a + c*b

(Think of the two collectors as the "sum" of a collector, and 
"distributing" says that you can send the elements to the sum by sending 
all of the elements to each.)


That said, I agree that the less mathematically-inclined might be drawn 
to the plain-english meaning, which is more like an (imprecise) bisection.


On 6/19/2018 10:14 AM, Zheka Kozlov wrote:
I don't like `distributing` for the same reason as `bisecting`: for 
me, it sounds like a Stream is giving each collector only a part of 
elements.


2018-06-19 19:44 GMT+07:00 Brian Goetz >:




collectingToBoth


This one is actually both evocative of what the method does, and
in the spirit of the existing naming conventions (collectingAndThen.)

An n-ary version could just be called `collectingTo`, where it is
passed a varargs of Collector.  Could we get away with
collectingTo for a binary version as well?  The existence of the
"combiner" function might make that a stretch, but I prefer
`collectingTo` to `collectingToBoth`.


(I still like `distributing` too.)

Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.

[Roger Riggs]

Hi Vivek,

Do you need to concerned that the arrays are zero length?

Are there any tests that test the zero length case.
(I assume you ran the existing tests).

Thanks, Roger


On 6/18/2018 7:52 PM, Deshpande, Vivek R wrote:

Hi All

Forgot to add the links:
https://bugs.openjdk.java.net/browse/JDK-8205194
webrev:
http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.00/

Regards.
Vivek

From: Deshpande, Vivek R
Sent: Monday, June 18, 2018 4:50 PM
To: 'jdk-...@openjdk.java.net' 
Cc: 'Paul Sandoz' ; Viswanathan, Sandhya 

Subject: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch 
at first element.

Hi All

I would like to contribute a patch which improves the array comparison when 
there is a mismatch for the first element.
This avoids call to vectorizedMismatch method and gives ~80x speed up.
Please review and sponsor the patch.

Regards,
Vivek

RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

[Joe Wang]

Thanks Alan.  I created 8205058 to capture your suggestions. Please see 
below for more details.


On 6/14/18, 4:30 AM, Alan Bateman wrote:

On 12/06/2018 17:52, Joe Wang wrote:

Hi all,

It's been a while since the 1st round of reviews. Thank you all for 
the valuable comments and suggestions!  Note that Roger's last 
response went to nio-dev, but not core-libs-dev, I've therefore 
attached it below.


CSR [2]: the CSR is now approved. Note the write method has been 
changed to writeString.


Impl [3]: for performance reason and the different requirement from 
byte-string conversion for malformed/unmappable character handing, 
the implementation uses a specific method separate from the existing 
ones. Both Sherman and Alan preferred specific method than adding 
parameters to the APIs that might be error prone. Thanks Sherman for 
the code!


JMH benchmark tests showed the new read method performs better than 
an operation that reads the bytes and convert to string. The gains 
start to be significant even at quite reasonable sizes (< 10K).



[1] JBS: https://bugs.openjdk.java.net/browse/JDK-8201276
[2] CSR: https://bugs.openjdk.java.net/browse/JDK-8202055
[3] webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8201276/webrev/

Please review.

The javadoc looks good.

One part of the implementation that could be cleaner is the exception 
thrown for the malformed or unmappable cases. There are sub-classes of 
CharacterCodingException defined for this that would be clearer than 
an IOException with an IllegalArgumentException as cause.


Changed the internal APIs to throw CCE instead. In the same way as the 
previous changeset for 8201276, these methods are made specific for the 
use cases (though they are now for Files.read/writeString only) so that 
they are not mixed up with existing ones that may inadvertently affect 
other usages.


One thing to note is that MalformedInputException or 
UnmappableCharacterException will lose one piece of information in 
comparison to the existing IAE, that is where it happens (offset). 
Should there be an improvement in the future, we could consider add it 
back to this part of code.


A minor nit in Files is that you can import 
jdk.internal.misc.JavaLangAccess rather than repeating the fully 
qualified class name. You can also move the definition of JLA up to 
the top. There's also a few inconsistencies with the existing code 
that would be goof to fix too (indenting and line length issues mostly).


Moved JLA and others to the top. Fixed also the indentations (mostly 
method declarations) and a few long lines.


The test looks reasonable. In getData() then then "shouldn't happen" 
case should throw an exception as a NPE here might be tricky to 
diagnose there. Another nit is the sb field - can that be removed.


Fixed too.

JBS: https://bugs.openjdk.java.net/browse/JDK-8205058
webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev/


Regards,
Joe



-Alan

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[mandy chung]

The javadoc update looks good to me.

Mandy

On 6/19/18 9:56 PM, David Holmes wrote:
Some further adjustments to getNestMembers() was made. Everything 
updated in place.


Thanks,
David

On 20/06/2018 9:30 AM, David Holmes wrote:

Sorry another update is imminent ... stay tuned.

David

On 19/06/2018 2:41 PM, David Holmes wrote:
Discussions on the CSR request have led to further changes to the 
documentation involving nests and the new nest-related method. There 
are no semantic changes here just clearer explanations.


Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v7-incr/ 



(don't worry if you don't see a v6, it didn't really exist).

Full webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v7/


Specdiffs updated in place at: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/specs/


Summary of changes:

- The definition of a nest etc is moved to the class-level javadoc of 
java.lang.Class, along with some other edits provided by Alex Buckley 
to pave the way for future updates
- The nest-related methods are written in a more clear and consistent 
way


Thanks,
David

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[David Holmes]

Some further adjustments to getNestMembers() was made. Everything 
updated in place.


Thanks,
David

On 20/06/2018 9:30 AM, David Holmes wrote:

Sorry another update is imminent ... stay tuned.

David

On 19/06/2018 2:41 PM, David Holmes wrote:
Discussions on the CSR request have led to further changes to the 
documentation involving nests and the new nest-related method. There 
are no semantic changes here just clearer explanations.


Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v7-incr/ 



(don't worry if you don't see a v6, it didn't really exist).

Full webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v7/


Specdiffs updated in place at: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/specs/


Summary of changes:

- The definition of a nest etc is moved to the class-level javadoc of 
java.lang.Class, along with some other edits provided by Alex Buckley 
to pave the way for future updates

- The nest-related methods are written in a more clear and consistent way

Thanks,
David
-

On 12/06/2018 3:16 PM, David Holmes wrote:

Here is one further minor update from the CSR discussions:

http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v5-incr/src/java.base/share/classes/java/lang/Class.java.cdiff.html 



Thanks,
David

On 25/05/2018 3:52 PM, David Holmes wrote:
Here are the further minor updates so far in response to all the 
review comments.


Incremental corelibs webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3-incr/ 



Full corelibs webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3/

Change summary:

src/java.base/share/classes/jdk/internal/reflect/Reflection.java
- remove inaccurate pseudo-assertion comment

test/jdk/java/lang/reflect/Nestmates/SampleNest.java
- code cleanup: <> operator

test/jdk/java/lang/reflect/Nestmates/TestReflectionAPI.java
- code cleanup: streamify duplicate removals

test/jdk/java/lang/invoke/PrivateInterfaceCall.java
- use consistent @bug number

Thanks,
David

On 22/05/2018 8:15 PM, David Holmes wrote:

Here are the updates so far in response to all the review comments.

Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2-incr/ 



Full webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2/


Change summary:

src/java.base/share/classes/java/lang/Class.java
- getNesthost:
   - change "any error" -> "any linkage error" as runtime errors 
will propagate. [This needs ratifying by EG]
   - add clarification that primitive and array classes are not 
explicitly members of any nest and so form singleton nests

   - add clarification that all nestmates are in the same package
   - re-word @return text to exclude the "royal 'we'"
- fix javadoc cross references

---

Moved reflection API tests from 
test/hotspot/jtreg/runtime/Nestmates/reflectionAPI/ to 
test/jdk/java/lang/reflect/Nestmates/


---

java/lang/reflect/Nestmates/TestReflectionAPI.java

Run tests twice to show that failure reasons remain the same.

---

test/jdk/jdk/lambda/vm/InterfaceAccessFlagsTest.java

Disable test via annotation rather than commenting out.

---

src/java.base/share/classes/jdk/internal/reflect/Reflection.java

- Fix indent for nestmate access check.
- Remove unnecessary local variable

---

src/java.base/share/classes/sun/invoke/util/VerifyAccess.java

- Replace myassert with proper assert

---

Thanks,
David

On 15/05/2018 10:52 AM, David Holmes wrote:
This review is being spread across four groups: langtools, 
core-libs, hotspot and serviceability. This is the specific review 
thread for core-libs - webrev:


http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/ 



See below for full details - including annotated full webrev 
guiding the review.


The intent is to have JEP-181 targeted and integrated by the end 
of this month.


Thanks,
David
-

The nestmates project (JEP-181) introduces new classfile 
attributes to identify classes and interfaces in the same nest, so 
that the VM can perform access control based on those attributes 
and so allow direct private access between nestmates without 
requiring javac to generate synthetic accessor methods. These 
access control changes also extend to core reflection and the 
MethodHandle.Lookup contexts.


Direct private calls between nestmates requires a more general 
calling context than is permitted by invokespecial, and so the 
JVMS is updated to allow, and javac updated to use, invokevirtual 
and invokeinterface for private class and interface method calls 
respectively. These changed semantics also extend to MethodHandle 
findXXX operations.


At this time we are only concerned with static nest definitions, 
which map to a top-level class/interface as the nest-host and all 
its nested types as nest-members.


Please see the JEP for further details.

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

Re: BiCollector

[Dawid Weiss]

Not that it's an important factor but as a non-native English speaker
I like the simplest form of "toBoth()" best. I also doubt there will
ever be a "Both" class in the JDK to worry about, even if such
examples can be found in the wild [1].

Dawid

[1] https://github.com/search?q=filename%3ABoth.java


On Tue, Jun 19, 2018 at 9:52 AM, Peter Levart  wrote:
>
>
> On 06/19/18 09:43, Peter Levart wrote:
>>
>> We already have some toXxx() methods (toList, toSet, toCollection, toMap,
>> ...), so toBoth seems to me as a natural extension of that naming principle:
>
>
> Well, on a second thought, toList, toSet, etc... they all name a type that
> is a result of the collection (List, Set, etc.). But we don't have a type
> called Both (yet).
>
> So, please, continue with suggestions...
>
> Peter
>

Re: BiCollector

[Peter Levart]

On 06/19/18 09:43, Peter Levart wrote:
We already have some toXxx() methods (toList, toSet, toCollection, 
toMap, ...), so toBoth seems to me as a natural extension of that 
naming principle:


Well, on a second thought, toList, toSet, etc... they all name a type 
that is a result of the collection (List, Set, etc.). But we don't have 
a type called Both (yet).


So, please, continue with suggestions...

Peter

Re: BiCollector

[Kirk Pepperdine]

> On Jun 19, 2018, at 9:11 AM, Zheka Kozlov  wrote:
> 
> The function you propose is just a binary variant of mapping:
> 
> Collector mapping(
>  Function mapper,
>  Collector downstream);
> 
> (omitted '? super' for readability)
> 
> So, it is logical to use the name biMapping:
> 
> Collector biMapping(
>  Function mapper1,
>  Function mapper2,
>  Collector downstream1,
>  Collector downstream2,
>  BiFunction finisher);

+1


> 
> 
> 2018-06-19 7:38 GMT+07:00 John Rose :
> 
>> On Jun 18, 2018, at 2:29 PM, Brian Goetz  wrote:
>>> 
>>> "bisecting" sounds like it sends half the elements to one collector and
>> half to the other …
>> 
>> The main bisection or splitting operation that's relevant to a stream is
>> what
>> a spliterator does, so this is a concern.
>> 
>> Nobody has mentioned "unzipping" yet; this is a term of art which applies
>> to streams
>> of tuples.  The image of a zipper is relatively clear and unambiguous, and
>> the tradition
>> is pretty strong.  https://en.wikipedia.org/wiki/
>> Convolution_(computer_science)
>> 
>> The thing we are looking at differs in two ways from classic "unzipping":
>> First, the
>> two collectors themselves convert the same T elements to whatever internal
>> value
>> (T1, T2) is relevant.  Second, we are looking at a new terminal operation
>> (a collector) which
>> consolidates the results from both of streams (a notional Stream and
>> Stream,
>> if you like), rather than delivering the streams as a pair of outputs.
>> 
>> The classic "unzip" operation applies "fst" and "snd" (or some other
>> conventional
>> set of access functions) to each T-element of the input stream.  Since we
>> don't
>> have a privileged 2-tuple type (like Pair) in Java, the user would
>> need
>> to nominate those two functions explicitly, either by folding them into a
>> "mapping"
>> on each collector, or as a utility overloading like this:
>> 
>>   unzipping(
>>Function f1,  // defaults to identity
>>Collector c1,
>>Function f2,  // defaults to identity
>>Collector c2,
>>BiFunction finisher) {
>> return toBoth(mapping(f1, c1), mapping(f2, c2));
>>  }
>> 
>> 
>>> "tee" might be a candidate, though it doesn't follow the `ing
>> convention.  "teeing" sounds dumb.
>> 
>> 
>> "tee" sounds asymmetrical.  "diverting" or "detouring" are "*ing" words
>> that might
>> express asymmetrical disposition of derivative streams.
>> 
>> An asymmetrical operation might be interesting if it could fork off a
>> stream of
>> its own.  It would have to have a side-effecting void-producing terminal
>> operation,
>> so the main (undiverted) stream could continue to progress at the top
>> level of
>> the expression.
>> 
>> interface Stream {
>>  default Stream diverting(Consumer> tee) { … }
>> }
>> 
>> values.stream().diverting(s2->s2.forEach(System.out::
>> println)).filter(…).collect(…);
>> 
>> Or (and this might be a sweet spot) a symmetric stream-tee operation could
>> materialize two sibling streams and rejoin their results with a bifunction:
>> 
>> class Collectors {
>>  static  Stream unzipping(
>>Function, R1> f1,
>>Function, R2> f2,
>>BiFunction finisher)
>> { … }
>> }
>> 
>> values.stream().unzipping(
>>s1->s1.forEach(System.out::println),
>>s2->s2.filter(…).collect(…),
>>(void1, r2)->r2
>>);
>> 
>> This would allow each "fork child" of the stream to continue to use the
>> Stream API instead of the more restrictive Collector operators.
>> 
>> Optimal code generation for forked/unzipped/teed streams would be tricky,
>> requiring simultaneous loop control logic for each stream.
>> To me that's a feature, not a bug, since hand-writing ad hoc
>> simultaneous loops is a pain.
>> 
>> My $0.02.
>> 
>> — John

Re: BiCollector

[Zheka Kozlov]

I agree that f1 and f2 are unnecessary.

I also noticed that toBoth is a binary form of `collectingAndThen`:

public static Collector collectingAndThen(
 Collector downstream,
 Function finisher);

So what about `collectionToBothAndThen`?

public static Collector collectingToBothAndThen(
 Collector downstream1,
 Collector downstream2,
 BiFunction finisher);

Another options:

collecting2
collectingToBoth
biCollecting


2018-06-19 14:43 GMT+07:00 Peter Levart :

>
>
> On 06/19/18 02:38, John Rose wrote:
>
>> unzipping(
>> Function f1,  // defaults to identity
>> Collector c1,
>> Function f2,  // defaults to identity
>> Collector c2,
>> BiFunction finisher)
>> {
>>   return toBoth(mapping(f1, c1), mapping(f2, c2));
>>}
>>
>
> You don't need these f1 and f2 as arguments, because we already have a
> Collectors.mapping(f1, c1) combinator. So you can write:
>
> unzipping(
> mapping(f1, c1),
> mapping(f2, c2)
> finisher
> )
>
> But then unzipping is not really "unzipping".
>
> What's wrong with my initial proposal of Collectors.toBoth()?
>
> We already have some toXxx() methods (toList, toSet, toCollection, toMap,
> ...), so toBoth seems to me as a natural extension of that naming principle:
>
> toBoth(
> toMap()
> toList()
> combiner
> )
>
> But I'm open to other naming suggestions.
>
> I would also call the 3rd parameter 'combiner' rather than 'finisher',
> because finisher is known as particular function that is bound to the
> particular Collector. And this 3rd argument is not the resulting
> collector's finisher - it is just a part of it. The real finisher of the
> resulting Collector is composed of that function and finishers of
> underlying collectors.
>
> Regards, Peter
>
>

Re: BiCollector

[Peter Levart]

On 06/19/2018 12:21 AM, Brian Goetz wrote:

distributing?


More "replicating" than "distributing" (thinking of replicated vs. 
distributed caches for example).


Peter

Re: RFR 8199435 : Unsafe publication of java.util.Properties.map

[Peter Levart]

On 06/18/2018 06:53 PM, Claes Redestad wrote:
Plain write that follows in program a volatile write, can in theory 
float before the volatile write. So if you publish a Properties 
instance via data race (via plain write), the observer may still see 
uninitialized 'map' and 'defaults' fields.




Right

http://cr.openjdk.java.net/~redestad/8199435.01/

(Yes, using VarHandle.storeStoreFence would do the exact same thing, 
but is not usable from Properties as the VarHandle impl needs to read 
some system properties...)


/Claes 


This looks good to me. One might think that the same fence is needed in 
deserialization too (readHashtable), but ObjectInputStream already makes 
care of that (see ObjectInputStream.freeze() and the two calls in 
readObject() and readUnshared()), so this is fine now.


Regards, Peter

Re: BiCollector

[Brian Goetz]

We already have some toXxx() methods (toList, toSet, toCollection, 
toMap, ...), so toBoth seems to me as a natural extension of that 
naming principle:


For toXxx methods, the Xxx is terminal, and tied to the result type.  
toArray converts to an array; toList to a List; toCollection to a 
Collection.  This does not convert the elements to a Both, and there's 
no Both type.  So, I think the analogy falls apart.

Re: BiCollector

[Brian Goetz]

collectingToBoth


This one is actually both evocative of what the method does, and in the 
spirit of the existing naming conventions (collectingAndThen.)


An n-ary version could just be called `collectingTo`, where it is passed 
a varargs of Collector.  Could we get away with collectingTo for a 
binary version as well?  The existence of the "combiner" function might 
make that a stretch, but I prefer `collectingTo` to `collectingToBoth`.



(I still like `distributing` too.)

Re: BiCollector

[Brian Goetz]

No. The essence of `mapping` is: apply this function, then pass them 
down to something else.  In that case, the something else is secondary 
to the mapping.


The essence of this method is: split the stream into _two_ streams, so 
it can be operated on by two collectors.  Any mapping here is incidental.






On 6/19/2018 2:11 AM, Zheka Kozlov wrote:

The function you propose is just a binary variant of mapping:

Collector mapping(
   Function mapper,
   Collector downstream);

(omitted '? super' for readability)

So, it is logical to use the name biMapping:

Collector biMapping(
   Function mapper1,
   Function mapper2,
   Collector downstream1,
   Collector downstream2,
   BiFunction finisher);


2018-06-19 7:38 GMT+07:00 John Rose :


On Jun 18, 2018, at 2:29 PM, Brian Goetz  wrote:

"bisecting" sounds like it sends half the elements to one collector and

half to the other …

The main bisection or splitting operation that's relevant to a stream is
what
a spliterator does, so this is a concern.

Nobody has mentioned "unzipping" yet; this is a term of art which applies
to streams
of tuples.  The image of a zipper is relatively clear and unambiguous, and
the tradition
is pretty strong.  https://en.wikipedia.org/wiki/
Convolution_(computer_science)

The thing we are looking at differs in two ways from classic "unzipping":
First, the
two collectors themselves convert the same T elements to whatever internal
value
(T1, T2) is relevant.  Second, we are looking at a new terminal operation
(a collector) which
consolidates the results from both of streams (a notional Stream and
Stream,
if you like), rather than delivering the streams as a pair of outputs.

The classic "unzip" operation applies "fst" and "snd" (or some other
conventional
set of access functions) to each T-element of the input stream.  Since we
don't
have a privileged 2-tuple type (like Pair) in Java, the user would
need
to nominate those two functions explicitly, either by folding them into a
"mapping"
on each collector, or as a utility overloading like this:

unzipping(
 Function f1,  // defaults to identity
 Collector c1,
 Function f2,  // defaults to identity
 Collector c2,
 BiFunction finisher) {
  return toBoth(mapping(f1, c1), mapping(f2, c2));
   }



"tee" might be a candidate, though it doesn't follow the `ing

convention.  "teeing" sounds dumb.


"tee" sounds asymmetrical.  "diverting" or "detouring" are "*ing" words
that might
express asymmetrical disposition of derivative streams.

An asymmetrical operation might be interesting if it could fork off a
stream of
its own.  It would have to have a side-effecting void-producing terminal
operation,
so the main (undiverted) stream could continue to progress at the top
level of
the expression.

interface Stream {
   default Stream diverting(Consumer> tee) { … }
}

values.stream().diverting(s2->s2.forEach(System.out::
println)).filter(…).collect(…);

Or (and this might be a sweet spot) a symmetric stream-tee operation could
materialize two sibling streams and rejoin their results with a bifunction:

class Collectors {
   static  Stream unzipping(
 Function, R1> f1,
 Function, R2> f2,
 BiFunction finisher)
{ … }
}

values.stream().unzipping(
 s1->s1.forEach(System.out::println),
 s2->s2.filter(…).collect(…),
 (void1, r2)->r2
 );

This would allow each "fork child" of the stream to continue to use the
Stream API instead of the more restrictive Collector operators.

Optimal code generation for forked/unzipped/teed streams would be tricky,
requiring simultaneous loop control logic for each stream.
To me that's a feature, not a bug, since hand-writing ad hoc
simultaneous loops is a pain.

My $0.02.

— John