Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Vitaly Davidovich
On Wednesday, September 28, 2016, Carsten Varming wrote: > Dear Vitaly and David, > > Looking at your webrev the original code is: > > public int hashCode() { > if (hash == 0 && value.length > 0) { > hash = isLatin1() ? StringLatin1.hashCode(value) > } > return

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Martin Buchholz
On Wed, Sep 28, 2016 at 9:33 AM, Volker Simonis wrote: > > I don't think this can be easily done with the current build system. > Remember for example that even such a sensitive part like jni.h is > still duplicated between the hotspot and the jdk repository: > >

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Carsten Varming
Dear Vitaly and David, Looking at your webrev the original code is: public int hashCode() { if (hash == 0 && value.length > 0) { hash = isLatin1() ? StringLatin1.hashCode(value) } return hash; } There is a constructor: public String(String original) { this.value = original.value;

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Volker Simonis
On Wed, Sep 28, 2016 at 3:03 PM, Kumar Srinivasan wrote: > > Hello Thomas, Volker, > > > Hi Kumar, > >> >> On 9/16/2016 10:34 AM, Volker Simonis wrote: >>> >>> Hi Christoph, >>> >>> I think your change is fine as a quick-fix to fix the build. But >>> you're

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread John Rose
On Sep 28, 2016, at 7:40 AM, Krystal Mok wrote: > > Let me post out the HotSpot version of the change and let you guys decide > whether or not you guys want to take that version (which will take care of > the ArrayList$1 case without the JDK-side change). My advice is:

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread David Holmes
On 29/09/2016 3:44 AM, Carsten Varming wrote: Dear Vitaly and David, Looking at your webrev the original code is: public int hashCode() { if (hash == 0 && value.length > 0) { hash = isLatin1() ? StringLatin1.hashCode(value) } return hash; } There is a constructor: public

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Vitaly Davidovich
On Wednesday, September 28, 2016, Carsten Varming wrote: > Dear David, > > See inline. > > On Wed, Sep 28, 2016 at 7:47 PM, David Holmes > wrote: > >> On 29/09/2016 3:44 AM, Carsten Varming

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Vitaly Davidovich
On Wednesday, September 28, 2016, David Holmes wrote: > On 28/09/2016 11:24 PM, Peter Levart wrote: > >> >> On 09/28/2016 03:05 PM, David Holmes wrote: >> >>> On 28/09/2016 10:44 PM, Peter Levart wrote: >>> Hi, According to discussion here:

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Ivan Gerasimov
Hi Claes! I can handle the backport. With kind regards, Ivan On 28.09.2016 17:37, Claes Redestad wrote: I dealt with this in isolation deliberately to ease backporting, but I don't have the 8u committer rights to do it myself. /Claes On 2016-09-28 16:27, Vitaly Davidovich wrote: Thanks

Re: [concurrency-interest] We need to add blocking methods to CompletionStage!

2016-09-28 Thread Gregg Wonderly
The completely less thread consuming alternative is call backs. VMS used/uses Asynchronous System Traps (ASTs) for everything in the 1980s. It was a very performant and friendly way to allow small software modules to be written to do one thing and used as the call back for asynchronous

Re: [concurrency-interest] We need to add blocking methods to CompletionStage!

2016-09-28 Thread k...@kodewerk.com
> On Sep 28, 2016, at 5:05 PM, Gregg Wonderly wrote: > > The completely less thread consuming alternative is call backs. This is a technique that we commonly used in a system that a group of use built in the 90s. The system had many variants on a number of different data

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Thomas Stüfe
On Wed, Sep 28, 2016 at 7:33 PM, Martin Buchholz wrote: > On Wed, Sep 28, 2016 at 9:33 AM, Volker Simonis > wrote: > > > > > I don't think this can be easily done with the current build system. > > Remember for example that even such a sensitive

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Thomas Stüfe
Hi Kumar, On Wed, Sep 28, 2016 at 3:03 PM, Kumar Srinivasan < kumar.x.sriniva...@oracle.com> wrote: > > Hello Thomas, Volker, > > > Hi Kumar, > > >> On 9/16/2016 10:34 AM, Volker Simonis wrote: >> >>> Hi Christoph, >>> >>> I think your change is fine as a quick-fix to fix the build. But >>>

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Chris Bensen
> On Sep 28, 2016, at 11:50 AM, Thomas Stüfe wrote: > > > > On Wed, Sep 28, 2016 at 7:33 PM, Martin Buchholz > wrote: > On Wed, Sep 28, 2016 at 9:33 AM, Volker Simonis

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread David Holmes
On 28/09/2016 11:24 PM, Peter Levart wrote: On 09/28/2016 03:05 PM, David Holmes wrote: On 28/09/2016 10:44 PM, Peter Levart wrote: Hi, According to discussion here: http://cs.oswego.edu/pipermail/concurrency-interest/2016-September/015414.html it seems compact strings introduced (at least

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Carsten Varming
Dear David, See inline. On Wed, Sep 28, 2016 at 7:47 PM, David Holmes wrote: > On 29/09/2016 3:44 AM, Carsten Varming wrote: > >> Dear Vitaly and David, >> >> Looking at your webrev the original code is: >> >> public int hashCode() { >> if (hash == 0 && value.length

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Kumar Srinivasan
One thing I missed to mention, the fix for 8165524 is primarily to assist the packager tool, which deploys client side/JavaFX based applications. If this tool is not relevant and does not exist on AIX. Then at a minimum the dladdr call can either be dynamically located/invoked using dlsym,

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread David Holmes
On 28/09/2016 11:26 PM, Aleksey Shipilev wrote: Peter, David, Would you mind discussing the theoretical topics on concurrency-interest@, for the benefits of others who don't track high-traffic OpenJDK list? Well, noone responded to my comment on c-i, and I didn't expect a patch to appear on

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread David Holmes
On 29/09/2016 10:49 AM, Carsten Varming wrote: Dear David, See inline. On Wed, Sep 28, 2016 at 7:47 PM, David Holmes > wrote: On 29/09/2016 3:44 AM, Carsten Varming wrote: Dear Vitaly and David, Looking at your

Re: RFR(L): 8151179: address issues raised by JCK team on JEP 274 API

2016-09-28 Thread Michael Haupt
Hello, reviews, please ... ? Thanks, Michael > Am 26.09.2016 um 21:01 schrieb Michael Haupt : > > Hi John, all, > > thank you very much for your reviews - may I ask for a second round? > > The updated webrev is at >

Re: RFR(L): 8151179: address issues raised by JCK team on JEP 274 API

2016-09-28 Thread John Rose
On Sep 26, 2016, at 12:01 PM, Michael Haupt wrote: > > thank you very much for your reviews - may I ask for a second round? > > The updated webrev is at > http://cr.openjdk.java.net/~mhaupt/8151179/webrev.01/ >

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Michael Haupt
Hi Claes, yes please. Thumbs up. :-) Best, Michael > Am 28.09.2016 um 13:48 schrieb Claes Redestad : > > Hi, > > as discussed recently on hotspot-compiler-dev[1], having a private class with > no default constructor can lead to C2 failing to inline, due to the

Re: RFR(L): 8151179: address issues raised by JCK team on JEP 274 API

2016-09-28 Thread Michael Haupt
Hi John, thank you very much - some comments are inlined below. > Am 28.09.2016 um 08:26 schrieb John Rose : > > Reviewed. This is a huge leap forward. > > A few small comments to consider: > > In FacLoop, the argument k should be named acc: > +- int fin(int i, int

Re: RFR: 8166287: MultiReleaseJarAPI.isMultiReleaseJar(): failure java.nio.file.AccessDeniedException: custom-mr.jar

2016-09-28 Thread Alan Bateman
On 28/09/2016 11:56, Claes Redestad wrote: Hi, recently added test cases to MultiReleaseJarAPI can cause an AccessDeniedException on some Windows systems, and using a distinct name of the jar generated for each test should avoid this. webrev:

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Remi Forax
yes, thumb up. Rémi On September 28, 2016 1:51:18 PM GMT+02:00, Michael Haupt wrote: >Hi Claes, > >yes please. Thumbs up. :-) > >Best, > >Michael > >> Am 28.09.2016 um 13:48 schrieb Claes Redestad >: >> >> Hi, >> >> as discussed recently

RE: RFR(M): 8166560: [s390] Basic enablement of s390 port.

2016-09-28 Thread Lindenmaier, Goetz
Hi, here a new webrev for this change: http://cr.openjdk.java.net/~goetz/wr16/8166560-basic_s390/hotspot.wr02/ I reverted the major reorderings in macros.hpp and os_linux.cpp. David asked me to do so, and I guess it makes reviewing more simple. Also this fixes the issue spotted by David which

RFR: 8166287: MultiReleaseJarAPI.isMultiReleaseJar(): failure java.nio.file.AccessDeniedException: custom-mr.jar

2016-09-28 Thread Claes Redestad
Hi, recently added test cases to MultiReleaseJarAPI can cause an AccessDeniedException on some Windows systems, and using a distinct name of the jar generated for each test should avoid this. webrev: http://cr.openjdk.java.net/~redestad/8166287/webrev.01/ bug:

Re: RFR(s): 8166841: Unused import causes test failure on compilation in java.text tests

2016-09-28 Thread Ivan Gerasimov
This looks good! With kind regards, Ivan On 28.09.2016 15:16, Sergei Kovalev wrote: Hi Team, Could you please review very small fix? BugID: https://bugs.openjdk.java.net/browse/JDK-8166841 WebRev: http://cr.openjdk.java.net/~skovalev/8166841/webrev.00/ Issue: In case of usage command line

RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Claes Redestad
Hi, as discussed recently on hotspot-compiler-dev[1], having a private class with no default constructor can lead to C2 failing to inline, due to the synthetic bridge constructor using a dummy argument of an uninitialized class. This is really a problem in C2, as well as something which could

Re: RFR: 8166287: MultiReleaseJarAPI.isMultiReleaseJar(): failure java.nio.file.AccessDeniedException: custom-mr.jar

2016-09-28 Thread Michael Haupt
Hi Claes, thumbs up. Best, Michael > Am 28.09.2016 um 12:56 schrieb Claes Redestad : > > Hi, > > recently added test cases to MultiReleaseJarAPI can cause an > AccessDeniedException on some Windows systems, and using a distinct name of > the jar generated for

Re: RFR(s): 8166624: java/util/jar/JarFile/mrjar regression tests has undeclared dependencies

2016-09-28 Thread Alan Bateman
On 27/09/2016 16:03, Sergei Kovalev wrote: Hi Alan, Looks like the root cause is jtreg issue. I'm going to close the CR as "Not an issue". Do you agree? I agree, esp. since this is jtreg bug is already fixed and so the issue will go away once a new version is released/promoted. -Alan

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Claes Redestad
Thanks for the quick reviews! /Claes On 2016-09-28 14:14, Remi Forax wrote: yes, thumb up. Rémi On September 28, 2016 1:51:18 PM GMT+02:00, Michael Haupt wrote: Hi Claes, yes please. Thumbs up. :-) Best, Michael Am 28.09.2016 um 13:48 schrieb Claes Redestad

Re: RFR(s): 8166841: Unused import causes test failure on compilation in java.text tests

2016-09-28 Thread Claes Redestad
+1 /Claes On 2016-09-28 14:16, Sergei Kovalev wrote: Hi Team, Could you please review very small fix? BugID: https://bugs.openjdk.java.net/browse/JDK-8166841 WebRev: http://cr.openjdk.java.net/~skovalev/8166841/webrev.00/ Issue: In case of usage command line option "--limit-modules

RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Peter Levart
Hi, According to discussion here: http://cs.oswego.edu/pipermail/concurrency-interest/2016-September/015414.html it seems compact strings introduced (at least theoretical) non-benign data race into String.hasCode() method. Here is a proposed patch:

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Aleksey Shipilev
On 09/28/2016 02:44 PM, Peter Levart wrote: > http://cr.openjdk.java.net/~plevart/jdk9-dev/8166842_String.hashCode/webrev.01/ +1, thanks. This is partially my fault for not spotting it earlier during the Compact Strings work. -Aleksey

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Aleksey Shipilev
Peter, David, Would you mind discussing the theoretical topics on concurrency-interest@, for the benefits of others who don't track high-traffic OpenJDK list? Thanks, -Aleksey On 09/28/2016 03:24 PM, Peter Levart wrote: > > On 09/28/2016 03:05 PM, David Holmes wrote: >> On 28/09/2016 10:44 PM,

Re: RFR: 8166287: MultiReleaseJarAPI.isMultiReleaseJar(): failure java.nio.file.AccessDeniedException: custom-mr.jar

2016-09-28 Thread Claes Redestad
Michael, Alan, thanks for reviewing. Sure, might as well push to jdk9/dev /Claes On 2016-09-28 14:10, Alan Bateman wrote: On 28/09/2016 11:56, Claes Redestad wrote: Hi, recently added test cases to MultiReleaseJarAPI can cause an AccessDeniedException on some Windows systems, and using a

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread David Holmes
On 28/09/2016 10:44 PM, Peter Levart wrote: Hi, According to discussion here: http://cs.oswego.edu/pipermail/concurrency-interest/2016-September/015414.html it seems compact strings introduced (at least theoretical) non-benign data race into String.hasCode() method. Here is a proposed

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Alan Bateman
On 28/09/2016 13:44, Peter Levart wrote: Hi, According to discussion here: http://cs.oswego.edu/pipermail/concurrency-interest/2016-September/015414.html it seems compact strings introduced (at least theoretical) non-benign data race into String.hasCode() method. Here is a proposed

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Peter Levart
On 09/28/2016 03:05 PM, David Holmes wrote: On 28/09/2016 10:44 PM, Peter Levart wrote: Hi, According to discussion here: http://cs.oswego.edu/pipermail/concurrency-interest/2016-September/015414.html it seems compact strings introduced (at least theoretical) non-benign data race into

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-28 Thread Kumar Srinivasan
Hello Thomas, Volker, Hi Kumar, On 9/16/2016 10:34 AM, Volker Simonis wrote: Hi Christoph, I think your change is fine as a quick-fix to fix the build. But you're completely right that this should be reworked in the long term. I hate to see that

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Claes Redestad
Hi, tangentially, is there any reason the Compact Strings work chose to revert to checking value.length > 0 rather than not writing to hash if the calculated hash code is 0? http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/6837759aa403 /Claes On 2016-09-28 14:47, Aleksey Shipilev wrote: On

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Vitaly Davidovich
Thanks Claes - this is an annoying one! Will it be backported to 8? On Wednesday, September 28, 2016, Claes Redestad wrote: > Thanks for the quick reviews! > > /Claes > > On 2016-09-28 14:14, Remi Forax wrote: > >> yes, >> thumb up. >> >> Rémi >> >> >> On September

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Krystal Mok
Hi Claes, For this particular case, this JDK-side change looks good to me. Let me post out the HotSpot version of the change and let you guys decide whether or not you guys want to take that version (which will take care of the ArrayList$1 case without the JDK-side change). Thanks, Kris On

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Claes Redestad
I dealt with this in isolation deliberately to ease backporting, but I don't have the 8u committer rights to do it myself. /Claes On 2016-09-28 16:27, Vitaly Davidovich wrote: Thanks Claes - this is an annoying one! Will it be backported to 8? On Wednesday, September 28, 2016, Claes

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Vladimir Ivanov
Looks good. Best regards, Vladimir Ivanov On 9/28/16 2:48 PM, Claes Redestad wrote: Hi, as discussed recently on hotspot-compiler-dev[1], having a private class with no default constructor can lead to C2 failing to inline, due to the synthetic bridge constructor using a dummy argument of an

Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-28 Thread Vitaly Davidovich
On Wednesday, September 28, 2016, David Holmes wrote: > On 28/09/2016 10:44 PM, Peter Levart wrote: > >> Hi, >> >> According to discussion here: >> >> http://cs.oswego.edu/pipermail/concurrency-interest/2016- >> September/015414.html >> >> >> it seems compact strings

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Vitaly Davidovich
On Wed, Sep 28, 2016 at 10:37 AM, Claes Redestad wrote: > I dealt with this in isolation deliberately to ease backporting, but I > don't have the 8u committer rights to do it myself. > Right. The change seems trivial enough that backporting would be easy. Who could

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

2016-09-28 Thread Claes Redestad
Hi Kris, right, I don't intend to meander away on a micro-optimization spree and chase down every case where we're hurt by this corner case in the JDK and elsewhere (I did check that other core collection classes aren't affected, though ;-)), but to get this simple and possibly high-impact