Re: RFR: JDK-8221641: Follow up code clean up for JDK-8221582

2019-04-09 Thread Alexander Matveev
Hi Andy, Looks good. Thanks, Alexander On 4/9/2019 9:33 AM, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). [1] https://bugs.openjdk.java.net/browse/JDK-8221641 [2] http://cr.

Re: RFR: JDK-8221749: Error messages

2019-04-09 Thread Alexander Matveev
Hi Andy, webrev.02 looks good. Thanks, Alexander On 4/9/2019 4:45 AM, Andy Herrick wrote: On 4/8/2019 6:05 PM, Andy Herrick wrote: On 4/8/2019 5:41 PM, Alexander Matveev wrote: Hi Andy, Looks good. Also, many messages (including added in this review) in MainResources.properties do have

Re: RFR: 8222029: Optimize Math.floorMod

2019-04-09 Thread Claes Redestad
In my cursory analysis of the test the interesting corner cases are covered and I have no reason to believe we'd see spurious errors elsewhere. I ran an adhoc semi-exhaustive test comparing results of the new version with the old and got an all-pass. /Claes On 2019-04-09 18:02, Joe Darcy wrote:

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Andrew Haley
On 4/9/19 11:42 AM, Andrew Dinn wrote: > This new API method was conceived as a preliminary change for JEP 352 to > allow selective writeback of NVRAM-backed buffers. However, it has been > implemented to provide a similar capability for file-mapped byte > buffers. The old brute-force API method, f

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Brian Burkhalter
> On Apr 9, 2019, at 10:59 AM, Alan Bateman wrote: > > There are a couple of implementation details to discuss I was wondering whether this topic has any synergy / overlap with this prior thread [1] the principal sticking point of which was IIRC forcing / loading a slice of a MBB. Brian ht

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Alan Bateman
On 09/04/2019 11:42, Andrew Dinn wrote: Could I please get reviews for the following patch which overloads MappedByteBuffer.force to accept a start offset and length. JIRA: https://bugs.openjdk.java.net/browse/JDK-8221696 webrev: http://cr.openjdk.java.net/~adinn/8221696/webrev.00 This new AP

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

2019-04-09 Thread Ivan Gerasimov
Hi Andrew! On 4/9/19 9:05 AM, Andrew Leonard wrote: Hi Ivan, Your micro benchmarks look good, that's something I need to learn how to execute. Here's how I ran them from the command line: make test TEST="micro:java.lang.StringBuilders.from" MICRO="FORK=3;WARMUP_ITER=4;ITER=6;WARMUP_TIME=4;

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

2019-04-09 Thread Ivan Gerasimov
Thanks Roger and Andrew! On 4/9/19 10:02 AM, Roger Riggs wrote: Hi Ivan, The JMH number look fine. thanks for comparing. And in this most recent webrev that has separated the AbstractStringBuilder constructors for String from CharSequence, is it more likely that the argument

Re: [8u] 8205432: Replace the placeholder Japanese era name

2019-04-09 Thread Andrew John Hughes
On 09/04/2019 18:18, Hohensee, Paul wrote: > I meant the current webrev > > https://cr.openjdk.java.net/~andrew/openjdk8/8205432/webrev.02/ > > is fine. Just backport what's in tip and fix whatever's wrong later as > another backport or backports. > > Paul Thanks. I've pushed based on the revi

Re: [8u] 8205432: Replace the placeholder Japanese era name

2019-04-09 Thread Hohensee, Paul
I meant the current webrev https://cr.openjdk.java.net/~andrew/openjdk8/8205432/webrev.02/ is fine. Just backport what's in tip and fix whatever's wrong later as another backport or backports. Paul On 4/9/19, 8:56 AM, "core-libs-dev on behalf of Hohensee, Paul" wrote: +1. Paul

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

2019-04-09 Thread Roger Riggs
Hi Ivan, The JMH number look fine.  thanks for comparing. And in this most recent webrev that has separated the AbstractStringBuilder constructors for String from CharSequence, is it more likely that the argument will be an AbstractStringBuilder than a String so that compari

Re: RFR: 8221397 Support implementation-defined Map Modes

2019-04-09 Thread Alan Bateman
On 09/04/2019 17:02, Andrew Dinn wrote: In response to Alan's suggestion (included below) I have reverted the constructor for MapMode to private and will use behind the scenes access to construct the extended enum values. This change removes the need to modify the test in MapTest.java (no new AP

RFR: JDK-8221641: Follow up code clean up for JDK-8221582

2019-04-09 Thread Andy Herrick
Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). [1] https://bugs.openjdk.java.net/browse/JDK-8221641 [2] http://cr.openjdk.java.net/~herrick/8221641/ /Andy

Re: Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

2019-04-09 Thread Andrew Leonard
Hi Ivan, Your micro benchmarks look good, that's something I need to learn how to execute. Do we think this is a good resolution now? Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_l

Re: RFR: 8222029: Optimize Math.floorMod

2019-04-09 Thread Joe Darcy
Basically I'm inquiring about whether the existing tests provide at least as good code coverage on the new implementation as the old one. As it is a relatively simple method, perhaps it there is full coverage before and after. However, at times changing the implementation requires updates to th

Re: RFR: 8221397 Support implementation-defined Map Modes

2019-04-09 Thread Andrew Dinn
In response to Alan's suggestion (included below) I have reverted the constructor for MapMode to private and will use behind the scenes access to construct the extended enum values. This change removes the need to modify the test in MapTest.java (no new API to exercise). It still requires javadoc

Re: RFR: 8222107 (CSR) Support implementation-defined Map Modes

2019-04-09 Thread Andrew Dinn
On 09/04/2019 10:09, Andrew Dinn wrote: > Could I please get a review for the following CSR. It relates to the > change proposed in JDK-8221397 which enables FileChannel > implementation-specific extensions to enum MapMode. > > https://bugs.openjdk.java.net/browse/JDK-8222107 I have updated this

Re: [8u] 8205432: Replace the placeholder Japanese era name

2019-04-09 Thread Hohensee, Paul
+1. Paul On 4/8/19, 8:28 PM, "core-libs-dev on behalf of Andrew John Hughes" wrote: On 08/04/2019 09:25, Deepak Kejriwal wrote: > Hi Andrew, > > Thanks for working on this. Please find below few minor comments:- > > 1>. For "src/share/classes/java/util/JapaneseImper

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Andrew Dinn
On 09/04/2019 16:04, Daniel Fuchs wrote: > . . . > That reads fine, thanks! Good. Thanks for reviewing this. > BTW: I haven't really looked at the implementation, I'm leaving that to > the experts of the field :-) No problem. I await their expert judgement. regards, Andrew Dinn --- Sen

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Daniel Fuchs
Hi Andrew, On 09/04/2019 13:37, Andrew Dinn wrote: I am not sure that this needs to be mentioned in an implNote. It is of the nature of most memory-mapped storage devices that writeback has a minimum granularity well above byte level. Would you be ok with a correspondingly general caveat? For e

Re: RFR (S): 8221979: Cleanups for building Windows resources

2019-04-09 Thread Erik Joelsson
Hello, Looks ok to me. I assume you have inspected all affected files and made sure all attributes are the same pre and post this change? /Erik On 2019-04-09 02:55, Langer, Christoph wrote: Hi, during work on JDK-8221880 I spotted some opportunity for cleanup in Windows resource files and

Re: RFR: 8221397 Support implementation-defined Map Modes

2019-04-09 Thread Alan Bateman
On 09/04/2019 13:55, Andrew Dinn wrote: : I'm happy to skulldig if you provide the advice on how to do it (an Idiot's Guide to Trepanning?). It definitely sounds better to expose this in a controlled way rather than via a general API. Can I assume this will remove the need for a CSR? The CSR is

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Andrew Dinn
On 09/04/2019 13:21, Claes Redestad wrote: > On 2019-04-09 11:05, Andrew Dinn wrote: >> It would probably also be good if you extended the comment to document >> the status quo i.e. as Peter noted that the assigned values are computed >> deterministically from immutable state. > > How about this:

Re: RFR: [8u] 8205432: Replace the placeholder Japanese era name

2019-04-09 Thread Andrew Haley
On 4/9/19 4:26 AM, Andrew John Hughes wrote: > I've fixed this too, but note that this comes from the original patch in > 11 and up. So it actually needs fixing there too. > > Revised webrev: > https://cr.openjdk.java.net/~andrew/openjdk8/8205432/webrev.02/ OK. -- Andrew Haley Java Platform Lea

Re: RFR: 8221397 Support implementation-defined Map Modes

2019-04-09 Thread Andrew Dinn
On 09/04/2019 13:47, Alan Bateman wrote: > On 08/04/2019 15:05, Andrew Dinn wrote: >> : >> Hmm, yes that's an interesting point. Of course, I'll take whatever >> suggestions you are willing to throw this way :-) >> > What would you think about not promoting the constructor to public? The > original

Re: RFR: 8221397 Support implementation-defined Map Modes

2019-04-09 Thread Alan Bateman
On 08/04/2019 15:05, Andrew Dinn wrote: : Hmm, yes that's an interesting point. Of course, I'll take whatever suggestions you are willing to throw this way :-) What would you think about not promoting the constructor to public? The original motivation for that was to be able to create additiona

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Andrew Dinn
Hi Daniel, Thanks for looking at this. On 09/04/2019 12:28, Daniel Fuchs wrote: > Hi Andrew, > > On 09/04/2019 11:42, Andrew Dinn wrote: >> One detail that is worth highlighting is that for file-backed buffers >> the start address passed to the native method force0 is rounded down to >> a page b

Re: RFR: 8222029: Optimize Math.floorMod

2019-04-09 Thread Claes Redestad
I think those tests cover all interesting corner cases, so the only way I see it can be improved is to make it more exhaustive (say generate a large random sample of tests every run). Do you feel that is needed? /Claes On 2019-04-09 01:35, Joseph D. Darcy wrote: Should any additional cases be ad

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Claes Redestad
On 2019-04-09 11:05, Andrew Dinn wrote: It would probably also be good if you extended the comment to document the status quo i.e. as Peter noted that the assigned values are computed deterministically from immutable state. How about this: http://cr.openjdk.java.net/~redestad/8221836/open.03

Re: RFR: JDK-8221749: Error messages

2019-04-09 Thread Andy Herrick
On 4/8/2019 6:05 PM, Andy Herrick wrote: On 4/8/2019 5:41 PM, Alexander Matveev wrote: Hi Andy, Looks good. Also, many messages (including added in this review) in MainResources.properties do have "." at the end and some don't. I think we should cleanup it and add "." if needed. Yes - som

Re: RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Daniel Fuchs
Hi Andrew, On 09/04/2019 11:42, Andrew Dinn wrote: One detail that is worth highlighting is that for file-backed buffers the start address passed to the native method force0 is rounded down to a page boundary. This is needed for Unix implementations to ensure that the underlying msync system cal

RFR : 8221696: MappedByteBuffer.force method to specify range

2019-04-09 Thread Andrew Dinn
Could I please get reviews for the following patch which overloads MappedByteBuffer.force to accept a start offset and length. JIRA: https://bugs.openjdk.java.net/browse/JDK-8221696 webrev: http://cr.openjdk.java.net/~adinn/8221696/webrev.00 This new API method was conceived as a preliminary ch

RFR (S): 8221979: Cleanups for building Windows resources

2019-04-09 Thread Langer, Christoph
Hi, during work on JDK-8221880 I spotted some opportunity for cleanup in Windows resource files and their handling in the build. The naming of variables used for customizing resource properties in the build system should be aligned between hotspot and JDK. This should be carefully reviewed by

Re: [PING?] RFR: 8217338: [Containers] Improve systemd slice memory limit support

2019-04-09 Thread Severin Gehwolf
Hi, Could I get another reviewer for this, please? Bob Vandette already reviewed it. Thank you! Cheers, Severin On Tue, 2019-04-02 at 13:48 +0200, Severin Gehwolf wrote: > Could I get a second review, please? > > Thanks, > Severin > > On Thu, 2019-03-28 at 11:37 -0400, Bob Vandette wrote: > >

Re: RFR: 8222144: Lazily create JRT code source URLs for LoadedModules

2019-04-09 Thread Claes Redestad
Hi Alan, On 2019-04-09 08:42, Alan Bateman wrote: On 08/04/2019 22:42, Claes Redestad wrote: Hi, currently we eagerly convert code source URIs to URLs to avoid recursive bootstrap issues with overrideable URL handlers. The JRT scheme handler is not overrideable, however, and also commonly used

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Aleksey Shipilev
On 4/9/19 10:53 AM, Peter Levart wrote: > On 4/9/19 10:11 AM, Aleksey Shipilev wrote: >>> 2. No risk of hashcode recomputation for the 2^-32 case. >>> This might seem laughable, until you remember that it's exactly >>> those cases that DOS attackers like to create. >> Alt-hashing covers this obscur

Re: RFR: 8222107 (CSR) Support implementation-defined Map Modes

2019-04-09 Thread Andrew Dinn
Could I please get a review for the following CSR. It relates to the change proposed in JDK-8221397 which enables FileChannel implementation-specific extensions to enum MapMode. https://bugs.openjdk.java.net/browse/JDK-8222107 Thanks. regards, Andrew Dinn --- Senior Principal Softwar

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Andrew Dinn
On 09/04/2019 09:42, Claes Redestad wrote: > Hi Andrew, > > On 2019-04-09 10:20, Andrew Dinn wrote: >> If the patch is to go in -- and I concede there is an acceptable >> argument for that -- then it >> needs commenting to help avoid this happening. > > open.02 already adds what I believed to

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Peter Levart
On 4/9/19 10:42 AM, Claes Redestad wrote: Hi Andrew, On 2019-04-09 10:20, Andrew Dinn wrote: If the patch is to go in -- and I concede there is an acceptable argument for that -- then it needs commenting to help avoid this happening. open.02 already adds what I believed to be sufficient c

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Peter Levart
Hi Aleksey, On 4/9/19 10:11 AM, Aleksey Shipilev wrote: 2. No risk of hashcode recomputation for the 2^-32 case. This might seem laughable, until you remember that it's exactly those cases that DOS attackers like to create. Alt-hashing covers this obscure case in the course of mitigating much

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Claes Redestad
Hi Andrew, On 2019-04-09 10:20, Andrew Dinn wrote: If the patch is to go in -- and I concede there is an acceptable argument for that -- then it needs commenting to help avoid this happening. open.02 already adds what I believed to be sufficient commenting, have you taken this into considerat

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Andrew Dinn
On 09/04/2019 09:11, Aleksey Shipilev wrote: > On 4/8/19 11:31 PM, John Rose wrote: >> I agree that this is a good change, and you can use me as a reviewer. > > Which opens up the process question: are you acting as Project Lead here to > resolve the disagreement > between Reviewers (the only

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Andrew Dinn
On 08/04/2019 22:19, Claes Redestad wrote: > First, I disagree strongly that this patch adds significant complexity > (especially after recent simplifications[1]) or that it risks increasing > maintenance headache down the line. It all depends what you mean by significant. It definitely adds compl

Re: RFR: 8221836: Avoid recalculating String.hash when zero

2019-04-09 Thread Aleksey Shipilev
On 4/8/19 11:31 PM, John Rose wrote: > I agree that this is a good change, and you can use me as a reviewer. Which opens up the process question: are you acting as Project Lead here to resolve the disagreement between Reviewers (the only accepting Reviewer being yourself)? (There are ways for me