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

[Peter Levart]

It's good to have alternative implementations on the table. Here's 
another variant that is space neutral for threads that don't use 
JdkThreadLocal, but also scales to many JdkThreadLocal instances:


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

Now we only need an arbiter to decide which one! :-)

Regards, Peter

On 05/17/18 22:39, Peter Levart wrote:

Hi Tony,

If we anticipate only small number of JdkThreadLocal(s) (there will be 
only one for the start) then this is a plausible solution. Then 
perhaps this limitation should be written somewhere in the 
JdkThreadLocal javadoc so that one day somebody will not be tempted to 
use JdkThreadLocal(s) en masse. Let's say there will be a few more 
JdkThreadLocal(s) in the future. Are we willing to pay for a few 
lookups into a ThreadLocalMap at each and every thread's exit even 
though such thread did not register a mapping for any JdkThreadLocal? 
Is an additional reference field in each and every ThreadLocalMap (one 
per Thread that uses thread locals) a bigger price to pay? I don't 
know. Will let others comment on this.


Otherwise the code looks good. Just a couple of observations:

Since private static method JdkThreadLocal.add is only called from 
JdkThreadLocal constructor with just constructed instance (this), 
there's no possibility for it to be called twice or more times with 
the same instance. The check for duplicates could go away then, right?


You keep an array of Entry objects which are just wrappers for 
JdkThreadLocal objects. Are you already planning for Entry to become a 
WeakReference? Otherwise you could just keep JdkThreadLocal objects in 
the array directly.


Regards, Peter

On 05/17/18 20:25, Tony Printezis wrote:

Hi all,

I have a new version of the code for your consideration:

http://cr.openjdk.java.net/~tonyp/8202788/webrev.1/ 



I basically combined our two approaches. The usage is as Alan had 
proposed it: Users have to use JdkThreadLocal (which is only 
available within java.base) and override threadTerminated(). However, 
I keep track of JdkThreadLocal instances globally (as I did before) 
and not per-thread. This way we don’t need to add any unnecessary 
complexity to ThreadLocalMap.


Currently, I don’t allow entries to be purged if the JdkThreadLocal 
instance becomes otherwise unreachable. I can easily add that 
functionality if needed (I can use WeakReferences for that). However, 
for the uses we’re considering, is it really necessary?


Thoughts?

Tony


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




On May 14, 2018 at 12:40:28 PM, Tony Printezis 
(tprinte...@twitter.com ) wrote:



Peter,

In my proposal, you can register the exit hook in the ThreadLocal 
c’tor, so it’s almost as nice as Alan’s in that respect (and it 
doesn't require an extra field per ThreadLocal plus two extra fields 
per JdkEntry). :-)


But, I do like the addition of the JdkEntry list to avoid 
unnecessarily iterating over all the map entries (which was my main 
concern with Alan’s original webrev). I’ll be totally happy with a 
version of this.


Tony


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




On May 12, 2018 at 6:44:08 AM, Peter Levart (peter.lev...@gmail.com 
) wrote:



Hi,

On 05/11/18 16:13, Alan Bateman wrote:

On 08/05/2018 16:07, Tony Printezis wrote:

Hi all,

Following the discussion on this a few weeks ago, here’s the 
first version

of the change:

http://cr.openjdk.java.net/~tonyp/8202788/webrev.0/

I think the consensus was that it’d be easier if the exit hooks 
were only
available within java.base. Is it enough that I added the 
functionality to
the jdk.internal.misc package? (And is jdk.internal.misc the best 
place for

this?)

I’ll also add a test for the new functionality. But let’s first 
come up

with an approach that everyone is happy with. :-)

Peter's approach in early April was clean (and we should come to 
the getIfPresent discussion) but it adds a field to Thread for the 
callback list. If I read your approach correctly, you are avoiding 
that by maintaining an array of hooks in ThreadLocalExitHooks.


Another approach to try is a java.base-internal ThreadLocal that 
defines a method to be invoked when a thread terminates. Something 
like the following:

http://cr.openjdk.java.net/~alanb/8202788/webrev/index.html

-Alan


From the API perspective, Alan's suggestion is the most attractive 
one as it puts the method to where it belongs - into the 
ThreadLocal instance. But the implementation could be improved a 
bit. I don't like the necessity to iterate over all ThreadLocal(s) 
to filter out JdkThreadLocal(s). There might be a situation where 
there's plenty of ThreadLocal(s) registered per exiting thread 
which would produce a spike in CPU processing at thread exit.

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

[forax]

- Mail original -
> De: "David Holmes" 
> À: "Remi Forax" , "Alan Bateman" 
> Cc: "core-libs-dev" 
> Envoyé: Jeudi 17 Mai 2018 12:59:56
> Objet: Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: 
> Nest-Based Access Control

> Clarification ...
> 
> On 17/05/2018 6:37 PM, David Holmes wrote:
>> Hi Remi,
>> 
>> On 17/05/2018 6:16 PM, Remi Forax wrote:
>>> Hi all,
>>>
>>> - Mail original -
 De: "Alan Bateman" 
 À: "David Holmes" , "core-libs-dev"
 
 Envoyé: Mardi 15 Mai 2018 15:53:44
 Objet: Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181:
 Nest-Based Access Control
>>>
 On 15/05/2018 01:52, 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/
>>>
>>> [...]
>>>
 Maybe a question for Kumar but are we planning to pull in any ASM
 updates for JDK 11? NestMembers extends Attribute looks okay, I'm less
 sure about the change to ClassReader as I don't know if there is
 somewhere else in ASM that has the list of attributes to always parse.
>>>
>>> With my ASM hat,
>>> the current master of ASM (the release of ASM 6.2 is scheduled for the
>>> next week-end) already supports nestmates (and constant dynamic and
>>> preview feature) so i suppose that at some point in the future Kumar
>>> will merge it to the JDK.
>> 
>> Unfortunately Kumar is no longer with us.
> 
> By which I simply mean he is no longer at Oracle.

oh, cool, sorry for this awkward moment.

> 
> David
> -

Rémi

Re: RFR: Small cleanups in java.lang.ref

[Per Liden]

On 05/17/2018 10:03 PM, mark.reinh...@oracle.com wrote:

2018/5/16 18:31:15 -0700, marti...@google.com:

I've been confused by NULL vs null for years.


That’s because `NULL` in ReferenceQueue.java refers to the singleton
object `ReferenceQueue.NULL`, not the null value.  See the long comment
at the top of Reference.java for an explanation.

To improve this you could change mentions of `NULL` in the comments to
`ReferenceQueue.NULL`, but changing them to `null` would be incorrect.


The comments that were changed in the proposed patch refer to the 
Reference.next and Reference.discovered fields, not Reference.queue. So 
"null" should be correct there.


/Per




8203327: Small cleanups in java.lang.ref
http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup/
https://bugs.openjdk.java.net/browse/JDK-8203327


The other changes look fine.

- Mark

Re: RFR: Small cleanups in java.lang.ref

[Thomas Schatzl]

Hi,

On Wed, 2018-05-16 at 18:31 -0700, Martin Buchholz wrote:
> I've been confused by NULL vs null for years.
> 
> 8203327: Small cleanups in java.lang.ref
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup/
> https://bugs.openjdk.java.net/browse/JDK-8203327

  JDK-8203028 which is currently out for review (http://cr.openjdk.java
.net/~kbarrett/8203028/open.02) changes and fixes the comments in that
respect apart from other significant changes to reference processing.

Pushing this cleanup would just cause merge conflicts for us (the gc
team). So this change does not seem to be required any more, i.e. do
you mind retracting this RFR and closing this change as duplicate of
the other?

(I apologize if my speech is too direct, I do not want to offend)

Thanks,
  Thomas

Re: RFR 8203279 : Faster calculation of power of two

[Claes Redestad]

On 2018-05-17 22:44, Ivan Gerasimov wrote:

The following variant showed slightly better performance on my machine:

    static final int numberOfLeadingZeros(int i) {
    if (i <= 0)
    return i == 0 ? 32 : 0;
    int n = 31;
    if (i >= 1 << 16) { n -= 16; i >>>= 16; }
    if (i >= 1 <<  8) { n -=  8; i >>>=  8; }
    if (i >= 1 <<  4) { n -=  4; i >>>=  4; }
    if (i >= 1 <<  2) { n -=  2; i >>>=  2; }
    return n - (i >>> 1);
    }


Nice, this version also wins on all of -Xint and 
-XX:TieredStopAtLevel=1-3 (my version lost out slightly versus
the baseline on -Xint), so is potentially a startup enhancement even on 
platforms with C2 intrinsics.




I agree that improving Java implementation of numberOfLeadingZeros() 
can be done as a separate RFE. 


I filed https://bugs.openjdk.java.net/browse/JDK-8203352 for this.

/Claes

RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Claes Redestad]

Hi,

while there are C2 intrinsics on most platforms providing access to 
specialized hardware instructions, e.g., lzcnt on Intel, optimizing the 
java implementations of Integer/Long.numberOfLeadingZeros can still be 
worthwhile, especially if it also helps C1 and implicitly 
startup/warmup. This implementation wins slightly (5-25%) over the 
baseline in all tested optimization modes (-Xint, 
-XX:TieredStopAtLevel=1-3), as well as on C2 if the intrinsics are disabled.


Webrev: http://cr.openjdk.java.net/~redestad/8203352/open.00/

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

Correctness is checked by existing tests, mainly 
test/jdk/java/lang/Integer|Long/BitTwiddle.java


/Claes

RFR: JDK-8200380 String::lines

[Jim Laskey]

String::lines instance method that returns a Stream with elements 
composed of substrings from the original string delimited by any recognized new 
line character sequence.

webrev: http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html 

bug: https://bugs.openjdk.java.net/browse/JDK-8200380 

csr: https://bugs.openjdk.java.net/browse/JDK-8200425 

Re: RFR: JDK-8200380 String::lines

[Sundararajan Athijegannathan]

+1

I think LinesSpliterator classes in StringLatin1 and StringUTF8 could be 
"private" and "final".


-Sundar

On 18/05/18, 7:14 PM, Jim Laskey wrote:

String::lines instance method that returns a Stream  with elements 
composed of substrings from the original string delimited by any recognized new line 
character sequence.

webrev: 
http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html
bug: 
https://bugs.openjdk.java.net/browse/JDK-8200380
csr: 
https://bugs.openjdk.java.net/browse/JDK-8200425

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Martin Buchholz]

On Fri, May 18, 2018 at 3:51 AM, Claes Redestad 
wrote:

>
> Correctness is checked by existing tests, mainly
> test/jdk/java/lang/Integer|Long/BitTwiddle.java


Do java library tests get run regularly with intrinsics disabled?
(Perhaps as a result of testing with -Xint?)

Re: RFR: JDK-8200380 String::lines

[Roger Riggs]

Hi Jim,

Can you add some test cases for the UTF-16.  I think all the cases fall 
into the Latin1 encoding.

The rest looks fine.

Roger

On 5/18/18 10:31 AM, Sundararajan Athijegannathan wrote:

+1

I think LinesSpliterator classes in StringLatin1 and StringUTF8 could 
be "private" and "final".


-Sundar

On 18/05/18, 7:14 PM, Jim Laskey wrote:
String::lines instance method that returns a Stream  with 
elements composed of substrings from the original string delimited by 
any recognized new line character sequence.


webrev: 
http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html
bug: 
https://bugs.openjdk.java.net/browse/JDK-8200380
csr: 
https://bugs.openjdk.java.net/browse/JDK-8200425

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Alan Bateman]

On 17/05/2018 01:56, Brian Burkhalter wrote:

Hi Ivan,

On May 16, 2018, at 2:54 PM, Ivan Gerasimov  wrote:


Maybe it is better to compare fileData.cFileName with the pathbuf to make sure 
we're dealing with the correct file?

Certainly making this change to the previous proposal would do no harm:

--- a/src/java.base/windows/native/libjava/WinNTFileSystem_md.c
+++ b/src/java.base/windows/native/libjava/WinNTFileSystem_md.c
@@ -541,10 +541,13 @@
  WIN32_FIND_DATAW fileData;
  HANDLE h = FindFirstFileW(pathbuf, &fileData);
  if (h != INVALID_HANDLE_VALUE) {
-ULARGE_INTEGER length;
-length.LowPart = fileData.nFileSizeLow;
-length.HighPart = fileData.nFileSizeHigh;
-rv = (jlong)length.QuadPart;
+size_t off = wcslen(pathbuf) - wcslen(fileData.cFileName);
+if (wcscmp(pathbuf + off, fileData.cFileName) == 0) {
+ULARGE_INTEGER length;
+length.LowPart = fileData.nFileSizeLow;
+length.HighPart = fileData.nFileSizeHigh;
+rv = (jlong)length.QuadPart;
+}
  FindClose(h);
  }
  }

The offset in the string comparison is necessary as “C:\\pagefile.sys” becomes 
“pagefile.sys” in fileData.cFileName. I’m not certain that this will work in 
all cases however but it is certainly no worse than before.

Can the call to _wstati64 be removed? That is, if GetFileAttributesExW 
fails with a sharing violation then just use FindFirstFileW as the fallback.


-Alan

Re: RFR: JDK-8200380 String::lines

[Jim Laskey]

D’oh - thanks for reminding me.  I had it in the back of my mind but failed to 
follow through.

Cheers,

— Jim


> On May 18, 2018, at 11:39 AM, Roger Riggs  wrote:
> 
> Hi Jim,
> 
> Can you add some test cases for the UTF-16.  I think all the cases fall into 
> the Latin1 encoding.
> The rest looks fine.
> 
> Roger
> 
> On 5/18/18 10:31 AM, Sundararajan Athijegannathan wrote:
>> +1
>> 
>> I think LinesSpliterator classes in StringLatin1 and StringUTF8 could be 
>> "private" and "final".
>> 
>> -Sundar
>> 
>> On 18/05/18, 7:14 PM, Jim Laskey wrote:
>>> String::lines instance method that returns a Stream  with elements 
>>> composed of substrings from the original string delimited by any recognized 
>>> new line character sequence.
>>> 
>>> webrev: 
>>> http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html
>>> bug: 
>>> https://bugs.openjdk.java.net/browse/JDK-8200380
>>> csr: 
>>> https://bugs.openjdk.java.net/browse/JDK-8200425
>>> 
>>> 
> 

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Brian Burkhalter]

On May 18, 2018, at 7:40 AM, Alan Bateman  wrote:

>> The offset in the string comparison is necessary as “C:\\pagefile.sys” 
>> becomes “pagefile.sys” in fileData.cFileName. I’m not certain that this will 
>> work in all cases however but it is certainly no worse than before.
>> 
> Can the call to _wstati64 be removed? That is, if GetFileAttributesExW fails 
> with a sharing violation then just use FindFirstFileW as the fallback.

Had not tried that yet but I will.

Thanks,

Brian

PS A proposed fix for https://bugs.openjdk.java.net/browse/JDK-8201407 should 
be forthcoming later today.

Re: RFR: Small cleanups in java.lang.ref

[Martin Buchholz]

On Fri, May 18, 2018 at 3:26 AM, Thomas Schatzl 
wrote:

> Hi,
>
> On Wed, 2018-05-16 at 18:31 -0700, Martin Buchholz wrote:
> > I've been confused by NULL vs null for years.
> >
> > 8203327: Small cleanups in java.lang.ref
> > http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup/
> > https://bugs.openjdk.java.net/browse/JDK-8203327
>
>   JDK-8203028 which is currently out for review (http://cr.openjdk.java
> .net/~kbarrett/8203028/open.02) changes and fixes the comments in that
> respect apart from other significant changes to reference processing.
>
> Pushing this cleanup would just cause merge conflicts for us (the gc
> team). So this change does not seem to be required any more, i.e. do
> you mind retracting this RFR and closing this change as duplicate of
> the other?
>
> (I apologize if my speech is too direct, I do not want to offend)
>

On the contrary, I'm very happy to see gc team actively maintaining
java.lang.ref.
I've reverted changes to Reference.java

http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup-1/

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Claes Redestad]

On 2018-05-18 16:35, Martin Buchholz wrote:



On Fri, May 18, 2018 at 3:51 AM, Claes Redestad 
mailto:claes.redes...@oracle.com>> wrote:



Correctness is checked by existing tests, mainly
test/jdk/java/lang/Integer|Long/BitTwiddle.java


Do java library tests get run regularly with intrinsics disabled?
(Perhaps as a result of testing with -Xint?)


Not sure but we might be running these with a few different stress flags 
in later tiers, but in the
standard mode the BitTwiddle tests are more likely to test -Xint and 
C1-compiled modes than the
C2 intrinsic.. I've also manually run exhaustive tests to ensure the new 
implementation matches
the result of the intrinsic version for all possible inputs (rather than 
corner cases + 1000 random

inputs like BitTwiddle does)

/Claes

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

[Tony Printezis]

Hi Peter,

Thanks for taking a look at the new webrev!

Initially, I think we’re expecting two uses of JdkThreadLocal: one in
sun.nio.ch.Utils, shown on my webrev and my original motivation for working
on this, and one in sun.nio.fs.NativeBuffers, as shown on Alan’s webrev
(I’m not familiar with that part of the code at all; I assume it’s
addressing a similar issue to sun.nio.ch.Utils).

When I originally brought up this issue, Alan said that he's only expecting
2-3 uses (literally) inside java.base, so I did the implementation
accordingly and tried to keep it as simple as possible. We could maybe look
at other uses of ThreadLocal inside java.base to get a better sense of how
many more will benefit from a thread termination callback?

Response to your comments:

I can definitely add javadoc to explain the limitations of the
implementation. It takes me a long time to write coherent javadoc /
comments, so I wanted to make sure we have the right approach first before
I did that. :-)

Re: sanity tests in add(): I was being paranoid but, sure, I can remove
them.

Re: Entry objects: You’re absolutely right. I did it this way to make it
easier to extend WeakReference if needed. It also keeps the code a bit less
verbose. I can use JdkThreadLocal directly.

Tony

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


On May 17, 2018 at 4:39:20 PM, Peter Levart (peter.lev...@gmail.com) wrote:

Hi Tony,

If we anticipate only small number of JdkThreadLocal(s) (there will be only
one for the start) then this is a plausible solution. Then perhaps this
limitation should be written somewhere in the JdkThreadLocal javadoc so
that one day somebody will not be tempted to use JdkThreadLocal(s) en
masse. Let's say there will be a few more JdkThreadLocal(s) in the future.
Are we willing to pay for a few lookups into a ThreadLocalMap at each and
every thread's exit even though such thread did not register a mapping for
any JdkThreadLocal? Is an additional reference field in each and every
ThreadLocalMap (one per Thread that uses thread locals) a bigger price to
pay? I don't know. Will let others comment on this.

Otherwise the code looks good. Just a couple of observations:

Since private static method JdkThreadLocal.add is only called from
JdkThreadLocal constructor with just constructed instance (this), there's
no possibility for it to be called twice or more times with the same
instance. The check for duplicates could go away then, right?

You keep an array of Entry objects which are just wrappers for
JdkThreadLocal objects. Are you already planning for Entry to become a
WeakReference? Otherwise you could just keep JdkThreadLocal objects in the
array directly.

Regards, Peter

On 05/17/18 20:25, Tony Printezis wrote:

Hi all,

I have a new version of the code for your consideration:

http://cr.openjdk.java.net/~tonyp/8202788/webrev.1/

I basically combined our two approaches. The usage is as Alan had proposed
it: Users have to use JdkThreadLocal (which is only available within
java.base) and override threadTerminated(). However, I keep track of
JdkThreadLocal instances globally (as I did before) and not per-thread.
This way we don’t need to add any unnecessary complexity to ThreadLocalMap.

Currently, I don’t allow entries to be purged if the JdkThreadLocal
instance becomes otherwise unreachable. I can easily add that functionality
if needed (I can use WeakReferences for that). However, for the uses we’re
considering, is it really necessary?

Thoughts?

Tony


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


On May 14, 2018 at 12:40:28 PM, Tony Printezis (tprinte...@twitter.com)
wrote:

Peter,

In my proposal, you can register the exit hook in the ThreadLocal c’tor, so
it’s almost as nice as Alan’s in that respect (and it doesn't require an
extra field per ThreadLocal plus two extra fields per JdkEntry). :-)

But, I do like the addition of the JdkEntry list to avoid unnecessarily
iterating over all the map entries (which was my main concern with Alan’s
original webrev). I’ll be totally happy with a version of this.

Tony


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


On May 12, 2018 at 6:44:08 AM, Peter Levart (peter.lev...@gmail.com) wrote:

Hi,

On 05/11/18 16:13, Alan Bateman wrote:

On 08/05/2018 16:07, Tony Printezis wrote:

Hi all,

Following the discussion on this a few weeks ago, here’s the first version
of the change:

http://cr.openjdk.java.net/~tonyp/8202788/webrev.0/

I think the consensus was that it’d be easier if the exit hooks were only
available within java.base. Is it enough that I added the functionality to
the jdk.internal.misc package? (And is jdk.internal.misc the best place for
this?)

I’ll also add a test for the new functionality. But let’s first come up
with an approach that everyone is happy with. :-)

Peter's approach in early April was clean (and we should come to the
getIfPresent discussion) but it adds a field to Thread for the callback
list. If I r

Re: RFR: Small cleanups in java.lang.ref

[mark . reinhold]

2018/5/18 3:11:25 -0700, per.li...@oracle.com:
> On 05/17/2018 10:03 PM, mark.reinh...@oracle.com wrote:
>> 2018/5/16 18:31:15 -0700, marti...@google.com:
>>> I've been confused by NULL vs null for years.
>> 
>> That’s because `NULL` in ReferenceQueue.java refers to the singleton
>> object `ReferenceQueue.NULL`, not the null value.  See the long comment
>> at the top of Reference.java for an explanation.
>> 
>> To improve this you could change mentions of `NULL` in the comments to
>> `ReferenceQueue.NULL`, but changing them to `null` would be incorrect.
> 
> The comments that were changed in the proposed patch refer to the 
> Reference.next and Reference.discovered fields, not Reference.queue. So 
> "null" should be correct there.

Oops, you’re right!  Confused by my own twenty-year old code ...

What’s missing here is a similar comment on the `Reference.queue` field.
Suggested text:

/* When active: queue with which this reference is registered
 * pending: queue with which this reference is registered
 *enqueued: ReferenceQueue.ENQUEUED
 *inactive: ReferenceQueue.NULL
 */

Martin -- while you’re at it, please lowercase “Enqueued:” and
“Inactive:” in the other comments, and hard-wrap the comment on
the `discovered` field to 80 columns.

- Mark


>>> 8203327: Small cleanups in java.lang.ref
>>> http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup/
>>> https://bugs.openjdk.java.net/browse/JDK-8203327

Re: RFR: Small cleanups in java.lang.ref

[Roger Riggs]

Hi Kim,

In the cleanup category also is:JDK-8132984 
 incorrect type for 
Reference.discovered


Is that within the scope of 8203028?

Thanks, Roger


On 5/18/18 10:47 AM, Martin Buchholz wrote:

On Fri, May 18, 2018 at 3:26 AM, Thomas Schatzl 
wrote:


Hi,

On Wed, 2018-05-16 at 18:31 -0700, Martin Buchholz wrote:

I've been confused by NULL vs null for years.

8203327: Small cleanups in java.lang.ref
http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup/
https://bugs.openjdk.java.net/browse/JDK-8203327

   JDK-8203028 which is currently out for review (http://cr.openjdk.java
.net/~kbarrett/8203028/open.02) changes and fixes the comments in that
respect apart from other significant changes to reference processing.

Pushing this cleanup would just cause merge conflicts for us (the gc
team). So this change does not seem to be required any more, i.e. do
you mind retracting this RFR and closing this change as duplicate of
the other?

(I apologize if my speech is too direct, I do not want to offend)


On the contrary, I'm very happy to see gc team actively maintaining
java.lang.ref.
I've reverted changes to Reference.java

http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup-1/

Re: RFR: Small cleanups in java.lang.ref

[Thomas Schatzl]

Hi Martin,

On Fri, 2018-05-18 at 07:47 -0700, Martin Buchholz wrote:
> 
> 
> On Fri, May 18, 2018 at 3:26 AM, Thomas Schatzl  e.com> wrote:
> > Hi,
> > 
> > On Wed, 2018-05-16 at 18:31 -0700, Martin Buchholz wrote:
> > > I've been confused by NULL vs null for years.
> > > 
> > > 8203327: Small cleanups in java.lang.ref
> > > http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup/
> > > https://bugs.openjdk.java.net/browse/JDK-8203327
> > 
> >   JDK-8203028 which is currently out for review
> > (http://cr.openjdk.java.net/~kbarrett/8203028/open.02) changes and
> > fixes the comments in that respect apart from other significant
> > changes to reference processing.
> > 
> > Pushing this cleanup would just cause merge conflicts for us (the
> > gc team). So this change does not seem to be required any more,
> > i.e. do you mind retracting this RFR and closing this change as
> > duplicate of the other?
> > 
> > (I apologize if my speech is too direct, I do not want to offend)
> 
> On the contrary, I'm very happy to see gc team actively maintaining
> java.lang.ref.

We are only fixing some performance issues for the current
implementation in G1 (and only incidentally for some of the other GCs)
(look for the "gc-reference-processor" label in the bug tracker) as
kind-of part of JEP 308 :)

In the future G1 may do reference processing completely concurrently
(as already mentioned elsewhere).

All these changes are and will most likely be completely unrelated to
java.lang.ref, apart from that one you just tripped over. I believe
there are no further changes in java.lang.ref needed, so gc-team
"maintenance" for that package can probably be considered "inactive"
again :P

> I've reverted changes to Reference.java
> 
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup-1/

  okay, I did not notice that we did not touch the ReferenceQueue,
currently travelling...

The change looks good to me now, but there are probably more
knowledgable people around :)

The copyright could be updated though.

Thanks,
  Thomas

RFR: CSR - JDK-8203428 Predicate::not

[Jim Laskey]

Introduce a new static method Predicate::not which will allow developers to 
negate predicate lambdas trivially.


csr: https://bugs.openjdk.java.net/browse/JDK-8203428

Re: RFR: CSR - JDK-8203428 Predicate::not

[Chris Hegarty]

On 18/05/18 17:35, Jim Laskey wrote:

Introduce a new static method Predicate::not which will allow developers to 
negate predicate lambdas trivially.


csr: https://bugs.openjdk.java.net/browse/JDK-8203428


I have looked for this a number of times. +1

-Chris.

Re: RFR: Small cleanups in java.lang.ref

[Martin Buchholz]

On Fri, May 18, 2018 at 8:32 AM,  wrote:

>
> What’s missing here is a similar comment on the `Reference.queue` field.
> Suggested text:
>
> /* When active: queue with which this reference is registered
>  * pending: queue with which this reference is registered
>  *enqueued: ReferenceQueue.ENQUEUED
>  *inactive: ReferenceQueue.NULL
>  */
>
>
I agree Reference.queue should have a comment.


> Martin -- while you’re at it, please lowercase “Enqueued:” and
> “Inactive:” in the other comments, and hard-wrap the comment on
> the `discovered` field to 80 columns.
>

Mark: Kim and Thomas are rewriting those comments over in
http://cr.openjdk.java.net/~kbarrett/8203028/open.02

Kim and Thomas: Take Mark's suggestions into consideration.

Re: RFR: CSR - JDK-8203428 Predicate::not

[Sundararajan Athijegannathan]

+1

-Sundar

On 18/05/18, 10:05 PM, Jim Laskey wrote:

Introduce a new static method Predicate::not which will allow developers to 
negate predicate lambdas trivially.


csr: https://bugs.openjdk.java.net/browse/JDK-8203428

Re: RFR: CSR - JDK-8203428 Predicate::not

[Daniel Fuchs]

Hi Jim,

Have you thought of introducing something like:

static  Predicate Predicate.of(Predicate target) {
   return target;
}

instead?

I think that might allow you to do things like:


 Stream.of("", "A", "b", "c")
.filter(Predicate.of(String::isEmpty).not())
.filter(Predicate.of("a"::equalsIgnoreCase).or("b"::equalsIgnoreCase))
.count();

best regards,

-- daniel

On 18/05/2018 17:35, Jim Laskey wrote:

Introduce a new static method Predicate::not which will allow developers to 
negate predicate lambdas trivially.


csr: https://bugs.openjdk.java.net/browse/JDK-8203428

Re: RFR: CSR - JDK-8203428 Predicate::not

[Sundararajan Athijegannathan]

Actually it would be:

Predicate.of(String::isEmpty).negate()

But

not(String::isEmpty) reads almost like !str.isEmpty()

-Sundar

On 18/05/18, 10:41 PM, Daniel Fuchs wrote:

Hi Jim,

Have you thought of introducing something like:

static  Predicate Predicate.of(Predicate target) {
   return target;
}

instead?

I think that might allow you to do things like:


 Stream.of("", "A", "b", "c")
.filter(Predicate.of(String::isEmpty).not())
.filter(Predicate.of("a"::equalsIgnoreCase).or("b"::equalsIgnoreCase))
.count();

best regards,

-- daniel

On 18/05/2018 17:35, Jim Laskey wrote:
Introduce a new static method Predicate::not which will allow 
developers to negate predicate lambdas trivially.



csr: https://bugs.openjdk.java.net/browse/JDK-8203428

Re: RFR: JDK-8200380 String::lines

[Paul Sandoz]

Hi Jim,

The Spliterators could be marginally improved:

- the package private constructor need not declare a cs argument, and can pass 
in the characteristics to the other constructor.

- the other constructor can be made private (i use this pattern just to signal 
that it is used internally) and be adjusted to take the fence directly, then 
you don’t need to subtract in the call and then add back in the constructor.


Given the two spliterators are almost identical in functionality it's tempting 
to provide an abstract implementation with two concrete implementation each 
providing their version of a getChar. Possibly not worth it.

Paul.


> On May 18, 2018, at 6:44 AM, Jim Laskey  wrote:
> 
> String::lines instance method that returns a Stream with elements 
> composed of substrings from the original string delimited by any recognized 
> new line character sequence.
> 
> webrev: http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html 
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8200380 
> 
> csr: https://bugs.openjdk.java.net/browse/JDK-8200425 
> 
> 
> 

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Ivan Gerasimov]

Hi Claes!


On 5/18/18 3:51 AM, Claes Redestad wrote:

Hi,

while there are C2 intrinsics on most platforms providing access to 
specialized hardware instructions, e.g., lzcnt on Intel, optimizing 
the java implementations of Integer/Long.numberOfLeadingZeros can 
still be worthwhile, especially if it also helps C1 and implicitly 
startup/warmup. This implementation wins slightly (5-25%) over the 
baseline in all tested optimization modes (-Xint, 
-XX:TieredStopAtLevel=1-3), as well as on C2 if the intrinsics are 
disabled.


Webrev: http://cr.openjdk.java.net/~redestad/8203352/open.00/

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

Correctness is checked by existing tests, mainly 
test/jdk/java/lang/Integer|Long/BitTwiddle.java



I think the Long version needs some adjustment.
The old code correctly handled the case when 32nd bit is the highest bit 
set.
However, the in the new code such a value will be cast to a negative 
int, and all the checks at lines 1774-1777 will fail.


We may want to add such test case to the existing tests.

With kind regards,
Ivan


/Claes




--
With kind regards,
Ivan Gerasimov

Re: RFR: CSR - JDK-8203428 Predicate::not

[Paul Sandoz]

> On May 18, 2018, at 9:35 AM, Jim Laskey  wrote:
> 
> Introduce a new static method Predicate::not which will allow developers to 
> negate predicate lambdas trivially.
> 
> 
> csr: https://bugs.openjdk.java.net/browse/JDK-8203428


+1 thank you for taking action on this.

Predicate not captures the majority use case very concisely and clearly.

I am reluctant to go for an alternative or companion Predicate.of, and would 
need to think carefully about that idiom and it's application on other 
functional interfaces (perhaps we went too far adding such default methods to 
these interfaces…). 

Paul.

Re: RFR: JDK-8200380 String::lines

[Jim Laskey]

Guilty of just cloning the CharacterSpliterator.

Sundar recommended "full on" private final. How about just having the 
characteristics constant in the characteristics() method and dropping the cs 
args and fields altogether? 

— Jim


> On May 18, 2018, at 2:47 PM, Paul Sandoz  wrote:
> 
> Hi Jim,
> 
> The Spliterators could be marginally improved:
> 
> - the package private constructor need not declare a cs argument, and can 
> pass in the characteristics to the other constructor.
> 
> - the other constructor can be made private (i use this pattern just to 
> signal that it is used internally) and be adjusted to take the fence 
> directly, then you don’t need to subtract in the call and then add back in 
> the constructor.
> 
> 
> Given the two spliterators are almost identical in functionality it's 
> tempting to provide an abstract implementation with two concrete 
> implementation each providing their version of a getChar. Possibly not worth 
> it.
> 
> Paul.
> 
> 
>> On May 18, 2018, at 6:44 AM, Jim Laskey  wrote:
>> 
>> String::lines instance method that returns a Stream with elements 
>> composed of substrings from the original string delimited by any recognized 
>> new line character sequence.
>> 
>> webrev: http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html 
>> 
>> bug: https://bugs.openjdk.java.net/browse/JDK-8200380 
>> 
>> csr: https://bugs.openjdk.java.net/browse/JDK-8200425 
>> 
>> 
>> 
> 

Re: RFR: JDK-8200380 String::lines

[Paul Sandoz]

> On May 18, 2018, at 10:57 AM, Jim Laskey  wrote:
> 
> Guilty of just cloning the CharacterSpliterator.
> 

:-) I have done my fair share of spliterator cloning.


> Sundar recommended "full on" private final. How about just having the 
> characteristics constant in the characteristics() method and dropping the cs 
> args and fields altogether? 
> 

Doh! yes, that’s even better.

Paul.

Re: RFR: CSR - JDK-8203428 Predicate::not

[Brian Goetz]

We did discover that default methods on FIs combined with subtyping of 
FIs caused trouble.  But static methods are fine.


If we're going to do Xxx.of(...), we should do it uniformly across FIs, 
not just for Predicate.  I think this is a reasonable move, but we don't 
have to do it right now.  (The benefit is mostly that we pick up 
inference of the type parameters.)


On 5/18/2018 1:54 PM, Paul Sandoz wrote:



On May 18, 2018, at 9:35 AM, Jim Laskey  wrote:

Introduce a new static method Predicate::not which will allow developers to 
negate predicate lambdas trivially.


csr: https://bugs.openjdk.java.net/browse/JDK-8203428


+1 thank you for taking action on this.

Predicate not captures the majority use case very concisely and clearly.

I am reluctant to go for an alternative or companion Predicate.of, and would 
need to think carefully about that idiom and it's application on other 
functional interfaces (perhaps we went too far adding such default methods to 
these interfaces…).

Paul.

Re: [11] RFR: 8202088: Japanese new era implementation

[Roger Riggs]

Hi Naoto,

Is there a reference to the official description or anticipation of the 
new Era?


JapaneseImperialCalendar: 134 NEWERA = 5;   (The real name can also be 
defined later; but still might be more unique as ERA_MAY_1_2019.)


Syntax style:

 - TCKJapaneseChronology:692: align the  columns of decimal values.

 - TestJapaneseChronology:61-62: space before the '}' brackets
    :89: extra space before '}'  //  inconsistent within the file but 
local consistency is good


TestUmmAlQuraChronology: there might be test dates that would not 
require more changes later when the era name changes.


Regards, Roger


On 5/17/18 4:31 PM, Naoto Sato wrote:

Hi,

Please review the changes to the subject issue:

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

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/8202088/webrev.01/

This is the implementation part of the upcoming Japanese new era, 
starting from May 1st, 2019. Current anticipation is that the new name 
won't be known till one month prior to the beginning of the era. So 
it's not possible to make changes to the JDK with the proper name. 
Instead, here we are going to implement the new era with a place 
holder name which will be immediately replaced with the proper name 
once it's known. The CSR is currently under review:


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

Naoto

Discussion: Introduce a `Stream.reject` method, the opposite of `Stream.filter`

[Jacob Glickman]

Seeing as `Predicate.not` was just proposed, I think it would be a good
idea to also introduce `Stream.reject`, which would be logically equivalent
to calling `Stream.filter` with a negated `Predicate`, but with fewer
method calls.

Instead of:

Stream.of("", "A", "B", "C")
 .filter(s -> !s.isEmpty())
 .count();

You could call:

Stream.of("", "A", "B", "C")
 .reject(String::isEmpty)
 .count();

I welcome any suggestions for better names other than `reject`.  If this
suggestion is supported, then I'll happily submit an RFE and the code,
tests, etc.

- Jacob

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Ivan Gerasimov]

On 5/18/18 10:47 AM, Ivan Gerasimov wrote:

Hi Claes!


On 5/18/18 3:51 AM, Claes Redestad wrote:

Hi,

while there are C2 intrinsics on most platforms providing access to 
specialized hardware instructions, e.g., lzcnt on Intel, optimizing 
the java implementations of Integer/Long.numberOfLeadingZeros can 
still be worthwhile, especially if it also helps C1 and implicitly 
startup/warmup. This implementation wins slightly (5-25%) over the 
baseline in all tested optimization modes (-Xint, 
-XX:TieredStopAtLevel=1-3), as well as on C2 if the intrinsics are 
disabled.


Webrev: http://cr.openjdk.java.net/~redestad/8203352/open.00/

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

Correctness is checked by existing tests, mainly 
test/jdk/java/lang/Integer|Long/BitTwiddle.java



I think the Long version needs some adjustment.
The old code correctly handled the case when 32nd bit is the highest 
bit set.
However, the in the new code such a value will be cast to a negative 
int, and all the checks at lines 1774-1777 will fail.


We may want to add such test case to the existing tests.


One possible implementation may be as following:

public static int numberOfLeadingZeros(long i) {
// HD, Count leading 0's
int n = 63;
int x = (int)(i >>> 32);
if (x == 0) {
x = (int)i;
} else {
n -= 32;
}
if (x <= 0)
return x == 0 ? 64 : n - 31;
if (x >= 1 << 16) { n -= 16; x >>>= 16; }
if (x >= 1 <<  8) { n -=  8; x >>>=  8; }
if (x >= 1 <<  4) { n -=  4; x >>>=  4; }
if (x >= 1 <<  2) { n -=  2; x >>>=  2; }
return n - (x >>> 1);
}

Comparing to the base line, it is expected to be slightly slower for 
non-positive longs.
However for positive numbers it may be a tiny-bit faster because we 
replace long-comparison with int-comparison :)


Another approach may be to just delegate to 
Integer.numberOfLeadingZeros() like this:


public static int numberOfLeadingZeros(long i) {
int x = (int)(i >>> 32);
return x == 0 ? 32 + Integer.numberOfLeadingZeros((int)i)
: Integer.numberOfLeadingZeros(x);
}

By the way, is it possible that we have an intrinsic for 
Integer.numberOfLeadingZeros, but not for Long.numberOfLeadingZeros on 
some platform?

If yes, then the later variant may be preferable.

With kind regards,
Ivan


With kind regards,
Ivan


/Claes






--
With kind regards,
Ivan Gerasimov

Re: RFR: JDK-8200380 String::lines

[Xueming Shen]

On 5/18/18, 6:44 AM, Jim Laskey wrote:

String::lines instance method that returns a Stream  with elements 
composed of substrings from the original string delimited by any recognized new line 
character sequence.

webrev: 
http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html
bug: 
https://bugs.openjdk.java.net/browse/JDK-8200380
csr: 
https://bugs.openjdk.java.net/browse/JDK-8200425



(1) seems like we probably don' t need the "cs" field, and do ?
   @Override
public int characteristics() {
return Spliterator.ORDERED | Spliterator.IMMUTABLE | 
Spliterator.NONNULL;


(2)

 622 private int skipLineSeparator(int start) {
 623 if (start<  fence) {
 624 if (value[start] == '\r') {
 625 int next = start + 1;
 626 if (next<  fence&&  value[next] == '\n') {
 627 return next + 1;
 628 }
 629 }
 630 return start + 1;
 631 }
 632 return fence;
 633 }

  if (value[start++] == '\r') {
  if (start<  fence&&  value[start] == '\n') {
  start++;  // return start + 1;
  }
 }
 return start;

not sure if it's really better or not ?

Re: Discussion: Introduce a `Stream.reject` method, the opposite of `Stream.filter`

[Paul Sandoz]

Hi Jason,

One of the rationals for adding “Predicate.not" is to avoid adding another 
stream operation, such as “reject" or “filterNot", and instead one can use 
"filter(not(String::isEmpty))” rather than “filter(s -> !s.isEmpty())” if so 
desired. (This was the reason why i did not close down the issue, and sat on 
it, and thankfully Jim took action.)

We are trying to avoid adding “overloaded" forms of the same kind of operation, 
such as in this case, or filtering classes, or filtering values etc.

Hth,
Paul.

> On May 18, 2018, at 11:22 AM, Jacob Glickman  wrote:
> 
> Seeing as `Predicate.not` was just proposed, I think it would be a good
> idea to also introduce `Stream.reject`, which would be logically equivalent
> to calling `Stream.filter` with a negated `Predicate`, but with fewer
> method calls.
> 
> Instead of:
> 
>Stream.of("", "A", "B", "C")
> .filter(s -> !s.isEmpty())
> .count();
> 
> You could call:
> 
>Stream.of("", "A", "B", "C")
> .reject(String::isEmpty)
> .count();
> 
> I welcome any suggestions for better names other than `reject`.  If this
> suggestion is supported, then I'll happily submit an RFE and the code,
> tests, etc.
> 
> - Jacob

Re: RFR: CSR - JDK-8203428 Predicate::not

[Remi Forax]

- Mail original -
> De: "Brian Goetz" 
> À: "Paul Sandoz" , "Jim Laskey" 
> 
> Cc: "core-libs-dev" 
> Envoyé: Vendredi 18 Mai 2018 20:08:42
> Objet: Re: RFR: CSR - JDK-8203428 Predicate::not

> We did discover that default methods on FIs combined with subtyping of
> FIs caused trouble.  But static methods are fine.
> 
> If we're going to do Xxx.of(...), we should do it uniformly across FIs,
> not just for Predicate.  I think this is a reasonable move, but we don't
> have to do it right now.  (The benefit is mostly that we pick up
> inference of the type parameters.)

we can also:
  - allow diamond syntax on cast
  - teach the inference that when a method is called on a method reference, 
it's maybe an automorphism, so the target type could be back-propagated as the 
target type of the method reference and then the method can be checked from 
left to right (it's maybe too magical but it seems to work great in practice)
  - when inference fails, pick the closest interface from java.util.function to 
the function type, i.e we abandon the idea to introduce real function type in 
the future for more convenience.

regards,
Rémi

> 
> On 5/18/2018 1:54 PM, Paul Sandoz wrote:
>>
>>> On May 18, 2018, at 9:35 AM, Jim Laskey  wrote:
>>>
>>> Introduce a new static method Predicate::not which will allow developers to
>>> negate predicate lambdas trivially.
>>>
>>>
>>> csr: https://bugs.openjdk.java.net/browse/JDK-8203428
>>
>> +1 thank you for taking action on this.
>>
>> Predicate not captures the majority use case very concisely and clearly.
>>
>> I am reluctant to go for an alternative or companion Predicate.of, and would
>> need to think carefully about that idiom and it's application on other
>> functional interfaces (perhaps we went too far adding such default methods to
>> these interfaces…).
>>
> > Paul.

Re: RFR: CSR - JDK-8203428 Predicate::not

[Jim Laskey]

I’m going to pull a Brian here and say, “not” should stand on its own merits. 
The “of” discussion should diverge to it’s own RFE. :-)

> On May 18, 2018, at 3:41 PM, Remi Forax  wrote:
> 
> - Mail original -
>> De: "Brian Goetz" 
>> À: "Paul Sandoz" , "Jim Laskey" 
>> 
>> Cc: "core-libs-dev" 
>> Envoyé: Vendredi 18 Mai 2018 20:08:42
>> Objet: Re: RFR: CSR - JDK-8203428 Predicate::not
> 
>> We did discover that default methods on FIs combined with subtyping of
>> FIs caused trouble.  But static methods are fine.
>> 
>> If we're going to do Xxx.of(...), we should do it uniformly across FIs,
>> not just for Predicate.  I think this is a reasonable move, but we don't
>> have to do it right now.  (The benefit is mostly that we pick up
>> inference of the type parameters.)
> 
> we can also:
>  - allow diamond syntax on cast
>  - teach the inference that when a method is called on a method reference, 
> it's maybe an automorphism, so the target type could be back-propagated as 
> the target type of the method reference and then the method can be checked 
> from left to right (it's maybe too magical but it seems to work great in 
> practice)
>  - when inference fails, pick the closest interface from java.util.function 
> to the function type, i.e we abandon the idea to introduce real function type 
> in the future for more convenience.
> 
> regards,
> Rémi
> 
>> 
>> On 5/18/2018 1:54 PM, Paul Sandoz wrote:
>>> 
 On May 18, 2018, at 9:35 AM, Jim Laskey  wrote:
 
 Introduce a new static method Predicate::not which will allow developers to
 negate predicate lambdas trivially.
 
 
 csr: https://bugs.openjdk.java.net/browse/JDK-8203428
>>> 
>>> +1 thank you for taking action on this.
>>> 
>>> Predicate not captures the majority use case very concisely and clearly.
>>> 
>>> I am reluctant to go for an alternative or companion Predicate.of, and would
>>> need to think carefully about that idiom and it's application on other
>>> functional interfaces (perhaps we went too far adding such default methods 
>>> to
>>> these interfaces…).
>>> 
>>> Paul.

Re: RFR: JDK-8200380 String::lines

[Jim Laskey]

Made the change to 1) already (based on Paul’s remarks)

2) skipLineSeparator needs to handle the end of string with no terminator case 
(start == fence.)  Could change it but then would have to change other code 
around.
 

> On May 18, 2018, at 3:33 PM, Xueming Shen  wrote:
> 
> On 5/18/18, 6:44 AM, Jim Laskey wrote:
>> 
>> String::lines instance method that returns a Stream with elements 
>> composed of substrings from the original string delimited by any recognized 
>> new line character sequence.
>> 
>> webrev: http://cr.openjdk.java.net/~jlaskey/8200380/webrev/index.html 
>>  
>>  
>> 
>> bug: https://bugs.openjdk.java.net/browse/JDK-8200380 
>>  
>>  
>> 
>> csr: https://bugs.openjdk.java.net/browse/JDK-8200425 
>>  
>>  
>> 
>> 
>> 
> (1) seems like we probably don' t need the "cs" field, and do ?
>@Override
> public int characteristics() {
> return Spliterator.ORDERED | Spliterator.IMMUTABLE | 
> Spliterator.NONNULL;
> 
> (2) 
>   622 private int skipLineSeparator(int start) {
>  623 if (start < fence) {
>  624 if (value[start] == '\r') {
>  625 int next = start + 1;
>  626 if (next < fence && value[next] == '\n') {
>  627 return next + 1;
>  628 }
>  629 }
>  630 return start + 1;
>  631 }
>  632 return fence;
>  633 }
> 
>   if (value[start++] == '\r') {
>   if (start < fence && value[start] == '\n') {
>   start++;  // return start + 1;
>   }
>  }
>  return start;
> 
> not sure if it's really better or not ?
> 

Re: RFR: CSR - JDK-8203428 Predicate::not

[forax]

Hi Jim,

- Mail original -
> De: "Jim Laskey" 
> À: "core-libs-dev" 
> Cc: "Remi Forax" , "Brian Goetz" , 
> "Paul Sandoz" 
> Envoyé: Vendredi 18 Mai 2018 20:48:35
> Objet: Re: RFR: CSR - JDK-8203428 Predicate::not

> I’m going to pull a Brian here and say, “not” should stand on its own merits.
> The “of” discussion should diverge to it’s own RFE. :-)

i agree,
sorry to not have been crystal clear, it my head it was clear that that 
everything i've written was a way to avoid Predicate.of() and not something 
should stop the inclusion of Predicate.not, stackoverflow has already decided 
that Predicate.not was the right method [1].

Rémi 

[1] 
https://stackoverflow.com/questions/21488056/how-to-negate-a-method-reference-predicate

> 
>> On May 18, 2018, at 3:41 PM, Remi Forax  wrote:
>> 
>> - Mail original -
>>> De: "Brian Goetz" 
>>> À: "Paul Sandoz" , "Jim Laskey"
>>> 
>>> Cc: "core-libs-dev" 
>>> Envoyé: Vendredi 18 Mai 2018 20:08:42
>>> Objet: Re: RFR: CSR - JDK-8203428 Predicate::not
>> 
>>> We did discover that default methods on FIs combined with subtyping of
>>> FIs caused trouble.  But static methods are fine.
>>> 
>>> If we're going to do Xxx.of(...), we should do it uniformly across FIs,
>>> not just for Predicate.  I think this is a reasonable move, but we don't
>>> have to do it right now.  (The benefit is mostly that we pick up
>>> inference of the type parameters.)
>> 
>> we can also:
>>  - allow diamond syntax on cast
>>  - teach the inference that when a method is called on a method reference, 
>> it's
>>  maybe an automorphism, so the target type could be back-propagated as the
>>  target type of the method reference and then the method can be checked from
>>  left to right (it's maybe too magical but it seems to work great in 
>> practice)
>>  - when inference fails, pick the closest interface from java.util.function 
>> to
>>  the function type, i.e we abandon the idea to introduce real function type 
>> in
>>  the future for more convenience.
>> 
>> regards,
>> Rémi
>> 
>>> 
>>> On 5/18/2018 1:54 PM, Paul Sandoz wrote:
 
> On May 18, 2018, at 9:35 AM, Jim Laskey  wrote:
> 
> Introduce a new static method Predicate::not which will allow developers 
> to
> negate predicate lambdas trivially.
> 
> 
> csr: https://bugs.openjdk.java.net/browse/JDK-8203428
 
 +1 thank you for taking action on this.
 
 Predicate not captures the majority use case very concisely and clearly.
 
 I am reluctant to go for an alternative or companion Predicate.of, and 
 would
 need to think carefully about that idiom and it's application on other
 functional interfaces (perhaps we went too far adding such default methods 
 to
 these interfaces…).
 
> >>> Paul.

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Claes Redestad]

On 2018-05-18 20:23, Ivan Gerasimov wrote:
Another approach may be to just delegate to 
Integer.numberOfLeadingZeros() like this:


    public static int numberOfLeadingZeros(long i) {
    int x = (int)(i >>> 32);
    return x == 0 ? 32 + Integer.numberOfLeadingZeros((int)i)
    : Integer.numberOfLeadingZeros(x);
    }


I'm partial to the simplicity of this solution - whether or not there 
are platforms where 64-bit
lz instruction is missing. It's very slightly slower than baseline for 
-Xint, but markedly faster

when C1-compiled, which I think weighs heavier.

http://cr.openjdk.java.net/~redestad/8203352/open.01/

/Claes

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Brian Burkhalter]

On May 18, 2018, at 7:42 AM, Brian Burkhalter  
wrote:

> On May 18, 2018, at 7:40 AM, Alan Bateman  wrote:
> 
>>> The offset in the string comparison is necessary as “C:\\pagefile.sys” 
>>> becomes “pagefile.sys” in fileData.cFileName. I’m not certain that this 
>>> will work in all cases however but it is certainly no worse than before.
>>> 
>> Can the call to _wstati64 be removed? That is, if GetFileAttributesExW fails 
>> with a sharing violation then just use FindFirstFileW as the fallback.
> 
> Had not tried that yet but I will.


_wstati64 branch excised and verified to pass sans and with VS 2017.

http://cr.openjdk.java.net/~bpb/8202076/webrev.01/

Is this good to go?

Thanks,

Brian

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

[Tony Printezis]

Hi again,

Stylistically, I strongly prefer this version over the previous one (the
one with the doubly-linked list of JdkThreadLocal entries) you had posted.
This one is a lot cleaner.

A few suggestions:

* I’m not crazy about the fact that the users have to override
calculateInitialValue() instead of initialValue() as it will be a bit
error-prone. If they accidentally override initialValue() then the
per-thread registering is not going to happen. Maybe I’m overthinking this.
One way to get round this is for each JdkThreadLocal instance to delegate
calls to get(), set(), and remove() to a private ThreadLocal instance,
which can in turn delegate initialValue() to the outer instance. (Hope this
makes sense?) I did a quick prototype of this and it seems to work, but I
didn’t heavily test it.

* I would prefer if the method users override actually took the
thread-local value as a parameter, i.e., protected void threadTerminated(T
value). This is very easy to add:

protected void threadTerminated(T value) {
}

private void threadTerminated() {
threadTerminated(get());
}

and I think it will be easier to use, as most uses will probably need to do
get() anyway.

Tony

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


On May 18, 2018 at 4:23:19 AM, Peter Levart (peter.lev...@gmail.com) wrote:

It's good to have alternative implementations on the table. Here's another
variant that is space neutral for threads that don't use JdkThreadLocal,
but also scales to many JdkThreadLocal instances:

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

Now we only need an arbiter to decide which one! :-)

Regards, Peter

On 05/17/18 22:39, Peter Levart wrote:

Hi Tony,

If we anticipate only small number of JdkThreadLocal(s) (there will be only
one for the start) then this is a plausible solution. Then perhaps this
limitation should be written somewhere in the JdkThreadLocal javadoc so
that one day somebody will not be tempted to use JdkThreadLocal(s) en
masse. Let's say there will be a few more JdkThreadLocal(s) in the future.
Are we willing to pay for a few lookups into a ThreadLocalMap at each and
every thread's exit even though such thread did not register a mapping for
any JdkThreadLocal? Is an additional reference field in each and every
ThreadLocalMap (one per Thread that uses thread locals) a bigger price to
pay? I don't know. Will let others comment on this.

Otherwise the code looks good. Just a couple of observations:

Since private static method JdkThreadLocal.add is only called from
JdkThreadLocal constructor with just constructed instance (this), there's
no possibility for it to be called twice or more times with the same
instance. The check for duplicates could go away then, right?

You keep an array of Entry objects which are just wrappers for
JdkThreadLocal objects. Are you already planning for Entry to become a
WeakReference? Otherwise you could just keep JdkThreadLocal objects in the
array directly.

Regards, Peter

On 05/17/18 20:25, Tony Printezis wrote:

Hi all,

I have a new version of the code for your consideration:

http://cr.openjdk.java.net/~tonyp/8202788/webrev.1/

I basically combined our two approaches. The usage is as Alan had proposed
it: Users have to use JdkThreadLocal (which is only available within
java.base) and override threadTerminated(). However, I keep track of
JdkThreadLocal instances globally (as I did before) and not per-thread.
This way we don’t need to add any unnecessary complexity to ThreadLocalMap.

Currently, I don’t allow entries to be purged if the JdkThreadLocal
instance becomes otherwise unreachable. I can easily add that functionality
if needed (I can use WeakReferences for that). However, for the uses we’re
considering, is it really necessary?

Thoughts?

Tony


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


On May 14, 2018 at 12:40:28 PM, Tony Printezis (tprinte...@twitter.com)
wrote:

Peter,

In my proposal, you can register the exit hook in the ThreadLocal c’tor, so
it’s almost as nice as Alan’s in that respect (and it doesn't require an
extra field per ThreadLocal plus two extra fields per JdkEntry). :-)

But, I do like the addition of the JdkEntry list to avoid unnecessarily
iterating over all the map entries (which was my main concern with Alan’s
original webrev). I’ll be totally happy with a version of this.

Tony


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


On May 12, 2018 at 6:44:08 AM, Peter Levart (peter.lev...@gmail.com) wrote:

Hi,

On 05/11/18 16:13, Alan Bateman wrote:

On 08/05/2018 16:07, Tony Printezis wrote:

Hi all,

Following the discussion on this a few weeks ago, here’s the first version
of the change:

http://cr.openjdk.java.net/~tonyp/8202788/webrev.0/

I think the consensus was that it’d be easier if the exit hooks were only
available within java.base. Is it enough that I added the functionality to
the jdk.internal.misc package? (And is jdk.internal.misc the be

Re: RFR: CSR - JDK-8203428 Predicate::not

[Remi Forax]

Hi Jim,
here is my review,
the target should use a wildcard,
  static  Predicate not(Predicate target) {
return ...
  }

Due to a limitation of the inference, 'return target.negate()' will not compile.

One may think that, we should write 
  static  Predicate not(Predicate target) {
return t -> !target.test(t);
  }
but if the class of 'target' override negate(), we will not call it.

So the right code seems to be
  @SuppressWarnings("unchecked")  // Predicate is contra-variant
  static  Predicate not(Predicate target) {
return (Predicate)target.negate();
  }

at least until JEP 300 [1] is implemented.

regards,
Rémi
  
[1] https://bugs.openjdk.java.net/browse/JDK-8043488

- Mail original -
> De: "Jim Laskey" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 18 Mai 2018 18:35:24
> Objet: RFR:  CSR - JDK-8203428 Predicate::not

> Introduce a new static method Predicate::not which will allow developers to
> negate predicate lambdas trivially.
> 
> 
> csr: https://bugs.openjdk.java.net/browse/JDK-8203428

Easy and safe cloning of methods, constructors, fields and parameters.

[Kasper Nielsen]

Hi,

I'm making heavy use of class in java.lang.reflect such as methods,
constructors, fields and parameters in a library that I am trying to
secure. I both take them as parameters and return them from various
methods. Internally, I need to keep "safe" copies with the accessible bit
set. I do not want to expose these "safe" copies to the outside, but I want
to cache them for performance reasons.

However, I find it rather cumbersome to make safe copies of all of these.
Take, for example, a Constructor. If I want to return a copy
of a Constructor with the setAccessible set to false I need to do this:

Class declaringClass = constructor.getDeclaringClass();
try {
return declaringClass.getConstructor(constructor.getParameterTypes());
} catch (NoSuchMethodException e) {
throw new RuntimeException("This should never happen", e);
}

Copying a Method looks similar.

And if I want to make a safe copy of a parameter. I first need to clone its
declaring executable. And then run through all the parameters and see if
they are equal, because Parameter does not expose the index field.

Thoughts?

/Kasper

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Ivan Gerasimov]

Hi Brian!


On 5/18/18 12:18 PM, Brian Burkhalter wrote:

On May 18, 2018, at 7:42 AM, Brian Burkhalter  
wrote:


On May 18, 2018, at 7:40 AM, Alan Bateman  wrote:


The offset in the string comparison is necessary as “C:\\pagefile.sys” becomes 
“pagefile.sys” in fileData.cFileName. I’m not certain that this will work in 
all cases however but it is certainly no worse than before.


Can the call to _wstati64 be removed? That is, if GetFileAttributesExW fails 
with a sharing violation then just use FindFirstFileW as the fallback.

Had not tried that yet but I will.


_wstati64 branch excised and verified to pass sans and with VS 2017.

http://cr.openjdk.java.net/~bpb/8202076/webrev.01/

Is this good to go?


It looks sensible to check the file name after a call to FindFirstFileW, 
but I have a concern.


What if pathbuf contained an "alternative" name of the directory and/or 
the file itself (e.g. "C:\\Progra~1\\file" instead of "C:\\Program 
Files\\file")?
I guess that FindFirstFileW() may expand the name to normal length (I'm 
not certain it will really do), so that content of fileData.cFileName 
will be longer than pathbuf.
Then, the call to wcscmp() would read the memory before the start of 
pathbuf.


In fact, we only need to check that that only file name (without the 
directory name) is the same, so maybe it will be safer to compare the 
substrings starting at the very last backslash?
If the check fails, then we may want to try to see if pathbuf ends with 
fileData.cAlternateFileName.


I understand, it might look like too much work for a rare edge case :)

With kind regards,
Ivan


Thanks,

Brian


--
With kind regards,
Ivan Gerasimov

Re: RFR: jsr166 jdk integration 2018-05

[Paul Sandoz]

Hi,

My CME budget is definitely in the red now :-) but i would like to attempt to 
summarize the different positions and suggest a solution. If that does not get 
traction i believe a graceful retreat is in order.  

We appear to be in two different philosophical camps with regards to the 
interpretation of modCount (i suspect what both camps believe is compatible 
with what CME specifies [*]).

One camp believes AbstractList.modCount should only (on a best-effort basis) be 
incremented on structural modifications that change the size (even if the size 
change cannot be externally observed, like the example Doug presents).

The other camp believes there are other forms of modification that perturb the 
list leading to incorrect results since those forms can affect yet to be 
traversed elements. It is argued that bulk operations such as replaceAll and 
sort fit in this category, regardless of the implementation.

I hope that is an accurate representation, and if that is so here is a rough 
proposal bring those camps together. The specification of AbstractList.modCount 
could be modified as follows:

/**
 * The number of times this list has been structurally modified
 * or globally modified.
 * Structural modifications are those that change the size of the
 * list, or otherwise perturb it in such a fashion that iterations in
 * progress may yield incorrect results.
 * Global modifications are those where the size may not change but the 
 * list is perturbed as a whole in such a fashion that iterations in progress 
 * may yield incorrect results.

Thoughts?

Paul.

[*] on CME
* Note that this exception does not always indicate that an object has
* been concurrently modified by a different thread.  If a single
* thread issues a sequence of method invocations that violates the
* contract of an object, the object may throw this exception.  For
* example, if a thread modifies a collection directly while it is
* iterating over the collection with a fail-fast iterator, the iterator
* will throw this exception.
*
* Note that fail-fast behavior cannot be guaranteed as it is, generally
* speaking, impossible to make any hard guarantees in the presence of
* unsynchronized concurrent modification.  Fail-fast operations
* throw {@code ConcurrentModificationException} on a best-effort basis.
* Therefore, it would be wrong to write a program that depended on this
* exception for its correctness: {@code ConcurrentModificationException}
* should be used only to detect bugs.


> On May 16, 2018, at 4:04 PM, Doug Lea  wrote:
> 
> 
> Sorry Stuart, but I'm joining the no-modCount barrage.
> To pick on the main issue...
> 
> On 05/16/2018 05:21 PM, Stuart Marks wrote:
> 
>> Suppose that a replaceAll() on another thread occurs, and that this is
>> allowed. Does the application care whether the eventual printout
>> contains partly new values and partly old values? How can you tell? It
>> seems to me that this is more likely a programming error than a valid
>> use case.
> 
> There are many data-races that are never detected via CME. CME was
> designed to trap only some of them (and even at that, only heuristically
> because modCount operations are not strictly ordered). ModCount
> mechanics only deal with insertions and removals. Failing to catch a
> race in replaceAll is no different than failing to catch one with
> multiple concurrent setValues. It might be "nice" to adjust modCount on
> any operation that might contribute to a datarace, but no reason to
> single out replaceAll.
> 
> Having said this, I don't think that anything in the spec (vs default
> implementation) prohibits an implementation of replaceAll from removing
> and then re-inserting each element, in which case an implementation
> would modify modCounts.
> 
> -Doug
> 

Re: RFR (JDK11/JAXP/java.xml) 8198548: Initialization race in com.sun.org.apache.xerces.internal.impl.xpath.regex.Token.getRange() on Token.categories

[Lance Andersen]

Hi Joe,

The current webrev looks fine.

> On May 16, 2018, at 7:40 PM, Joe Wang  wrote:
> 
> Hi,
> 
> Please review a fix on synchronization. The real change is the double-checked 
> locking, all others are s/Token.categories/tmpMap/g.
> 
> All JAXP tests passed. Before the fix, the test attached to JBS failed only 
> once for me after 10,000 runs and then a few 1000 runs (impossible to get one 
> with only 20 runs as the comment mentioned).  The author of the original 
> report put the actual pass rate at 99.994%. After the fix then, in theory it 
> should be 100%, I've made a 100,000 run without seeing a failure.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8198548
> webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8198548/webrev/
> 
> Thanks,
> Joe

 
  

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

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Brian Burkhalter]

Hi Ivan,

On May 18, 2018, at 2:24 PM, Ivan Gerasimov  wrote:

> In fact, we only need to check that that only file name (without the 
> directory name) is the same, so maybe it will be safer to compare the 
> substrings starting at the very last backslash?
> If the check fails, then we may want to try to see if pathbuf ends with 
> fileData.cAlternateFileName.

Indeed it might make more sense to locate the last backslash using strrchr or 
equivalent and then do the comparison against the returned pointers without a 
length.

Thanks,

Brian

Re: RFR (JDK11/JAXP/java.xml) 8198548: Initialization race in com.sun.org.apache.xerces.internal.impl.xpath.regex.Token.getRange() on Token.categories

[Joe Wang]

Thanks Lance!  Running one last test. Will check in once the tests pass.

Best,
Joe

On 5/18/2018 2:26 PM, Lance Andersen wrote:

Hi Joe,

The current webrev looks fine.

On May 16, 2018, at 7:40 PM, Joe Wang > wrote:


Hi,

Please review a fix on synchronization. The real change is the 
double-checked locking, all others are s/Token.categories/tmpMap/g.


All JAXP tests passed. Before the fix, the test attached to JBS 
failed only once for me after 10,000 runs and then a few 1000 runs 
(impossible to get one with only 20 runs as the comment mentioned). 
 The author of the original report put the actual pass rate at 
99.994%. After the fix then, in theory it should be 100%, I've made a 
100,000 run without seeing a failure.


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



Thanks,
Joe




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Ivan Gerasimov]

I vote for this variant.

With kind regards,

Ivan


On 5/18/18 12:05 PM, Claes Redestad wrote:



On 2018-05-18 20:23, Ivan Gerasimov wrote:
Another approach may be to just delegate to 
Integer.numberOfLeadingZeros() like this:


public static int numberOfLeadingZeros(long i) {
int x = (int)(i >>> 32);
return x == 0 ? 32 + Integer.numberOfLeadingZeros((int)i)
: Integer.numberOfLeadingZeros(x);
}


I'm partial to the simplicity of this solution - whether or not there 
are platforms where 64-bit
lz instruction is missing. It's very slightly slower than baseline for 
-Xint, but markedly faster

when C1-compiled, which I think weighs heavier.

http://cr.openjdk.java.net/~redestad/8203352/open.01/

/Claes



--
With kind regards,
Ivan Gerasimov

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Brian Burkhalter]

Hi Ivan,

On May 18, 2018, at 2:30 PM, Brian Burkhalter  
wrote:

> On May 18, 2018, at 2:24 PM, Ivan Gerasimov  wrote:
> 
>> In fact, we only need to check that that only file name (without the 
>> directory name) is the same, so maybe it will be safer to compare the 
>> substrings starting at the very last backslash?
>> If the check fails, then we may want to try to see if pathbuf ends with 
>> fileData.cAlternateFileName.
> 
> Indeed it might make more sense to locate the last backslash using strrchr or 
> equivalent and then do the comparison against the returned pointers without a 
> length.

Here is another version which incorporates your suggestion:

http://cr.openjdk.java.net/~bpb/8202076/webrev.02/

Thanks,

Brian

Re: [11] RFR: 8202088: Japanese new era implementation

[naoto . sato]

Hi Roger, thank you for the comments.

On 5/18/18 11:11 AM, Roger Riggs wrote:

Hi Naoto,

Is there a reference to the official description or anticipation of the 
new Era?


AFAIK, the most recent information was for Japanese Govt. to announce 
the new era name one month prior to the ascension. This is indeed the 
reason I decided to introduce the placeholder.




JapaneseImperialCalendar: 134 NEWERA = 5;   (The real name can also be 
defined later; but still might be more unique as ERA_MAY_1_2019.)


I wanted to keep the name "NEWERA" for the convenience when they are to 
be replaced with the real name. I changed the access modifier to 
"private", though.




Syntax style:

  - TCKJapaneseChronology:692: align the  columns of decimal values.

  - TestJapaneseChronology:61-62: space before the '}' brackets
     :89: extra space before '}'  //  inconsistent within the file but 
local consistency is good


TestUmmAlQuraChronology: there might be test dates that would not 
require more changes later when the era name changes.


Fixed as suggested. The updated webrev is here:

http://cr.openjdk.java.net/~naoto/8202088/webrev.02/

Naoto



Regards, Roger


On 5/17/18 4:31 PM, Naoto Sato wrote:

Hi,

Please review the changes to the subject issue:

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

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/8202088/webrev.01/

This is the implementation part of the upcoming Japanese new era, 
starting from May 1st, 2019. Current anticipation is that the new name 
won't be known till one month prior to the beginning of the era. So 
it's not possible to make changes to the JDK with the proper name. 
Instead, here we are going to implement the new era with a place 
holder name which will be immediately replaced with the proper name 
once it's known. The CSR is currently under review:


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

Naoto

Re: RFR: 8203352: Improve java implementation of Integer/Long.numberOfLeadingZeros

[Martin Buchholz]

+1

Consider checking in your exhaustive test in such a way that it doesn't get
run automatically in exhaustive mode by jtreg, perhaps controlled by a
system property.

On Fri, May 18, 2018 at 2:44 PM, Ivan Gerasimov 
wrote:

> I vote for this variant.
>
> With kind regards,
>
> Ivan
>
>
>
> On 5/18/18 12:05 PM, Claes Redestad wrote:
>
>>
>>
>> On 2018-05-18 20:23, Ivan Gerasimov wrote:
>>
>>> Another approach may be to just delegate to
>>> Integer.numberOfLeadingZeros() like this:
>>>
>>> public static int numberOfLeadingZeros(long i) {
>>> int x = (int)(i >>> 32);
>>> return x == 0 ? 32 + Integer.numberOfLeadingZeros((int)i)
>>> : Integer.numberOfLeadingZeros(x);
>>> }
>>>
>>
>> I'm partial to the simplicity of this solution - whether or not there are
>> platforms where 64-bit
>> lz instruction is missing. It's very slightly slower than baseline for
>> -Xint, but markedly faster
>> when C1-compiled, which I think weighs heavier.
>>
>> http://cr.openjdk.java.net/~redestad/8203352/open.01/
>>
>> /Claes
>>
>>
> --
> With kind regards,
> Ivan Gerasimov
>
>

Re: RFR: jsr166 jdk integration 2018-05

[joe darcy]

Hello,

On 5/17/2018 8:51 PM, Stuart Marks wrote:
I owe all of you replies on this, but I don't have time to continue 
the discussion, and I suspect you all want to get on with things.


Let me have a quick chat with Paul tomorrow and then I'll propose a 
path forward.




I've spoken about this proposed change with Stuart and Paul.

Given that the replaceAll methods have incremented modCount since they 
were introduced in JDK 8, changing that behavior in widely-used classes 
merits a CSR to be filed for JDK 11.


The modCount aspect of the changeset could be split out if you want to 
proceed with the rest.


Separately, it would be worthwhile to have a general discussion and 
review of the desired semantics for modCount and consistency of the 
various replaceAll implementations, including reviewing Map.replaceAll​.


Thanks,

-Joe

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Ivan Gerasimov]

Thanks Brian, the last webrev looks good to me!

I minor suggestion is that I would recommend to use L'\\' to explicitly 
indicate it is a wide char literal.


And the last comment:  Maybe it makes sense to update the test 
java/io/File/WinSpecialFiles.java to make sure we deal correctly with 
wildcards in a file name (and that nobody will accidentally optimize 
away the check of the filename).


With kind regards,
Ivan

On 5/18/18 3:08 PM, Brian Burkhalter wrote:

Hi Ivan,

On May 18, 2018, at 2:30 PM, Brian Burkhalter 
mailto:brian.burkhal...@oracle.com>> wrote:


On May 18, 2018, at 2:24 PM, Ivan Gerasimov 
mailto:ivan.gerasi...@oracle.com>> wrote:


In fact, we only need to check that that only file name (without the 
directory name) is the same, so maybe it will be safer to compare 
the substrings starting at the very last backslash?
If the check fails, then we may want to try to see if pathbuf ends 
with fileData.cAlternateFileName.


Indeed it might make more sense to locate the last backslash using 
strrchr or equivalent and then do the comparison against the returned 
pointers without a length.


Here is another version which incorporates your suggestion:

http://cr.openjdk.java.net/~bpb/8202076/webrev.02/ 



Thanks,

Brian


--
With kind regards,
Ivan Gerasimov

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Brian Burkhalter]

Hi Ivan,

All good ideas for next week!

Thanks,

Brian

On May 18, 2018, at 4:49 PM, Ivan Gerasimov  wrote:

> Thanks Brian, the last webrev looks good to me!
> 
> I minor suggestion is that I would recommend to use L'\\' to explicitly 
> indicate it is a wide char literal.
> 
> And the last comment:  Maybe it makes sense to update the test 
> java/io/File/WinSpecialFiles.java to make sure we deal correctly with 
> wildcards in a file name (and that nobody will accidentally optimize away the 
> check of the filename).

FW: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch

[Vivek Theeyarath]

Hi All,

   A gentle reminder seeking review.

Regards

Vivek

From: Vivek Theeyarath 
Sent: Thursday, May 17, 2018 6:22 AM
To: core-libs-dev@openjdk.java.net
Subject: RFR: 8177276: MethodHandles.insertArguments doesn't specify 
IllegalArgumentException on index mismatch

 

Hi All,

 Please review fix for https://bugs.openjdk.java.net/browse/JDK-8177276 
. 

 

http://cr.openjdk.java.net/~vtheeyarath/8177276/webrev.02/

 

The related CSR is https://bugs.openjdk.java.net/browse/JDK-8202991 .

 

Regards

Vivek

Reimplementing sun.nio.ch.WindowsSelectorImpl using WSAEventSelect and WSAWaitForMultipleEvents instead of a wakeup socket

[John Platts]

The need to allocate a wakeup socket can be eliminated in 
sun.nio.ch.WindowsSelectorImpl on Windows platforms by reimplementing 
sun.nio.ch.WindowsSelectorImpl to use the WSAEventSelect and 
WSAWaitForMultipleEvents APIs introduced in Windows Vista. If WSAEventSelect 
and WSAWaitForMultipleEvents are used, a wakeup can be done by simply signaling 
the wakeup event by calling WSASetEvent.

Here is how to do a select using WSAEventSelect and WSAWaitForMultipleEvents on 
Windows platforms:
1. Allocate events for selection and wakeup by calling the WSACreateEvent() 
function.
2. Perform a select operation by calling WSAEventSelect method, passing in the 
event object used for selection.
3. Call the WSAWaitForMultipleEvents function to wait until one of the events 
is signaled.
4. For the remaining events, call WSAWaitForMultipleEvents with a timeout of 0 
to check if the event is signaled.
5. For each of the selection events that are signaled, call the 
WSAEnumNetworkEvents method to check if the socket is ready for accept, 
connect, read, or write operations.
6. To cancel selection, call WSAEventSelect with 0 passed into the 
lNetworkEvents argument (the third argument).
7. Free the events allocated by the WSACreateEvent() function by calling the 
WSACloseEvent() function.

Re: 8202076: test/jdk/java/io/File/WinSpecialFiles.java on windows with VS2017

[Alan Bateman]

On 18/05/2018 23:08, Brian Burkhalter wrote:

:
Here is another version which incorporates your suggestion:

http://cr.openjdk.java.net/~bpb/8202076/webrev.02/

GetFileAttributesEx follows sym links, FindFirstFile does not. You 
should be able to quickly check it but I think the patch means that it 
will return the size of the link file when it linked to an lock file. 
Assuming I have this right, then you'll need to look at the 
dwFileAttributes field in the WIN32_FIND_DATAW structure and return 0 
when the attribute to indicate a reparse point is set.


BTW: Do you know if anyone has submitted a bug to Microsoft on this? 
This issue is really about working around a regression in the runtime 
that comes with the VS2017.


-Alan