RFR 8202086 : Improve performance characteristics of sun.security.util.MemoryCache

Hello! This enhancement was generously contributed by Peter Levart [1]. The original webrev is found here [2]. The goal was to improve concurrent accessibility of the cache, while maintaining some additional limitations dictated by the spec. Would you please help review this fix? BUGURL: h

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

I see. Generalization vs solution in a specific scope, it's kind of a balancing art indeed :-) -Joe On 4/30/2018 5:13 PM, Paul Sandoz wrote: On Apr 30, 2018, at 4:47 PM, Joe Wang wrote: — It’s tempting (well to me at least) to generalize to a mismatch method (like for arrays) returning t

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On Apr 30, 2018, at 4:47 PM, Joe Wang wrote: > > Are there real-life use cases? It may be useful for example to check if the > files have the same header. After equality comparison, lexical comparison is a key use case. By allowing the user to interpret the data around the mismatch, the compar

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

> On Apr 30, 2018, at 4:47 PM, Joe Wang wrote: >> >> — >> >> It’s tempting (well to me at least) to generalize to a mismatch method (like >> for arrays) returning the mismatching location in bytes, then you can >> determine if one file is a prefix of another given the files sizes. Bound >>

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Good point. So even if we go with "has" (instead of "is"), it'd be hasSameContent, semantically, it would be "has the same content". -Joe On 4/30/2018 4:10 PM, Jonathan Gibbons wrote: At the risk of triggering a #bikeshed on the relative merits of "content" vs. "contents", I note that String h

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On 4/30/2018 11:47 AM, Paul Sandoz wrote: On Apr 27, 2018, at 4:30 AM, Alan Bateman wrote: On 27/04/2018 05:51, Joe Wang wrote: Hi, Considering extending isSameFile to add isSameContent to Files. Please review. JBS: https://bugs.openjdk.java.net/browse/JDK-8202285 webrev: http://cr.openj

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

First, this is intended to be an extension to the existing isSameFile method since it stopped short of comparing the content to answer the query for whether two files are equal. We did a review/a bit of research on user demand. Comparing files isn't as high as for example readString, but there

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

At the risk of triggering a #bikeshed on the relative merits of "content" vs. "contents", I note that String has put a stake in the ground for the singular form, with contentEquals. https://docs.oracle.com/javase/9/docs/api/java/lang/String.html#contentEquals-java.lang.CharSequence- -- Jon O

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

On 4/27/2018 4:30 AM, Alan Bateman wrote: On 27/04/2018 05:51, Joe Wang wrote: Hi, Considering extending isSameFile to add isSameContent to Files. Please review. JBS: https://bugs.openjdk.java.net/browse/JDK-8202285 webrev: http://cr.openjdk.java.net/~joehw/jdk11/8202285/webrev/ specdiff

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Hi Jonathan, hasSameContents does read better in English. This one was made isSameContent since I thought we'd want to stack it next to the existing isSameFile method since it's meant to be an extend to that method. I'd love to hear what people think about this. I'm open to change the name if

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Hi Daniel, Thanks for reviewing the proposal! For the spec, or javadoc in general, the first sentence shall be a short summary of the method, a definition of what the method is. It appears in the method summary table and index. So in this case, this method "Tests if the content of two files i

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Hi Bernd, Thanks for the review. Please refer to the next email in this thread, I've changed it to read a chunk at a time instead. Best, Joe On 4/27/2018 2:32 AM, Bernd Eckenfels wrote: If this really stays this way and reads all bytes into memory it should at least state so, as this can ea

Hashing files/bytes Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

Thanks, better then i expected with the transferTo method we recently added, but i think we could do even better for the ease of use case of “give me the hash of this file contents or these bytes or this byte buffer". Paul. > On Apr 30, 2018, at 3:23 PM, Remi Forax wrote: > >> >> To Remi’s p

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

- Mail original - > De: "Paul Sandoz" > À: "Alan Bateman" > Cc: "nio-dev" , "core-libs-dev" > > Envoyé: Lundi 30 Avril 2018 20:47:06 > Objet: Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing > file contents >> On Apr 27, 2018, at 4:30 AM, Alan Bateman wrote: >> >

Re: RRF: 8187123: (reflect) Class#getCanonicalName and Class#getSimpleName is a part of performance issue

Hi Claes, On 30/04/2018 10:49 PM, Claes Redestad wrote: Hi, please review this patch to enable caching of getCanonicalName and getSimpleName, repeated calls of which has been reported to be a performance bottleneck. The caching improves performance of these methods by up to 20x. Rather than

Re: RRF: 8187123: (reflect) Class#getCanonicalName and Class#getSimpleName is a part of performance issue

Looks good. — I am not quite 100% sure but you could probably replace the null sentinel value with “/“ or opportunistically “[]”, but i cannot quite tell what exactly is an invalid binary name. Anyway that is not important. Paul. > On Apr 30, 2018, at 5:49 AM, Claes Redestad wrote: > > Hi,

Re: RFR: Here are some easy patches

> On Apr 30, 2018, at 11:18 AM, Martin Buchholz wrote: > > > > On Mon, Apr 30, 2018 at 10:35 AM, Paul Sandoz > wrote: >> An obvious optimization: >> >> 8202398: Optimize Arrays.deepHashCode >> http://cr.openjdk.java.net/~martin/webrevs/jdk/deepHashCode-optimiz

Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

> On Apr 27, 2018, at 4:30 AM, Alan Bateman wrote: > > On 27/04/2018 05:51, Joe Wang wrote: >> Hi, >> >> Considering extending isSameFile to add isSameContent to Files. Please >> review. >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8202285 >> >> webrev: http://cr.openjdk.java.net/~jo

Re: RFR: Here are some easy patches

On Mon, Apr 30, 2018 at 10:35 AM, Paul Sandoz wrote: > An obvious optimization: > > 8202398: Optimize Arrays.deepHashCode > http://cr.openjdk.java.net/~martin/webrevs/jdk/deepHashCode-optimize/ > https://bugs.openjdk.java.net/browse/JDK-8202398 > > I would prefer that the deeply nested ternary ex

Re: RFR: Here are some easy patches

> On Apr 30, 2018, at 9:05 AM, Martin Buchholz wrote: > > Off by one pixel! > > 8202397: Typo in X-Buffer javadoc > http://cr.openjdk.java.net/~martin/webrevs/jdk/X-Buffer-typo/ > > https://bugs.openjdk.java.net/browse/JDK-820239

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

On 30/04/2018 17:29, mandy chung wrote: : The 3 x getRoot methods on ReflectAccess looks okay. An alternative would to create T getRoot(T obj) and a package private getRoot() method on AccessibleObject. Good idea.  I updated the patch. The updated webrev looks good. A minor comment is

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

Hi Hamlin, Thanks for reviewing the test! Fixed now, by calling deleteOnExit. Indeed, I didn't realize I got a few empty tmp files :-) Best, Joe On 4/28/2018 12:35 AM, Hamlin Li wrote: Hi Joe, just a minor comment about the test in ReadWriteString.java. For both testMalformedWrite and test

Re: RFR: 8202419: Avoid creating Permission constants early

On 4/30/18 8:25 PM, Claes Redestad wrote: Hi, moving a couple of local permission constants to sun.security.util.SecurityConstants ensures we delay and avoid loading permission classes very early during bootstrap. Delaying load is profitable since it's happening early enough (before System

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

On 4/30/18 7:39 PM, Alan Bateman wrote: The approach looks good, seems like this one was lurking (for protected members at least) for a long time. Yes and this issue becomes more noticeable in JDK 9 as public members needs additional module access check. The 3 x getRoot methods on Refl

RFR: Here are some easy patches

Off by one pixel! 8202397: Typo in X-Buffer javadoc http://cr.openjdk.java.net/~martin/webrevs/jdk/X-Buffer-typo/ https://bugs.openjdk.java.net/browse/JDK-8202397 Fixes a long-term micro-embarrassment: 8201634: Random seedUniquifier uses incorrect LCG http://cr.openjdk.java.net/~martin/webrevs/j

Re: RFR: JDK-8193877- DateTimeFormatterBuilder throws ClassCastException when using padding

On 30 April 2018 at 09:20, Pallavi Sonal wrote: > Should the PadPrinterDecoratorParsers be allowed to participate in adjacent > value parsing and should this affect both padded followed by non-padded > fields as well as non-padded followed by padded fields ? Since JSR-310 was first included, th

Re: RFR: 8202419: Avoid creating Permission constants early

On 2018-04-30 15:05, Sean Mullan wrote: Looks fine to me. Thanks! I think the TO DO comment on line 131 of ReflectionFactory can be removed - it looks like a leftover comment which doesn't seem relevant anymore. Sure /Claes

Re: RFR: 8202419: Avoid creating Permission constants early

Looks fine to me. I think the TO DO comment on line 131 of ReflectionFactory can be removed - it looks like a leftover comment which doesn't seem relevant anymore. --Sean On 4/30/18 8:25 AM, Claes Redestad wrote: Hi, moving a couple of local permission constants to sun.security.util.Securit

Re: RFR: 8202419: Avoid creating Permission constants early

On 2018-04-30 14:47, Alan Bateman wrote: This looks okay to me. Thanks! Just a few nits - the comment from AccessibleObject has moved to SecurityConstants. I think it would be better to leave the comment in AccessibleObject, then you can just comment the SecurityConstants update with the

RRF: 8187123: (reflect) Class#getCanonicalName and Class#getSimpleName is a part of performance issue

Hi, please review this patch to enable caching of getCanonicalName and getSimpleName, repeated calls of which has been reported to be a performance bottleneck. The caching improves performance of these methods by up to 20x. Rather than adding new fields to Class itself, which would have foot

Re: RFR: 8202419: Avoid creating Permission constants early

On 30/04/2018 13:25, Claes Redestad wrote: Hi, moving a couple of local permission constants to sun.security.util.SecurityConstants ensures we delay and avoid loading permission classes very early during bootstrap. Delaying load is profitable since it's happening early enough (before System.

RFR: 8202419: Avoid creating Permission constants early

Hi, moving a couple of local permission constants to sun.security.util.SecurityConstants ensures we delay and avoid loading permission classes very early during bootstrap. Delaying load is profitable since it's happening early enough (before System.initPhase1) to contribute to delaying the in

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

On 28/04/2018 10:44, mandy chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access to a member has been verified for a given caller.  At the

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

Hi All, Please review the fix. bug: https://bugs.openjdk.java.net/browse/JDK-8202329 webrev: http://cr.openjdk.java.net/~aleonard/8202329/webrev.00/ Thanks, Bhaktavatsal Reddy -"core-libs-dev" wrote: - To: Volker Simonis From: "Bhaktavatsal R Maram" Sent by: "core-libs-dev" Date:

Re: RFR: JDK-8193877- DateTimeFormatterBuilder throws ClassCastException when using padding

Hi Roger/Stephen, > The fix here seems to make padding to a fixed width incompatible with > adjacent value parsing. > That's not intuitive, since adjacent value parsing is intended to > allow more flexible parsing of a combination leading fixed-width > fields and subsequent variable length fiel