Re: BiCollector

2018-06-13 Thread Tagir Valeev
Hello!

In my StreamEx library I created a "pairing" collector which does similar
job, but allows user to decide how to combine the results of two
collectors. This adds more flexibility. The signature is like this:

public static  Collector pairing(
Collector c1,
Collector c2,
BiFunction finisher)

Having such collector, The proposed `toBoth(c1, c2)` can be implemented as
simple as `pairing(c1, c2, Map::entry)`. OTOH if somebody wants to use
their own Pair class, it would be `pairing(c1, c2, Pair::new)`.
Sometimes you don't need a pair, but can create compound result object
right here. E.g.:

Collector summing =
Collectors.reducing(BigDecimal.ZERO, BigDecimal::add);
Collector averaging =
pairing(summing, Collectors.counting(), (sum, count) ->
sum.divide(BigDecimal.valueOf(count),
RoundingMode.HALF_EVEN));

So locking to Map.Entry is entirely unnecessary.

With best regards,
Tagir Valeev.



On Thu, Jun 14, 2018 at 6:11 AM Brian Goetz  wrote:

> I really like how BiCollector can be private, so all we'd have to expose
> is toBoth(), and the arguments of toBoth() are just ordinary
> collectors.  So no new types or abstractions; just a multiplexing.
>
> The use of Map.Entry as a pair surrogate is unfortunate -- its
> definitely not an Entry -- though I understand why you did this. I'm not
> sure if this is fatal or not for a JDK method, but it's pretty bad*.
> (You could generalize to n-ary, returning a List or array (you can take
> a varargs of Collector), at the loss of sharp types for the results.)
>
>
> *Yes, I'm sure one can find precedent of this being done; this has no
> effect on whether it's bad.
>
> On 6/11/2018 8:39 AM, Peter Levart wrote:
> > Hi,
> >
> > Have you ever wanted to perform a collection of the same Stream into
> > two different targets using two Collectors? Say you wanted to collect
> > Map.Entry elements into two parallel lists, each of them containing
> > keys and values respectively. Or you wanted to collect elements into
> > groups by some key, but also count them at the same time? Currently
> > this is not possible to do with a single Stream. You have to create
> > two identical streams, so you end up passing Supplier to other
> > methods instead of bare Stream.
> >
> > I created a little utility Collector implementation that serves the
> > purpose quite well:
> >
> > /**
> >  * A {@link Collector} implementation taking two delegate Collector(s)
> > and producing result composed
> >  * of two results produced by delegating collectors, wrapped in {@link
> > Map.Entry} object.
> >  *
> >  * @param  the type of elements collected
> >  * @param  the type of 1st delegate collector collected result
> >  * @param  tye type of 2nd delegate collector collected result
> >  */
> > public class BiCollector implements Collector > Map.Entry, Map.Entry> {
> > private final Collector keyCollector;
> > private final Collector valCollector;
> >
> > @SuppressWarnings("unchecked")
> > public BiCollector(Collector keyCollector, Collector > ?, V> valCollector) {
> > this.keyCollector = (Collector)
> > Objects.requireNonNull(keyCollector);
> > this.valCollector = (Collector)
> > Objects.requireNonNull(valCollector);
> > }
> >
> > @Override
> > public Supplier> supplier() {
> > Supplier keySupplier = keyCollector.supplier();
> > Supplier valSupplier = valCollector.supplier();
> > return () -> new
> > AbstractMap.SimpleImmutableEntry<>(keySupplier.get(), valSupplier.get());
> > }
> >
> > @Override
> > public BiConsumer, T> accumulator() {
> > BiConsumer keyAccumulator =
> > keyCollector.accumulator();
> > BiConsumer valAccumulator =
> > valCollector.accumulator();
> > return (accumulation, t) -> {
> > keyAccumulator.accept(accumulation.getKey(), t);
> > valAccumulator.accept(accumulation.getValue(), t);
> > };
> > }
> >
> > @Override
> > public BinaryOperator> combiner() {
> > BinaryOperator keyCombiner = keyCollector.combiner();
> > BinaryOperator valCombiner = valCollector.combiner();
> > return (accumulation1, accumulation2) -> new
> > AbstractMap.SimpleImmutableEntry<>(
> > keyCombiner.apply(accumulation1.getKey(),
> > accumulation2.getKey()),
> > valCombiner.apply(accumulation1.getValue(),
> > accumulation2.getValue())
> > );
> > }
> >
> > @Override
> > public Function, Map.Entry>
> > finisher() {
> > Function keyFinisher = keyCollector.finisher();
> > Function valFinisher = valCollector.finisher();
> > return accumulation -> new AbstractMap.SimpleImmutableEntry<>(
> > keyFinisher.apply(accumulation.getKey()),
> > valFinisher.apply(accumulation.getValue())
> > );
> > }
> >
> > @Override
> > public Set characteristics() {
> > EnumSet intersection =
> > 

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

2018-06-13 Thread David Holmes

On 13/06/2018 4:08 PM, joe darcy wrote:

Hi David,

On 5/24/2018 10: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/



In Class.java,

3990 Class[] members = getNestMembers0();
3991 // Can't actually enable this due to bootstrapping issues
3992 // assert(members.length != 1 || members[0] == this); // 
expected invariant from VM


can these checks be enabled unconditionally without using the assert 
facility, throwing AssertionError or some other kind of error?


Of course - but we don't want to pay the price of always checking 
something that would indicate an error on the VM side. There's an 
equivalent assertion on the VM side.


Thanks,
David


Thanks,

-Joe


Re: RFR(XS): 8202329 [AIX] Fix codepage mappings for IBM-943 and Big5

2018-06-13 Thread Bhaktavatsal R Maram
Hi Thomas,

I've added testcase along with fix. Please find webrev at 
http://cr.openjdk.java.net/~aleonard/8202329/webrev.01/

Thanks,
Bhaktavatsal Reddy

  

-"core-libs-dev"  wrote: -
To: "Thomas Stüfe" 
From: "Bhaktavatsal R Maram" 
Sent by: "core-libs-dev" 
Date: 06/01/2018 01:34PM
Cc: Java Core Libs 
Subject: Re: RFR(XS): 8202329 [AIX] Fix codepage mappings for IBM-943 and Big5

Hi Thomas,

Sorry, I was on vacation. I will submit webrev with jtreg testcase.

Thanks,
Bhaktavatsal
 

  

-"Thomas Stüfe"  wrote: -
To: Bhaktavatsal R Maram 
From: "Thomas Stüfe" 
Date: 05/23/2018 12:32AM
Cc: Ichiroh Takiguchi , Volker Simonis 
, Java Core Libs 
Subject: Re: RFR(XS): 8202329 [AIX] Fix codepage mappings for IBM-943 and Big5

Hi Bhaktavatsal,

the fix is fine. Reviewed. Thanks a lot for your work!

Could you please (its okay in a future patch) add a regression test
for IBM943 and IBM943C, in the form of a jtreg test? I would like to
test conversions for both code pages (at least for the crucial
tilde/backslash code points).

Additionally, that when started on AIX with Ja_JP.IBM-943, we
correctly default to IBM943C.

As an example, here is an old test I wrote years ago. You won't be
able to use it verbatim, so it is just a suggestion. Feel free to do
it differently.

http://cr.openjdk.java.net/~stuefe/other/IBM943MappingTest.java

If you have not written jtreg tests before:

http://openjdk.java.net/jtreg/

Also, under /test are many many examples.

Best Regards, Thomas

On Mon, May 21, 2018 at 9:47 AM, Bhaktavatsal R Maram
 wrote:
> Hi Thomas,
>
> Is this fix ready to merge?
>
> Thanks,
> Bhaktavatsal
>
>
>
>
> -"Thomas Stüfe"  wrote: -
> To: Ichiroh Takiguchi , Bhaktavatsal R Maram 
> 
> From: "Thomas Stüfe" 
> Date: 05/11/2018 11:49AM
> Cc: Volker Simonis , Java Core Libs 
> 
> Subject: Re: RFR(XS): 8202329 [AIX] Fix codepage mappings for IBM-943 and Big5
>
> Hi,
>
> I'll test and review next week. We also have some in-house tests which
> I'd like to run.
>
> You IBM folks should really apply for authorship so that this
> contribution process gets streamlined. After all, if something breaks
> in this code, you want to be able to fix it, yes? So even if you do
> not contribute much else, more patches may be forthcoming.
>
> Of course I hope these are not your last contributions :)
>
> Best, Thomas
>
>
>
> On Fri, May 11, 2018 at 7:57 AM, Ichiroh Takiguchi
>  wrote:
>> Hi.
>>
>> I tested this fix on AIX.
>>
>> I got following results.
>> $ LANG=Ja_JP ~/jdk/bin/java PrintDefaultCharset
>> Ja_JP   x-IBM943C   IBM-943CIBM-943C
>> $ LANG=Ja_JP.IBM-943 ~/jdk/bin/java PrintDefaultCharset
>> Ja_JP.IBM-943   x-IBM943C   IBM-943CIBM-943C
>> $ LANG=Zh_TW ~/jdk/bin/java PrintDefaultCharset
>> Zh_TW   x-IBM950IBM-950 IBM-950
>> $ LANG=Zh_TW.big5 ~/jdk/bin/java PrintDefaultCharset
>> Zh_TW.big5  x-IBM950IBM-950 IBM-950
>>
>> Also I reviewed source code, it's fine
>>
>> Since this testing requires locale installation for Ja_JP and Zh_TW,
>> so it's not easy to test it...
>> (At least, I think bos.loc.pc.Ja_JP and bos.loc.iso.Zh_TW filesets are
>> required)
>>
>>
>> On 2018-05-02 18:32, Volker Simonis wrote:
>>>
>>> Hi Bhaktavatsal Reddy,
>>>
>>> your change looks good. I can sponsor it.
>>>
>>> Just waiting for a second review...
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>>
>>> On Mon, Apr 30, 2018 at 11:29 AM, Bhaktavatsal R Maram
>>>  wrote:

 Hi All,

 Please review the fix.

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

 Thanks,
 Bhaktavatsal Reddy

 -"core-libs-dev"  wrote:
 -
 To: Volker Simonis 
 From: "Bhaktavatsal R Maram"
 Sent by: "core-libs-dev"
 Date: 04/26/2018 09:31PM
 Cc: Java Core Libs 
 Subject: Re: [AIX] Fix codepage mappings in Java for IBM-943 and Big5

 Hi Volker,

 Thank you. I will address your review comments and send webrev for
 review.

 - Bhaktavatsal Reddy



 -Volker Simonis  wrote: -
 To: Bhaktavatsal R Maram 
 From: Volker Simonis 
 Date: 04/26/2018 09:12PM
 Cc: Java Core Libs 
 Subject: Re: [AIX] Fix codepage mappings in Java for IBM-943 and Big5

 Hi Bhaktavatsal Reddy,

 I've opened the following issue for this problem:

 8202329: [AIX] Fix codepage mappings for IBM-943 and Big5

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

 Looking at you fix, can you please replace the "#elif AIX" by "#ifdef
 AIX" and the original "#else" by "#ifdef __solaris__". The original
 else branch contains Solaris-only code anyway and it is an historical
 omission that there are still a lot of places in the code where "not
 Linux" implicitly means "Solaris", but that's often wrong.

 Regards,
 Volker


 On Thu, Apr 26, 2018 

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

2018-06-13 Thread Weijun Wang
Thanks a lot for the explanation. Everything looks OK to me now.

--Max

> On Jun 13, 2018, at 10:10 PM, Roger Riggs  wrote:
> 
> Hi Joe,
> 
> The CSR text is still in draft and got out of sync with the open review and 
> comments.
> 
> The updated apiNote clarifies that changes made through the System.*property* 
>  methods
> may not affect the value cached during initialization. 
> The implementation changes affect a very short list of properties.
> 
> The note on the methods is to raise awareness that individual properties may 
> have
> different behavior and may be unpredictable.
> 
> On 6/12/2018 10:10 PM, Weijun Wang wrote:
>> In fact, why is setting user.name and user.home always evil? If I only want 
>> to set them on the command line so that a special "user environment" is 
>> used, why is it a problem?
> There is no change to the ability to set properties on the command line.
> The concern is with the predictability of (some) properties changing 
> dynamically while the application 
> is running.
> 
> The property user.home is used for mime-types and mailcap files and user.name 
> is used in some
> networking cases where a username is needed.
>> 
>> In fact, we have a test setting user.home to an empty directory to avoid 
>> unexpected result because we cannot control the test runner's home directory.
>> 
> Right, there is no problem with that
> 
> Regards, Roger
>> 
>> Thanks
>> Max
>> 
>> 
>>> On Jun 13, 2018, at 9:59 AM, Weijun Wang 
>>>  wrote:
>>> 
>>> Hi Roger
>>> 
>>> 1. Should all occurrences of reading of these system properties be updated? 
>>> For example, the following one is not touched
>>>  
>>> http://hg.openjdk.java.net/jdk/jdk/file/4d2e3f5abb48/src/java.base/share/classes/sun/security/tools/keytool/Main.java#l842
> That usage was in a tool and I did not modify any tools.
>>> 
>>> 2. I assume that with this change not only there is no use calling 
>>> System.setProperty() in the application but also setting it on the command 
>>> line is now useless. Is this right? Do we need to make this clear in the 
>>> CSR?
>>> 
> No change to command line setting of properties.  
> 
> Roger
> 
>>> 
>>> Thanks
>>> Max
>>> 
>>> 
>>> 
 On Jun 4, 2018, at 9:32 PM, Roger Riggs 
  wrote:
 
 Please review a change to make the values of java.home, user.home, 
 user.dir, and user.name
 effectively read-only for internal use.  The values are cached during 
 initialization and the
 cached values are used.
 
 Webrev:
  
 http://cr.openjdk.java.net/~rriggs/webrev-static-property-8066709/
 
 
 Issue:
  
 https://bugs.openjdk.java.net/browse/JDK-8066709
 
 
 CSR:
  
 https://bugs.openjdk.java.net/browse/JDK-8204235
 
 
 Thanks, Roger
 
 
 
> 



Re: JDK 11 RFR of JDK-8205003: Replace selected link tags with linkplain in java.lang.Class

2018-06-13 Thread Lance Andersen
+1

--

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Sent from my iPhone

> On Jun 13, 2018, at 8:51 PM, joe darcy  wrote:
> 
> Hello,
> 
> Please review the small patch below to address
> 
> JDK-8205003: Replace selected link tags with linkplain in java.lang.Class
> 
> Thanks,
> 
> -Joe
> 
> diff -r 0742a087710e src/java.base/share/classes/java/lang/Class.java
> --- a/src/java.base/share/classes/java/lang/Class.javaWed Jun 13 13:12:50 
> 2018 -0700
> +++ b/src/java.base/share/classes/java/lang/Class.javaWed Jun 13 17:50:12 
> 2018 -0700
> @@ -820,7 +820,7 @@
>   * primitive type or void, then the {@code Module} object for the
>   * {@code java.base} module is returned.
>   *
> - * If this class is in an unnamed module then the {@link
> + * If this class is in an unnamed module then the {@linkplain
>   * ClassLoader#getUnnamedModule() unnamed} {@code Module} of the class
>   * loader for this class is returned.
>   *
> @@ -953,14 +953,14 @@
>   * empty string if the class is in an unnamed package.
>   *
>   *  If this class is a member class, then this method is equivalent to
> - * invoking {@code getPackageName()} on the {@link #getEnclosingClass
> + * invoking {@code getPackageName()} on the {@linkplain 
> #getEnclosingClass
>   * enclosing class}.
>   *
> - *  If this class is a {@link #isLocalClass local class} or an {@link
> + *  If this class is a {@linkplain #isLocalClass local class} or an 
> {@linkplain
>   * #isAnonymousClass() anonymous class}, then this method is equivalent 
> to
> - * invoking {@code getPackageName()} on the {@link #getDeclaringClass
> - * declaring class} of the {@link #getEnclosingMethod enclosing method} 
> or
> - * {@link #getEnclosingConstructor enclosing constructor}.
> + * invoking {@code getPackageName()} on the {@linkplain 
> #getDeclaringClass
> + * declaring class} of the {@linkplain #getEnclosingMethod enclosing 
> method} or
> + * {@linkplain #getEnclosingConstructor enclosing constructor}.
>   *
>   *  If this class represents an array type then this method returns 
> the
>   * package name of the element type. If this class represents a primitive
> @@ -2576,7 +2576,7 @@
>   * @param  name name of the desired resource
>   * @return  A {@link java.io.InputStream} object; {@code null} if no
>   *  resource with this name is found, the resource is in a 
> package
> - *  that is not {@link Module#isOpen(String, Module) open} to at
> + *  that is not {@linkplain Module#isOpen(String, Module) open} 
> to at
>   *  least the caller module, or access to the resource is denied
>   *  by the security manager.
>   * @throws  NullPointerException If {@code name} is {@code null}
> @@ -2675,7 +2675,7 @@
>   * @return A {@link java.net.URL} object; {@code null} if no resource 
> with
>   * this name is found, the resource cannot be located by a URL, 
> the
>   * resource is in a package that is not
> - * {@link Module#isOpen(String, Module) open} to at least the 
> caller
> + * {@linkplain Module#isOpen(String, Module) open} to at least 
> the caller
>   * module, or access to the resource is denied by the security
>   * manager.
>   * @throws NullPointerException If {@code name} is {@code null}
> 


Re: JDK 11 RFR of JDK-8205003: Replace selected link tags with linkplain in java.lang.Class

2018-06-13 Thread mandy chung

+1

Mandy

On 6/13/18 5:51 PM, joe darcy wrote:

Hello,

Please review the small patch below to address

     JDK-8205003: Replace selected link tags with linkplain in 
java.lang.Class


Thanks,

-Joe

diff -r 0742a087710e src/java.base/share/classes/java/lang/Class.java
--- a/src/java.base/share/classes/java/lang/Class.java    Wed Jun 13 
13:12:50 2018 -0700
+++ b/src/java.base/share/classes/java/lang/Class.java    Wed Jun 13 
17:50:12 2018 -0700

@@ -820,7 +820,7 @@
   * primitive type or void, then the {@code Module} object for the
   * {@code java.base} module is returned.
   *
- * If this class is in an unnamed module then the {@link
+ * If this class is in an unnamed module then the {@linkplain
   * ClassLoader#getUnnamedModule() unnamed} {@code Module} of the 
class

   * loader for this class is returned.
   *
@@ -953,14 +953,14 @@
   * empty string if the class is in an unnamed package.
   *
   *  If this class is a member class, then this method is 
equivalent to

- * invoking {@code getPackageName()} on the {@link #getEnclosingClass
+ * invoking {@code getPackageName()} on the {@linkplain 
#getEnclosingClass

   * enclosing class}.
   *
- *  If this class is a {@link #isLocalClass local class} or an 
{@link
+ *  If this class is a {@linkplain #isLocalClass local class} or 
an {@linkplain
   * #isAnonymousClass() anonymous class}, then this method is 
equivalent to

- * invoking {@code getPackageName()} on the {@link #getDeclaringClass
- * declaring class} of the {@link #getEnclosingMethod enclosing 
method} or

- * {@link #getEnclosingConstructor enclosing constructor}.
+ * invoking {@code getPackageName()} on the {@linkplain 
#getDeclaringClass
+ * declaring class} of the {@linkplain #getEnclosingMethod 
enclosing method} or

+ * {@linkplain #getEnclosingConstructor enclosing constructor}.
   *
   *  If this class represents an array type then this method 
returns the
   * package name of the element type. If this class represents a 
primitive

@@ -2576,7 +2576,7 @@
   * @param  name name of the desired resource
   * @return  A {@link java.io.InputStream} object; {@code null} if no
   *  resource with this name is found, the resource is in a 
package
- *  that is not {@link Module#isOpen(String, Module) open} 
to at
+ *  that is not {@linkplain Module#isOpen(String, Module) 
open} to at
   *  least the caller module, or access to the resource is 
denied

   *  by the security manager.
   * @throws  NullPointerException If {@code name} is {@code null}
@@ -2675,7 +2675,7 @@
   * @return A {@link java.net.URL} object; {@code null} if no 
resource with
   * this name is found, the resource cannot be located by a 
URL, the

   * resource is in a package that is not
- * {@link Module#isOpen(String, Module) open} to at least 
the caller
+ * {@linkplain Module#isOpen(String, Module) open} to at 
least the caller
   * module, or access to the resource is denied by the 
security

   * manager.
   * @throws NullPointerException If {@code name} is {@code null}



Re: JDK 11 RFR of JDK-8205003: Replace selected link tags with linkplain in java.lang.Class

2018-06-13 Thread Brian Burkhalter
Hi Joe,

+1

Brian

On Jun 13, 2018, at 5:51 PM, joe darcy  wrote:

> Please review the small patch below to address
> 
> JDK-8205003: Replace selected link tags with linkplain in java.lang.Class


JDK 11 RFR of JDK-8205003: Replace selected link tags with linkplain in java.lang.Class

2018-06-13 Thread joe darcy

Hello,

Please review the small patch below to address

    JDK-8205003: Replace selected link tags with linkplain in 
java.lang.Class


Thanks,

-Joe

diff -r 0742a087710e src/java.base/share/classes/java/lang/Class.java
--- a/src/java.base/share/classes/java/lang/Class.java    Wed Jun 13 
13:12:50 2018 -0700
+++ b/src/java.base/share/classes/java/lang/Class.java    Wed Jun 13 
17:50:12 2018 -0700

@@ -820,7 +820,7 @@
  * primitive type or void, then the {@code Module} object for the
  * {@code java.base} module is returned.
  *
- * If this class is in an unnamed module then the {@link
+ * If this class is in an unnamed module then the {@linkplain
  * ClassLoader#getUnnamedModule() unnamed} {@code Module} of the class
  * loader for this class is returned.
  *
@@ -953,14 +953,14 @@
  * empty string if the class is in an unnamed package.
  *
  *  If this class is a member class, then this method is 
equivalent to

- * invoking {@code getPackageName()} on the {@link #getEnclosingClass
+ * invoking {@code getPackageName()} on the {@linkplain 
#getEnclosingClass

  * enclosing class}.
  *
- *  If this class is a {@link #isLocalClass local class} or an 
{@link
+ *  If this class is a {@linkplain #isLocalClass local class} or 
an {@linkplain
  * #isAnonymousClass() anonymous class}, then this method is 
equivalent to

- * invoking {@code getPackageName()} on the {@link #getDeclaringClass
- * declaring class} of the {@link #getEnclosingMethod enclosing 
method} or

- * {@link #getEnclosingConstructor enclosing constructor}.
+ * invoking {@code getPackageName()} on the {@linkplain 
#getDeclaringClass
+ * declaring class} of the {@linkplain #getEnclosingMethod 
enclosing method} or

+ * {@linkplain #getEnclosingConstructor enclosing constructor}.
  *
  *  If this class represents an array type then this method 
returns the
  * package name of the element type. If this class represents a 
primitive

@@ -2576,7 +2576,7 @@
  * @param  name name of the desired resource
  * @return  A {@link java.io.InputStream} object; {@code null} if no
  *  resource with this name is found, the resource is in a 
package
- *  that is not {@link Module#isOpen(String, Module) open} 
to at
+ *  that is not {@linkplain Module#isOpen(String, Module) 
open} to at
  *  least the caller module, or access to the resource is 
denied

  *  by the security manager.
  * @throws  NullPointerException If {@code name} is {@code null}
@@ -2675,7 +2675,7 @@
  * @return A {@link java.net.URL} object; {@code null} if no 
resource with
  * this name is found, the resource cannot be located by a 
URL, the

  * resource is in a package that is not
- * {@link Module#isOpen(String, Module) open} to at least 
the caller
+ * {@linkplain Module#isOpen(String, Module) open} to at 
least the caller

  * module, or access to the resource is denied by the security
  * manager.
  * @throws NullPointerException If {@code name} is {@code null}



Re: BiCollector

2018-06-13 Thread Brian Goetz
I really like how BiCollector can be private, so all we'd have to expose 
is toBoth(), and the arguments of toBoth() are just ordinary 
collectors.  So no new types or abstractions; just a multiplexing.


The use of Map.Entry as a pair surrogate is unfortunate -- its 
definitely not an Entry -- though I understand why you did this. I'm not 
sure if this is fatal or not for a JDK method, but it's pretty bad*.  
(You could generalize to n-ary, returning a List or array (you can take 
a varargs of Collector), at the loss of sharp types for the results.)



*Yes, I'm sure one can find precedent of this being done; this has no 
effect on whether it's bad.


On 6/11/2018 8:39 AM, Peter Levart wrote:

Hi,

Have you ever wanted to perform a collection of the same Stream into 
two different targets using two Collectors? Say you wanted to collect 
Map.Entry elements into two parallel lists, each of them containing 
keys and values respectively. Or you wanted to collect elements into  
groups by some key, but also count them at the same time? Currently 
this is not possible to do with a single Stream. You have to create 
two identical streams, so you end up passing Supplier to other 
methods instead of bare Stream.


I created a little utility Collector implementation that serves the 
purpose quite well:


/**
 * A {@link Collector} implementation taking two delegate Collector(s) 
and producing result composed
 * of two results produced by delegating collectors, wrapped in {@link 
Map.Entry} object.

 *
 * @param  the type of elements collected
 * @param  the type of 1st delegate collector collected result
 * @param  tye type of 2nd delegate collector collected result
 */
public class BiCollector implements CollectorMap.Entry, Map.Entry> {

    private final Collector keyCollector;
    private final Collector valCollector;

    @SuppressWarnings("unchecked")
    public BiCollector(Collector keyCollector, Collector?, V> valCollector) {
    this.keyCollector = (Collector) 
Objects.requireNonNull(keyCollector);
    this.valCollector = (Collector) 
Objects.requireNonNull(valCollector);

    }

    @Override
    public Supplier> supplier() {
    Supplier keySupplier = keyCollector.supplier();
    Supplier valSupplier = valCollector.supplier();
    return () -> new 
AbstractMap.SimpleImmutableEntry<>(keySupplier.get(), valSupplier.get());

    }

    @Override
    public BiConsumer, T> accumulator() {
    BiConsumer keyAccumulator = 
keyCollector.accumulator();
    BiConsumer valAccumulator = 
valCollector.accumulator();

    return (accumulation, t) -> {
    keyAccumulator.accept(accumulation.getKey(), t);
    valAccumulator.accept(accumulation.getValue(), t);
    };
    }

    @Override
    public BinaryOperator> combiner() {
    BinaryOperator keyCombiner = keyCollector.combiner();
    BinaryOperator valCombiner = valCollector.combiner();
    return (accumulation1, accumulation2) -> new 
AbstractMap.SimpleImmutableEntry<>(
    keyCombiner.apply(accumulation1.getKey(), 
accumulation2.getKey()),
    valCombiner.apply(accumulation1.getValue(), 
accumulation2.getValue())

    );
    }

    @Override
    public Function, Map.Entry> 
finisher() {

    Function keyFinisher = keyCollector.finisher();
    Function valFinisher = valCollector.finisher();
    return accumulation -> new AbstractMap.SimpleImmutableEntry<>(
    keyFinisher.apply(accumulation.getKey()),
    valFinisher.apply(accumulation.getValue())
    );
    }

    @Override
    public Set characteristics() {
    EnumSet intersection = 
EnumSet.copyOf(keyCollector.characteristics());

    intersection.retainAll(valCollector.characteristics());
    return intersection;
    }
}


Do you think this class is general enough to be part of standard 
Collectors repertoire?


For example, accessed via factory method Collectors.toBoth(Collector 
coll1, Collector coll2), bi-collection could then be coded simply as:


    Map map = ...

    Map.Entry, List> keys_values =
    map.entrySet()
   .stream()
   .collect(
   toBoth(
   mapping(Map.Entry::getKey, toList()),
   mapping(Map.Entry::getValue, toList())
   )
   );


    Map.Entry, Long> histogram_count =
    ThreadLocalRandom
    .current()
    .ints(100, 0, 10)
    .boxed()
    .collect(
    toBoth(
    groupingBy(Function.identity(), counting()),
    counting()
    )
    );


Regards, Peter





Re: RFR JDK-8204930 Reader:nullReader() spec does not match the behavior

2018-06-13 Thread Brian Burkhalter
Hi Roger,

Thanks for pointing this out: simpler and cleaner.

Brian

On Jun 13, 2018, at 2:06 PM, Roger Riggs  wrote:

> For the CSR, the easiest path to clarify the spec is to withdraw the CSR, 
> update the spec,
> and add a note on the revised behavior.
> 
> Then finalize the CSR again.  That's enough to get it reviewed and approved.
> 
> (Using a new CSR would just spread the behavior over two CSRs).



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

2018-06-13 Thread mandy chung
Thanks for the explanation, Joe.  I see the subtle difference on these 
terminologies.


I'm okay with this patch.

I like the other option better to remove @apiNote since the spec states 
that the duplicates may not be removed.


Mandy

On 6/12/18 5:49 PM, Joseph D. Darcy wrote:

On 6/11/2018 10:38 PM, mandy chung wrote:



On 6/11/18 10: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 




"This implementation" is fine, as used in many @implNote.  Any reason 
why it has to be changed to "reference implementation"?




To summarize the concern there, the phrase "This implementation..." when 
used elsewhere has a different meaning.


Often the phrasing "This implementation..." or the more commonly used 
"The default implementation..." is used for text that is part of the 
contract of a method that can be overridden; that is, used to separate 
out the contract that is independent of which class or interface 
provides the implementation from the contract of a particular 
implementation.


One example from an API I work on occurs for the method 
javax.lang.model.element.ElementVisitor.visitModule. The default method 
defined in an interface states "The default implementation visits a 
ModuleElement by calling visitUnknown..." and then various visitor 
classes define their own behavior for this method while still being able 
to @inheritDoc the general "visit a module element" contract of the 
visitModule method.


However, java.lang.Class is final so for a particular JDK version there 
is only one implementation of the method in question. In that context 
"This implementation" doesn't mean "the implementation in this 
particular class or interface as opposed to the implementation in an a 
more specific subtype" it means "the implemetnation for the final method 
used in a particular JDK release."


I think using the term "This implementation" in the latter context is 
misleading so I suggested the alternative wording David sent out for 
review.


HTH,

-Joe


Re: RFR JDK-8204930 Reader:nullReader() spec does not match the behavior

2018-06-13 Thread Roger Riggs

Hi Patrick,

Yes, looks good to me too.

For the CSR, the easiest path to clarify the spec is to withdraw the 
CSR, update the spec,

and add a note on the revised behavior.

Then finalize the CSR again.  That's enough to get it reviewed and approved.

(Using a new CSR would just spread the behavior over two CSRs).

Thanks, Roger


On 6/13/18 4:56 PM, Brian Burkhalter wrote:

Hi Patrick,

Not part of your change, but I noticed that at line 66 of Reader.java there is 
an extra parenthesis after ready().

In the test, the bug ID at line 39 could simply be appended to line 38.

Otherwise looks good although I suppose given the specification update you’ll 
need an approved CSR before checking it in.

Thanks,

Brian

On Jun 13, 2018, at 1:22 PM, Patrick Reinhart  wrote:


While looking into the current Reader Spec and the failing methods I found that 
the ready() method actually does behave wrong and I fixed this.

For the case of mark() I would like to revise the specification to align with 
the Reader’s default behavior that states for the mark method:

IOException - If the stream does not support mark(), or if some other I/O error 
occurs

The new spec for those methods would then read like:

70 *  The {@code markSupported()} method returns {@code false}.  The
71 * {@code mark()} and {@code reset()} methods throw an {@code 
IOException}.


Here the link to the webrev:

http://cr.openjdk.java.net/~reinhapa/reviews/8204930/webrev




Re: RFR JDK-8204930 Reader:nullReader() spec does not match the behavior

2018-06-13 Thread Brian Burkhalter
Hi Patrick,

Not part of your change, but I noticed that at line 66 of Reader.java there is 
an extra parenthesis after ready().

In the test, the bug ID at line 39 could simply be appended to line 38.

Otherwise looks good although I suppose given the specification update you’ll 
need an approved CSR before checking it in.

Thanks,

Brian

On Jun 13, 2018, at 1:22 PM, Patrick Reinhart  wrote:

> While looking into the current Reader Spec and the failing methods I found 
> that the ready() method actually does behave wrong and I fixed this.
> 
> For the case of mark() I would like to revise the specification to align with 
> the Reader’s default behavior that states for the mark method:
> 
> IOException - If the stream does not support mark(), or if some other I/O 
> error occurs
> 
> The new spec for those methods would then read like:
> 
> 70 *  The {@code markSupported()} method returns {@code false}.  The
> 71 * {@code mark()} and {@code reset()} methods throw an {@code 
> IOException}.
> 
> 
> Here the link to the webrev:
> 
> http://cr.openjdk.java.net/~reinhapa/reviews/8204930/webrev



RFR JDK-8204930 Reader:nullReader() spec does not match the behavior

2018-06-13 Thread Patrick Reinhart
Hi everybody,

While looking into the current Reader Spec and the failing methods I found that 
the ready() method actually does behave wrong and I fixed this.

For the case of mark() I would like to revise the specification to align with 
the Reader’s default behavior that states for the mark method:

IOException - If the stream does not support mark(), or if some other I/O error 
occurs

The new spec for those methods would then read like:

70 *  The {@code markSupported()} method returns {@code false}.  The
71 * {@code mark()} and {@code reset()} methods throw an {@code 
IOException}.


Here the link to the webrev:

http://cr.openjdk.java.net/~reinhapa/reviews/8204930/webrev


-Patrick


Re: RFR: JDK-8204172 Predicate::not should explicitly mention "NullPointerException - if target is null"

2018-06-13 Thread Daniel Fuchs

Hi Jim,

Looks good to me.

best regards,

-- daniel

On 13/06/18 20:36, Jim Laskey wrote:

CSR: https://bugs.openjdk.java.net/browse/JDK-8204959
webrev: http://cr.openjdk.java.net/~jlaskey/8204172/webrev/index.html

Thank you,

— Jim





Re: RFR: JDK-8204973: Add build support for filtering translations

2018-06-13 Thread Daniel Fuchs

Hi Erik,

The modifications to the logging test look good to me.
Caveat: I don't speak chinese nor japanese ;-)

cheers,

-- daniel

On 13/06/18 20:47, Erik Joelsson wrote:

Hello,

Oracle will reduce the number of languages that it maintains 
translations of JDK resources for. The current translations will remain 
in the source for now, but we need a way to filter out a set of 
translations at build time so that we only include the ones we support. 
This patch adds such a configuration option. It also changes how Oracle 
builds by using the option to exclude all translations except English, 
Japanese, Simplified Chinese and Traditional Chinese. Anyone else 
building OpenJDK will by default include all translations present in the 
source, just as before.


I added a test that verifies this for builds with the "IMPLEMENTOR" 
field in the release file set to "Oracle Corporation". The test will not 
be run for other OpenJDK builds.


I had to modify an existing test for java.logging which used various 
translations to verify localized log messages to only use translations 
that Oracle chooses to include.


Since this is the second test that specifically verifies build behavior, 
I moved the previous such test together with this new test into a common 
top level test directory "build", under the jdk test root. I put these 
tests in the jdk tier3 test group.


I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically 
looked for tests that use the java.util.Locale class and ran them locally.


Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html

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

/Erik





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

2018-06-13 Thread Joe Wang

Thanks Roger!

I pushed the changeset after s/unmappble/unmappable, it found three of 
them :-)


Best,
Joe

On 6/12/18, 10:37 AM, Roger Riggs wrote:

Hi Joe,

Looks good to me.

Typo: StringCoding.java:1026 "unmappble"  (no new webrev needed)

Regards, Roger


On 6/12/2018 12:52 PM, 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.

Thanks,
Joe

On 5/15/18, 7:51 AM, Roger Riggs wrote:

Hi Joe, et. al.

My $.02 on line separators:

 - We should avoid clever tricks trying to solve problems that are 
infrequent such as cross OS file line operations.
Most files will be read/written on a single system so the line 
separators will be as expected.


 - Have/use APIs that split lines consistently accepting both line 
separators so developer code can be agnostic to line separators.
aka BufferedReader.readLine for developers that are processing 
the contents *as lines*.
   Those other methods already exist; if there are any gaps in line 
oriented processing that's a separate task.


 - These new file methods are defined to handle Charset 
encoding/decoding and buffering.
Since there are other methods to deal with files as lines these 
methods should not look for or break to lines.


 - Performance: adding code to look for line characters will slow it 
down and in most cases would have no impact

since the line endings will match the system.

 - The strings passed to writeString (CharSequence) should have been 
constructed using the proper line separators.
There are any number of methods that insert the os specific line 
terminator and its easy to write File.separator as needed.


As for the method naming:

I'd prefer "writeString" as a familiar term that isn't stretched too 
far by making the argument be a CharSequence.

its fine to call a CharSequence a string (with a lower case s).

$.02, Roger


On 4/27/18 6:33 PM, Joe Wang wrote:



On 4/27/2018 12:50 PM, fo...@univ-mlv.fr wrote:

- Mail original -

De: "Joe Wang" 
À: "Remi Forax" , "Alan Bateman" 

Cc: "nio-dev" , "core-libs-dev" 


Envoyé: Vendredi 27 Avril 2018 21:21:01
Objet: Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for 
reading/writing a string from/to a file

Hi Rémi, Alan,

Hi Joe,

I'm not sure we'd want to replace system dependent line 
separators with

'\n'. There are cases as you described where the replacement makes
sense. However,  there are other cases too. For example, the 
purpose is
to read, edit a text file and then write it back. If this is on 
Windows,

and if line separators are replaced with '\n', that would cause the
resulting file not formatted properly in for example Notepad. 
There are
cases where users write text files on Windows using Java, and 
only found

the lines were not separated in Notepad.
I agree that why the counterpart of readString() that write the 
string should inserts the platform's line separator.
BTW, i just found that those methods are not named writeString, or 
writeCharSequence, i think they should.


While readString() does not modify the original content (e.g. by 
replacing the platform's line separator with '\n'), write(String) 
won't either, by adding extra characters such as the line separator.


I would think interfaces shall read along with the parameters.
readString(Path) == read as a String from the specified Path 
(one could argue for readToString, readAsString, but we generally 
omit the preps)
write(Path, CharSequence) == write the CharSequence to the 
file, since CharSequence is already in the method signature as a 
parameter, we probably don't want to add that to the name, 
otherwise it would read like repeating the word CharSequence.


It is in a similar situation as write(Path, Iterable) where it was 
defined as writeLines(Path, Iterable).





Files.write(Path, Iterable) is also specified to terminate each line
with the platform's line separator. If readString does the 
replacement,

it would be inconsistent.


Anyway, 

RFR: JDK-8204973: Add build support for filtering translations

2018-06-13 Thread Erik Joelsson

Hello,

Oracle will reduce the number of languages that it maintains 
translations of JDK resources for. The current translations will remain 
in the source for now, but we need a way to filter out a set of 
translations at build time so that we only include the ones we support. 
This patch adds such a configuration option. It also changes how Oracle 
builds by using the option to exclude all translations except English, 
Japanese, Simplified Chinese and Traditional Chinese. Anyone else 
building OpenJDK will by default include all translations present in the 
source, just as before.


I added a test that verifies this for builds with the "IMPLEMENTOR" 
field in the release file set to "Oracle Corporation". The test will not 
be run for other OpenJDK builds.


I had to modify an existing test for java.logging which used various 
translations to verify localized log messages to only use translations 
that Oracle chooses to include.


Since this is the second test that specifically verifies build behavior, 
I moved the previous such test together with this new test into a common 
top level test directory "build", under the jdk test root. I put these 
tests in the jdk tier3 test group.


I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically 
looked for tests that use the java.util.Locale class and ran them locally.


Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html

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

/Erik



RFR: JDK-8204172 Predicate::not should explicitly mention "NullPointerException - if target is null"

2018-06-13 Thread Jim Laskey
CSR: https://bugs.openjdk.java.net/browse/JDK-8204959
webrev: http://cr.openjdk.java.net/~jlaskey/8204172/webrev/index.html

Thank you,

— Jim



Re: RFR(xs): 8204663: clean up remaining native parts after JDK-8187631

2018-06-13 Thread Thomas Stüfe
Thank you Paul!

On Wed, Jun 13, 2018 at 6:19 PM, Paul Sandoz  wrote:
> +1
> Paul.
>
>> On Jun 13, 2018, at 2:56 AM, Thomas Stüfe  wrote:
>>
>> May I have a second review please.
>>
>> Thanks, Thomas
>>
>> On Mon, Jun 11, 2018, 15:13 Thomas Stüfe  wrote:
>>
>>> Hi,
>>>
>>> may I please have reviews for this small cleanup.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8204663
>>> Webrev:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8204663-clean-up-after-JDK-8187631/webrev.00/webrev/
>>>
>>> This change removes some native functions from FOS/FIS which are
>>> orphaned and unused since "JDK-8187631: Refactor FileDescriptor close
>>> implementation".
>>>
>>> Tests on jdk-submit came back clean save one strange test error on
>>> windows which I cannot reproduce locally. I am investigating that one.
>>>
>>> Thank you, Thomas
>>>
>


Re: RFR: 8204967: Resolve disabled warnings for libunpack

2018-06-13 Thread Jim Laskey
Does -Wimplicit-fallthrough=3 work for gnu c and clang?

unpack.cpp redundant comment

3713 // else fall through:

+1  

— Jim


> On Jun 13, 2018, at 1:57 PM, Srinivas Dama  wrote:
> 
> Hi,
> 
> Please review http://cr.openjdk.java.net/~sdama/8204967/webrev.00/
> for https://bugs.openjdk.java.net/browse/JDK-8204967
> 
> Regards,
> Srinivas



Re: RFR: [11] 8202537: CLDR 33

2018-06-13 Thread naoto . sato

Hi Rachna,

A couple of comments:

- NumberingSystemsParseHandler.java

Since the code substitutes latin digits for supplementary digits, it can 
skip line 68-79.


- A test should be written for the above substitution.

Naoto

On 6/12/18 10:33 PM, Rachna Goel wrote:

Hi,

Kindly review fix for JDK-8202537. Fix is to upgrade Unicode 
consortium's CLDR data into JDK from current version 29 to 33.


For more info : http://cldr.unicode.org/index/downloads/cldr-33

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

Patch: http://cr.openjdk.java.net/~rgoel/JDK-8202537/webrev.06/index.html



RFR: 8204967: Resolve disabled warnings for libunpack

2018-06-13 Thread Srinivas Dama
Hi,

Please review http://cr.openjdk.java.net/~sdama/8204967/webrev.00/
for https://bugs.openjdk.java.net/browse/JDK-8204967

Regards,
Srinivas


Re: [JDK 11] RFR 8204944: Remove java/util/Map/InPlaceOpsCollisions.java from ProblemList.txt

2018-06-13 Thread Paul Sandoz
+1
Paul.

> On Jun 13, 2018, at 2:31 AM, Amy Lu  wrote:
> 
> Please review this patch to remove java/util/Map/InPlaceOpsCollisions.java 
> from ProblemList.txt,
> related bug JDK-8203387 (JDK-8203425) has been fixed.
> 
> Thanks,
> Amy
> 
> --- old/test/jdk/ProblemList.txt2018-06-13 17:23:33.0 +0800
> +++ new/test/jdk/ProblemList.txt2018-06-13 17:23:32.0 +0800
> @@ -853,8 +853,6 @@
> 
>  # jdk_util
> 
> -java/util/Map/InPlaceOpsCollisions.java 8203387 generic-all
> -
>  
> 
>  # jdk_instrument
> 
> 



Re: RFR(xs): 8204663: clean up remaining native parts after JDK-8187631

2018-06-13 Thread Paul Sandoz
+1
Paul.

> On Jun 13, 2018, at 2:56 AM, Thomas Stüfe  wrote:
> 
> May I have a second review please.
> 
> Thanks, Thomas
> 
> On Mon, Jun 11, 2018, 15:13 Thomas Stüfe  wrote:
> 
>> Hi,
>> 
>> may I please have reviews for this small cleanup.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8204663
>> Webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8204663-clean-up-after-JDK-8187631/webrev.00/webrev/
>> 
>> This change removes some native functions from FOS/FIS which are
>> orphaned and unused since "JDK-8187631: Refactor FileDescriptor close
>> implementation".
>> 
>> Tests on jdk-submit came back clean save one strange test error on
>> windows which I cannot reproduce locally. I am investigating that one.
>> 
>> Thank you, Thomas
>> 



Re: RFR: [11] 8202537: CLDR 33

2018-06-13 Thread Rachna Goel

Hi Roger,

No update is done mechanically between CLDR data and .xml files.

CLDR's data in .xml files are generated into resourcebundles at build 
time by cldrconverter tool.


Already existing regression test 
"test/jdk/sun/text/resources/LocaleDataTest.java" verify that same data 
is retrieved by APIs.


Thanks,

Rachna*
*


On 6/13/18 8:06 PM, Roger Riggs wrote:

Hi Rachna,

How much of the updates are done mechanically between the CLDR data 
and the .xml files?

Manually reviewing that many files is likely to be error prone.

Thanks, Roger


On 6/13/2018 1:33 AM, Rachna Goel wrote:

Hi,

Kindly review fix for JDK-8202537. Fix is to upgrade Unicode 
consortium's CLDR data into JDK from current version 29 to 33.


For more info : http://cldr.unicode.org/index/downloads/cldr-33

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

Patch: 
http://cr.openjdk.java.net/~rgoel/JDK-8202537/webrev.06/index.html






--
Thanks,
Rachna



Re: RFR: [11] 8202537: CLDR 33

2018-06-13 Thread Roger Riggs

Hi Rachna,

How much of the updates are done mechanically between the CLDR data and 
the .xml files?

Manually reviewing that many files is likely to be error prone.

Thanks, Roger


On 6/13/2018 1:33 AM, Rachna Goel wrote:

Hi,

Kindly review fix for JDK-8202537. Fix is to upgrade Unicode 
consortium's CLDR data into JDK from current version 29 to 33.


For more info : http://cldr.unicode.org/index/downloads/cldr-33

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

Patch: http://cr.openjdk.java.net/~rgoel/JDK-8202537/webrev.06/index.html





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

2018-06-13 Thread Roger Riggs

Hi Joe,

The CSR text is still in draft and got out of sync with the open review 
and comments.


The updated apiNote clarifies that changes made through the 
System.*property*  methods

may not affect the value cached during initialization.
The implementation changes affect a very short list of properties.

The note on the methods is to raise awareness that individual properties 
may have

different behavior and may be unpredictable.

On 6/12/2018 10:10 PM, Weijun Wang wrote:

In fact, why is setting user.name and user.home always evil? If I only want to set them 
on the command line so that a special "user environment" is used, why is it a 
problem?

There is no change to the ability to set properties on the command line.
The concern is with the predictability of (some) properties changing 
dynamically while the application

is running.

The property user.home is used for mime-types and mailcap files and 
user.name is used in some

networking cases where a username is needed.


In fact, we have a test setting user.home to an empty directory to avoid 
unexpected result because we cannot control the test runner's home directory.

Right, there is no problem with that

Regards, Roger


Thanks
Max


On Jun 13, 2018, at 9:59 AM, Weijun Wang  wrote:

Hi Roger

1. Should all occurrences of reading of these system properties be updated? For 
example, the following one is not touched
  
http://hg.openjdk.java.net/jdk/jdk/file/4d2e3f5abb48/src/java.base/share/classes/sun/security/tools/keytool/Main.java#l842

That usage was in a tool and I did not modify any tools.


2. I assume that with this change not only there is no use calling 
System.setProperty() in the application but also setting it on the command line 
is now useless. Is this right? Do we need to make this clear in the CSR?

No change to command line setting of properties.

Roger



Thanks
Max



On Jun 4, 2018, at 9:32 PM, Roger Riggs  wrote:

Please review a change to make the values of java.home, user.home, user.dir, 
and user.name
effectively read-only for internal use.  The values are cached during 
initialization and the
cached values are used.

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

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

CSR:
  https://bugs.openjdk.java.net/browse/JDK-8204235

Thanks, Roger






Re: RFR(xs): 8204663: clean up remaining native parts after JDK-8187631

2018-06-13 Thread Thomas Stüfe
May I have a second review please.

Thanks, Thomas

On Mon, Jun 11, 2018, 15:13 Thomas Stüfe  wrote:

> Hi,
>
> may I please have reviews for this small cleanup.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8204663
> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8204663-clean-up-after-JDK-8187631/webrev.00/webrev/
>
> This change removes some native functions from FOS/FIS which are
> orphaned and unused since "JDK-8187631: Refactor FileDescriptor close
> implementation".
>
> Tests on jdk-submit came back clean save one strange test error on
> windows which I cannot reproduce locally. I am investigating that one.
>
> Thank you, Thomas
>


[JDK 11] RFR 8204944: Remove java/util/Map/InPlaceOpsCollisions.java from ProblemList.txt

2018-06-13 Thread Amy Lu
Please review this patch to remove 
java/util/Map/InPlaceOpsCollisions.java from ProblemList.txt,

related bug JDK-8203387 (JDK-8203425) has been fixed.

Thanks,
Amy

--- old/test/jdk/ProblemList.txt    2018-06-13 17:23:33.0 +0800
+++ new/test/jdk/ProblemList.txt    2018-06-13 17:23:32.0 +0800
@@ -853,8 +853,6 @@

 # jdk_util

-java/util/Map/InPlaceOpsCollisions.java 8203387 generic-all
-
 

 # jdk_instrument




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

2018-06-13 Thread joe darcy

Hi David,

On 5/24/2018 10: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/



In Class.java,

3990 Class[] members = getNestMembers0();
3991 // Can't actually enable this due to bootstrapping issues
3992 // assert(members.length != 1 || members[0] == this); // 
expected invariant from VM


can these checks be enabled unconditionally without using the assert 
facility, throwing AssertionError or some other kind of error?


Thanks,

-Joe