Re: PriorityQueue

On Wed, May 13, 2015 at 11:17 PM, Brett Bernstein wrote: > I believe the linked sequence of messages refer to the addition of a > PriorityQueue constructor only taking a Comparator which was does appear in > Java 1.8. Did you have a link to something regarding the a constructor > taking a Collec

Re: PriorityQueue

Software is hard. We tried and got stuck, although the former effort can be revived. RFR : 6799426 : (xs) Add constructor PriorityQueue(Comparator) http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/019124.html On Wed, May 13, 2015 at 10:17 PM, Brett Bernstein wrote: > To whom this

Re: RFR(s): 8078463: optimize java/util/Map/Collisions.java

Your changes look good, but: 204 check(map.size() == i, "insertion: map expected size m%d != i%d", map.size(), i); many of those detail messages look like leftovers from a long debugging session. Here I would consider converting to a testng test (I ended up doing this a few times my

RFR(s): 8078463: optimize java/util/Map/Collisions.java

Hi all, Please review this change to optimize a test. Basically the test did string formatting for every assertion, but the string was thrown away if the assertion passed -- the most common case. The change is to do the string formatting only when an assertion fails and information needs to be

Re: AbstractList etc. functionality as interfaces with default methods?

Not only is there a problem with modCount, but also with equals/hashCode/toString. You can’t define these Object methods in an interface. On May 8, 2015, at 5:41 AM, Attila Szegedi wrote: > So I’m in a position where I’d need to have a class implement List, but it > already extends something

Re: AbstractList etc. functionality as interfaces with default methods?

I'm also in the business of maintaining List implementations and would like to share implementation code. Unfortunately, Java does not provide much help. Java confuses inheritance and subtyping. Public abstract superclasses leak through (users might mistakenly use AbstractList in their API) and o

Re: RFR: 8071571: Move substring of same string to slow path

On May 13, 2015, at 4:46 PM, Vitaly Davidovich wrote: > I'd add "And always look at generated asm for these types of constructs". +1, with caveats about micro-versions changing stuff at that level

Re: RFR: 8071571: Move substring of same string to slow path

I'd add "And always look at generated asm for these types of constructs". There's really nothing to profile in terms of "|" outcome unless JIT starts tracking bit position values, and it's a cheap instruction on its own. I'd be surprised if conditional moves are emitted here since these branches

Re: RFR: 8071571: Move substring of same string to slow path

On May 13, 2015, at 2:25 PM, Ivan Gerasimov wrote: > > My third concern is this: Wouldn't it be possible to implement this type of > optimization at jvm level? > At least some conditions can automatically be combined into one. > Given the information about which execution path is expected to be

Re: RFR: 8071571: Move substring of same string to slow path

Need JIT generated assembly, not bytecode :). That will tell you at least which optimizations JIT applied, how it register allocated things, etc. If nothing obvious there, see my other reply regarding cpu event based profiling. I'm sure Aleksey Shipilev could help out if you're really inclined t

Re: RFR: 8071571: Move substring of same string to slow path

By the way, this is pure speculation on my part by just looking at the code. To truly find out, at least say for Intel, you'd have to run these benchmarks under a cpu event profiler and see what it tells you for IPC, branch mispredictions, the various stalls, etc. JMH has perfasm I believe which

Re: RFR: 8071571: Move substring of same string to slow path

On 14.05.2015 2:06, Vitaly Davidovich wrote: Why not look at the generated asm and not guess? :) The branch avoiding versions may cause data dependence hazards whereas the branchy one just has branches but assuming perfectly predicted (and microbenchmarks typically are) can pipeline through

Re: RFR: 8071571: Move substring of same string to slow path

Yes, that should be the general rule of thumb for code targeting out of order chips. The caveat is that microbenchmarks have the advantage of being the only (typically very small) code running on the cpu, and will get full use of execution resources; specifically in this case, it's very likely tha

Re: RFR: 8071571: Move substring of same string to slow path

On Wed, May 13, 2015 at 4:06 PM, Vitaly Davidovich wrote: > :) The branch avoiding versions may cause data dependence hazards whereas > the branchy one just has branches but assuming perfectly predicted (and > microbenchmarks typically are) can pipeline through. Ivan, could you > please post the

Re: Updating existing JDK code to use InputStream.transferTo()

Hi Pavel, On 05/14/2015 12:14 AM, Pavel Rappo wrote: The other reason to have read that returns 0 is if the underlying channel is in non-blocking mode. A read on an InputStream created by Channels.newInputStream on a SelectableChannel may return 0 and the code will go in a loop until the Selec

Re: Updating existing JDK code to use InputStream.transferTo()

Hi Chris, On 05/14/2015 12:40 AM, Chris Hegarty wrote: On 13 May 2015, at 22:45, Remi Forax > wrote: may return 0, is when len == 0. And it's never the case here. Unless, of course, some misbehaving implementation of InputStream is used. The other reason to hav

Re: RFR: 8071571: Move substring of same string to slow path

Why not look at the generated asm and not guess? :) The branch avoiding versions may cause data dependence hazards whereas the branchy one just has branches but assuming perfectly predicted (and microbenchmarks typically are) can pipeline through. Ivan, could you please post the asm here? Assuming

Re: RFR: 8071571: Move substring of same string to slow path

On Wed, May 13, 2015 at 2:25 PM, Ivan Gerasimov wrote: > > Benchmark Mode Cnt Score Error Units > MyBenchmark.testMethod_1 thrpt 60 1132911599.680 ± 42375177.640 ops/s > MyBenchmark.testMethod_2 thrpt 60 813737659.576 ± 14226427.823 ops/s > MyBenchmar

Re: Updating existing JDK code to use InputStream.transferTo()

> On 13 May 2015, at 22:45, Remi Forax wrote: > >> >> may return 0, is when len == 0. And it's never the case here. Unless, of >> course, >> some misbehaving implementation of InputStream is used. > > The other reason to have read that returns 0 is if the underlying channel is > in non-block

Re: Updating existing JDK code to use InputStream.transferTo()

> So then our transferTo() implementation should be like this? > > while ((read = this.read(buffer, 0, TRANSFER_BUFFER_SIZE)) > -1) { Isn't it the same as we have now? :) If you mean "> 0", then we should probably wait for someone with both IO and NIO expertise to have a look at this.

Re: Updating existing JDK code to use InputStream.transferTo()

Hi Pavel, So then our transferTo() implementation should be like this? while ((read = this.read(buffer, 0, TRANSFER_BUFFER_SIZE)) > -1) { That would then look like the code I used to write… -Patrick > Am 14.05.2015 um 00:14 schrieb Pavel Rappo : > > The other reason to have read that

Re: Updating existing JDK code to use InputStream.transferTo()

>>> The other reason to have read that returns 0 is if the underlying channel >>> is in non-blocking mode. >>> A read on an InputStream created by Channels.newInputStream on a >>> SelectableChannel may return 0 >>> and the code will go in a loop until the SelectableChannel can read >>> somethin

Re: Updating existing JDK code to use InputStream.transferTo()

On 05/13/2015 11:58 PM, Pavel Rappo wrote: Remi, The other reason to have read that returns 0 is if the underlying channel is in non-blocking mode. A read on an InputStream created by Channels.newInputStream on a SelectableChannel may return 0 and the code will go in a loop until the Select

Re: Updating existing JDK code to use InputStream.transferTo()

Remi, > The other reason to have read that returns 0 is if the underlying channel is > in non-blocking mode. > A read on an InputStream created by Channels.newInputStream on a > SelectableChannel may return 0 > and the code will go in a loop until the SelectableChannel can read something. > whil

Re: RFR(M,v3): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

Hi Dmitry, Some review comments below... On 5/12/15 1:10 PM, Dmitry Samersoff wrote: Everybody, Updated version: http://cr.openjdk.java.net/~dsamersoff/JDK-8059036/webrev.03/ Now it iterates over queue and output result sorted by number of instances. FinalReference.java - Update copyright

Re: Updating existing JDK code to use InputStream.transferTo()

On 05/13/2015 05:29 PM, Pavel Rappo wrote: It now can be reviewed as usual at: http://cr.openjdk.java.net/~prappo/8080272/webrev.00/ Feel free to review. Thanks. Let me start then. 1. I've seen several cases where current behaviour while ((n = in.read(buffer)) > 0)

Re: RFR: 8071571: Move substring of same string to slow path

Hi Martin! On 13.05.2015 0:22, Martin Buchholz wrote: All your changes look good. But: --- Not your bug, but it looks like the below should instead be: throw new StringIndexOutOfBoundsException(beginIndex); Perhaps fix in a follow-on change. 1935 if (subLen < 0) { 1936 thr

Re: RFR(M,v3): JDK-8059036 : Implement Diagnostic Commands for heap and finalizerinfo

Hi Peter, I don't have smoking gun evidence that your change introduces a bug, but I have some concerns... On 5/12/15 6:05 PM, Peter Levart wrote: Hi Dmitry, You iterate the queue then, not the unfinalized list. That's more logical. Holding the queue's lock may pause reference handler and f

Re: RFR: 8079459: JCK test api/java_nio/ByteBuffer/index.html#GetPutXXX start failing after JDK-8026049

On 13/05/2015 17:14, Andrew Haley wrote: : Test coverage is here extended to include boundary conditions for all primitive types: Fix: http://cr.openjdk.java.net/~aph/8079459-2-jdk/ Testcase: http://cr.openjdk.java.net/~aph/8079459-2-hs/ Thanks for expanding the test coverage, just awkward

Re: Changes for JDK-8075327: moving jdk testlibraty files duplicated in hotspot to the common test repository

Chris, Following the feedback from Jon Gibbons, we have changed our current plan to the following: 1) We are waiting till the new jtreg is promoted. The new jtreg allows for specifying in the repository's TEST.properties file one or more paths to search for libraries 2) After the jtreg is ou

Re: Changes fro JDK-8075327: moving jdk testlibraty files duplicated in hotspot to the common test repository

Alexander, > On 13 May 2015, at 16:46, Alexander Kulyakhtin > wrote: > > Hi Chris, > > >> I suspect that these changes are best going directly into jdk9/dev, as >> opposed to a a downstream forest. > Yes, they are going directly to jdk9/dev, I forgot to add the group, adding > now. > >> In

Re: Updating existing JDK code to use InputStream.transferTo() (jdk)

If you have time it might be useful to have a look for places to retrofit with java.nio.file.Files This class provides lots of useful tools which cover many usecases. Especially take a look at `copy` methods. Thanks! -Pavel > On 12 May 2015, at 23:10, Patrick Reinhart wrote: > > Hi Pavel, > >

Re: RFR: 8079459: JCK test api/java_nio/ByteBuffer/index.html#GetPutXXX start failing after JDK-8026049

On 05/11/2015 05:25 PM, Andrew Haley wrote: > On 05/11/2015 04:39 PM, Alan Bateman wrote: >> On 11/05/2015 15:14, Andrew Haley wrote: >>> I mixed up my nextPutIndex and nextGetIndex. Sorry. >>> >>> http://cr.openjdk.java.net/~aph/8079459/ >>> >>> This is also the cause of Bug >>> https://bugs.ope

Re: Changes fro JDK-8075327: moving jdk testlibraty files duplicated in hotspot to the common test repository

Hi Chris, > I suspect that these changes are best going directly into jdk9/dev, as > opposed to a a downstream forest. Yes, they are going directly to jdk9/dev, I forgot to add the group, adding now. > In may places '@library /lib/testlibrary ...' remains. Is this > redundant, in many tests? I

Changes fro JDK-8075327: moving jdk testlibraty files duplicated in hotspot to the common test repository

Hi, Could you please, review the following tests-only changes to the hs-rt/jdk and hs-rt/test repositories. These changes are a part of the changes for "JDK-8075327: Merge jdk and hotspot test libraries" The changes are as follows: http://cr.openjdk.java.net/~akulyakh/8075327/jdk_patch/webrev/

Re: Changes fro JDK-8075327: moving jdk testlibraty files duplicated in hotspot to the common test repository

Adding jdk9-dev mailing list - Original Message - From: alexander.kulyakh...@oracle.com To: core-libs-dev@openjdk.java.net, awt-...@openjdk.java.net, hotspot-...@openjdk.java.net Cc: stefan.sa...@oracle.com, yekaterina.kantser...@oracle.com, alan.bate...@oracle.com Sent: Wednesday, May 1

Re: RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java

> On May 12, 2015, at 2:26 PM, Peter Levart wrote: > > > > On 05/12/2015 10:49 PM, Mandy Chung wrote: >>> But I think it should be pretty safe to make the java.util.Properties >>> object override all Hashtable methods and delegate to internal CMH so that: >>> - all modification methods + all

Re: Updating existing JDK code to use InputStream.transferTo()

> It now can be reviewed as usual at: > > http://cr.openjdk.java.net/~prappo/8080272/webrev.00/ > > Feel free to review. Thanks. Let me start then. 1. I've seen several cases where current behaviour while ((n = in.read(buffer)) > 0) ~~~ has been change

Re: Changes fro JDK-8075327: moving jdk testlibraty files duplicated in hotspot to the common test repository

Hi Alexander, On 13/05/15 15:52, Alexander Kulyakhtin wrote: Hi, Could you please, review the following tests-only changes to the hs-rt/jdk and hs-rt/test repositories. These changes are a part of the changes for "JDK-8075327: Merge jdk and hotspot test libraries" I suspect that these chang

Re: RFR 9: 8077350 Process API Updates Implementation Review

Hi Alan, Yes, the destroy use looks a bit odd. In Process, destroyForcibly returns this so that the application can fluently wait for the termination to complete. Returning Optional enables the case to fluently perform an action on the process when it does exit. ProcessHandle ph = ...;

Re: [9] RFR (M): 8079205: CallSite dependency tracking is broken after sun.misc.Cleaner became automatically cleared

On May 13, 2015, at 1:59 PM, Vladimir Ivanov wrote: > Peter, Paul, thanks for the feedback! > > Updated the webrev in place: > http://cr.openjdk.java.net/~vlivanov/8079205/webrev.02 > +1 >> I am not an export in the HS area but the code mostly made sense to me. I >> also like Peter's sug

Re: RFR 9: 8077350 Process API Updates Implementation Review

On 13/05/2015 15:16, Roger Riggs wrote: Hi, Are there any comments about the use of java.util.Optional in the ProcessHandle API? Or a review of the changes? Having parent return Optional looks good. Having all methods in ProcessHandle.Info return Optional is probably right, it gives us a bi

Re: RFR 9: 8077350 Process API Updates Implementation Review

Hi, Are there any comments about the use of java.util.Optional in the ProcessHandle API? Or a review of the changes? Thanks, Roger On 5/11/2015 11:49 AM, Roger Riggs wrote: Please review clarifications and updates to the proposed Precess API. A few loose ends in the ProcessHandle API were

Re: [9] RFR (M): 8079205: CallSite dependency tracking is broken after sun.misc.Cleaner became automatically cleared

> http://cr.openjdk.java.net/~vlivanov/8079205/webrev.02/ The hotspot code looks good to me. Roland.

Re: RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33

Thanks Claes. I think there's some room for improvement in the JIT to piggyback on the user check, but that's a side topic. Out of curiosity, have you tried moving the slow path (computation of the hash) into an out-of-line method? What's the bytecode size if you do that? This shouldn't matter fo

Re: RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33

I shouldn't have said better. :-) Running against versions of hashCode using value.length > 0 and h != 0 respectively show results that are within the error range of each other. It's hard to quantify the cost of one extra branch in the slow path, I guess. Checking value.length will only avoid

Re: RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33

It's interesting that this version performs as good or *better* than value.length > 0 check. Intuitively, value.length is going to be used anyway (if h == 0) in the loop and so there should be no penalty of loading and branching on it (given the change here has a branch anyway) and compiler can ke

Re: RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33

Just curious, what is the difference between this fix and an old version of the code: http://cr.openjdk.java.net/~shade/8058643/webrev.01/src/java.base/share/classes/java/lang/String.java.sdiff.html I meant: "if (h == 0 && value.length > 0) {}" vs "if (h != 0) {hash = h;}" On 13.05.15 14:51, Cl

Re: RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33

On 13.05.2015 14:51, Claes Redestad wrote: > 9-b33 introduced a sustained regression in SPECjvm2008 > xml.transform on a number of our test setups. > > JDK-8058643 removed the check on value.length > 0, which > means repeated calls to "".hashCode() now do a store of the > calculated value (0) to t

Re: [9] RFR (M): 8079205: CallSite dependency tracking is broken after sun.misc.Cleaner became automatically cleared

Peter, Paul, thanks for the feedback! Updated the webrev in place: http://cr.openjdk.java.net/~vlivanov/8079205/webrev.02 I am not an export in the HS area but the code mostly made sense to me. I also like Peter's suggestion of Context implementing Runnable. I agree. Integrated. Some mino

RFR: 8061254: SPECjvm2008-XML performance regressions in 9-b33

Hi, 9-b33 introduced a sustained regression in SPECjvm2008 xml.transform on a number of our test setups. JDK-8058643 removed the check on value.length > 0, which means repeated calls to "".hashCode() now do a store of the calculated value (0) to the hash field. This has potential to cause excess

Re: Updating existing JDK code to use InputStream.transferTo() (jdk)

On 13/05/15 12:24, patr...@reini.net wrote: Hi Chris, That would simplify the process greatly. What I exactly need to do for getting the JDK 9 project author role? See the section named "Becoming an Author" on the Projects page [1]. -Chris. [1] http://openjdk.java.net/projects/ Cheers, P

Re: Updating existing JDK code to use InputStream.transferTo() (jdk)

Hi Chris, That would simplify the process greatly. What I exactly need to do for getting the JDK 9 project author role? Cheers, Patrick On 2015-05-13 12:12, Chris Hegarty wrote: Given your previous contributions [1] and ongoing involvement in OpenJDK, it would be reasonable for you to requ

Re: [9] RFR (M): 8079205: CallSite dependency tracking is broken after sun.misc.Cleaner became automatically cleared

Hi Vladimir, I am not an export in the HS area but the code mostly made sense to me. I also like Peter's suggestion of Context implementing Runnable. Some minor comments. CallSite.java: 145 private final long dependencies = 0; // Used by JVM to store JVM_nmethodBucket* It's a little

Re: Updating existing JDK code to use InputStream.transferTo()

It now can be reviewed as usual at: http://cr.openjdk.java.net/~prappo/8080272/webrev.00/ Feel free to review. Thanks. -Pavel > On 10 May 2015, at 12:45, Martijn Verburg wrote: > > Hi Patrick, > > Have you posted the webrev somewhere for review? > > Cheers, > Martijn > > On 8 May

Re: [9] RFR (M): 8079205: CallSite dependency tracking is broken after sun.misc.Cleaner became automatically cleared

Hi Vladimir, This is nice. I don't know how space-savvy CallSite should be, but you can save one additional object - the Cleaner's thunk: static class Context implements Runnable { private final long dependencies = 0; // Used by JVM to store JVM_nmethodBucket* static Con

Re: Updating existing JDK code to use InputStream.transferTo() (jdk)

On 12/05/15 23:10, Patrick Reinhart wrote: ... Unfortunately I do not have no account on cr.openjdk.java.net , so I need to paste the patch to the mailing list directly. Given your previous contributions [1] and ongoing involvement in OpenJDK, it would be reasona

Re: [9] RFR (M): 8079205: CallSite dependency tracking is broken after sun.misc.Cleaner became automatically cleared

I finished experimenting with the idea inspired by private discussion with Kim Barrett: http://cr.openjdk.java.net/~vlivanov/8079205/webrev.02/ The idea is to use CallSite instance as a context for dependency tracking, instead of the Class CallSite is bound to. It requires extension of nmet

Re: RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java

On May 12, 2015, at 10:49 PM, Mandy Chung wrote: >> >> Ah, I understand Mandy now. You are talking about using special Properties >> implementation just for system properties. Unfortunately, this is currently >> valid code: >> >> Properties props = new Properties(); >> ... >> System.setPropert