Re: RFR 8251989: Hex encoder and decoder utility

Hi Roger, I'm reading this version http://cr.openjdk.java.net/~rriggs/webrev-hex-formatter-8251989-1/src/java.base/share/classes/java/util/Hex.java On 2020-08-28 16:22, Roger Riggs wrote: Hi Raffaello, Missed sending this yesterday, the changes are in the updated webrev. On 8/23/20 10:42 A

Re: RFR 8251989: Hex encoder and decoder utility

Hi Raffaello, Missed sending this yesterday, the changes are in the updated webrev. On 8/23/20 10:42 AM, Raffaello Giulietti wrote: ... Hi Roger, here are my notes about the Decoder. (1) The only serious issue I could find is on L.548. It should read L.548   CharBuffer cb = CharBuffer.wrap(c

Re: RFR 8251989: Hex encoder and decoder utility

Hi Mark, The updates to the API based on the comments received are in the works. On 8/25/20 8:01 PM, mark.reinh...@oracle.com wrote: 2020/8/20 13:59:39 -0700, roger.ri...@oracle.com: On 8/20/20 3:10 PM, mark.reinh...@oracle.com wrote: ... A few comments: - Why do the short-form `encoder

Re: RFR 8251989: Hex encoder and decoder utility

Paul, Thanks for the review, there are a number of updates pending. On 8/24/20 4:55 PM, Paul Sandoz wrote: Hi Roger, A nice minimal addition. I agree with Mark’s naming suggestion. - Use appropriate code convention for static field names. - The encoder comes with two encoding production sign

Re: RFR 8251989: Hex encoder and decoder utility

2020/8/20 13:59:39 -0700, roger.ri...@oracle.com: > On 8/20/20 3:10 PM, mark.reinh...@oracle.com wrote: >> ... >> >> A few comments: >> >> - Why do the short-form `encoder` factory methods return encoders that >> produce upper-case hex strings? `Integer::toHexString` and other >> exist

Re: RFR 8251989: Hex encoder and decoder utility

Hi Corey, If there's a performance issue, that's an option. Though its tempting to add intrinsics for each supported instruction set, it adds code that has to be maintained forever, so it best to wait till its determined to be a bottleneck. Most of the cases I've seen in the code are fairly s

Re: RFR 8251989: Hex encoder and decoder utility

Just as a general comment, an SIMD-based encode() intrinsic would be an easy performance upgrade for this Hex class. I'd guess you'd get a 5x-10x improvement, depending on the specific CPU/arch being used. That said, the intrinsic would do better on larger buffers, and I don't know what the av

Re: RFR 8251989: Hex encoder and decoder utility

Hi Roger, A nice minimal addition. I agree with Mark’s naming suggestion. - Use appropriate code convention for static field names. - The encoder comes with two encoding production signatures, that returning a String and that for encoding into a StringBuilder. The decoder comes with just one su

RFR 8251989: Hex encoder and decoder utility

In the meantime I'll nevertheless take a look at the current implementation of the Decoder. Greetings Raffaello Hi Roger, here are my notes about the Decoder. (1) The only serious issue I could find is on L.548. It should read L.548 CharBuffer cb = CharBuffer.wrap(chars, index, index +

Re: RFR 8251989: Hex encoder and decoder utility

Hi Tagir, On 8/21/20 6:15 AM, Tagir Valeev wrote: Hello! Great and long-awaited API, thanks! How about wither-style construction? Like Hex.encoder().upperCase().withDelimiter(":").wrappedIn("{", "}"). This allows modifying existing encoder (e.g. adding prefix and suffix, preserving delimiter a

Re: RFR 8251989: Hex encoder and decoder utility

Hi Roger, On 2020-08-21 18:38, Roger Riggs wrote: Hi Rafaello, (It's Raffaello, with 2 effs.) I'll let some of the higher level API questions settle before updating the webrev. Thanks, Roger Sure. In the meantime I'll nevertheless take a look at the current implementation of the D

Re: RFR 8251989: Hex encoder and decoder utility

Hi Rafaello, On 8/21/20 11:44 AM, Raffaello Giulietti wrote: Hi Roger, I'm only a contributor, not an official reviewer. Despite this, I would like to make some notes about the encoder part. Reviews are always appreciated, whether Reviewer or not. (1) Comparing with other APIs that accept

RFR 8251989: Hex encoder and decoder utility

Hi Roger, I'm only a contributor, not an official reviewer. Despite this, I would like to make some notes about the encoder part. (1) Comparing with other APIs that accept a range of a byte[], my understanding is that encode(byte[], int, int) should throw IOOE when index < 0 or too big, eve

Re: RFR 8251989: Hex encoder and decoder utility

Hello! Great and long-awaited API, thanks! How about wither-style construction? Like Hex.encoder().upperCase().withDelimiter(":").wrappedIn("{", "}"). This allows modifying existing encoder (e.g. adding prefix and suffix, preserving delimiter and case). Also, you can specify only necessary parame

Re: RFR 8251989: Hex encoder and decoder utility

Hi Mark, On 8/20/20 3:10 PM, mark.reinh...@oracle.com wrote: 2020/8/19 14:14:56 -0700, roger.ri...@oracle.com: Please review a java.util.Hex API to encode and decode hexadecimal strings to and from byte arrays. JavaDoc: http://cr.openjdk.java.net/~rriggs/hex-javadoc/java.base/java/util/Hex.htm

Re: RFR 8251989: Hex encoder and decoder utility

Hi Max, The idea was to keep all the Hex functions together. Its a bit of a mismatch with StringBuilder; StringBuilder doesn't deal with arrays and is pretty bulky as it is. And StringBuilder doesn't have a corresponding parse companion; the string to byte[] array functions are needed to. Tr

Re: RFR 8251989: Hex encoder and decoder utility

2020/8/19 14:14:56 -0700, roger.ri...@oracle.com: > Please review a java.util.Hex API to encode and decode hexadecimal > strings to and from byte arrays. > > JavaDoc: > http://cr.openjdk.java.net/~rriggs/hex-javadoc/java.base/java/util/Hex.html This mostly looks good -- it’ll be nice to have a s

Re: RFR 8251989: Hex encoder and decoder utility

How about StringBuilder::appendHex instead of Encoder::encode(SB, byte[])? Then we don’t need to break the chain when appending many different things to a StringBuilder. Also, are prefix and suffix really useful? One can easily add them. Thanks, Max > On Aug 19, 2020, at 5:14 PM, Roger Riggs

Re: RFR 8251989: Hex encoder and decoder utility

Hi Raffaello, Will do, an oversight in updating the example. Thanks, Roger On 8/20/20 12:46 PM, Raffaello Giulietti wrote: Hi Roger, in the examples in the javadoc you might want to correct the first argument to Hex.encoder and Hex.decoder to "," to be consistent with the narrative. Gre

RFR 8251989: Hex encoder and decoder utility

Hi Roger, in the examples in the javadoc you might want to correct the first argument to Hex.encoder and Hex.decoder to "," to be consistent with the narrative. Greetings Raffaello JavaDoc: http://cr.openjdk.java.net/~rriggs/hex-javadoc/java.base/java/util/Hex.html

Re: RFR 8251989: Hex encoder and decoder utility

Hi Chris, Thanks for the comments, I'll make the updates suggested. Roger On 8/20/20 11:33 AM, Chris Hegarty wrote: On 19 Aug 2020, at 22:14, Roger Riggs wrote: .. JavaDoc: http://cr.openjdk.java.net/~rriggs/hex-javadoc/java.base/java/util/Hex.html I like it Roger, very nice. A few minor c

Re: RFR 8251989: Hex encoder and decoder utility

> On 19 Aug 2020, at 22:14, Roger Riggs wrote: > > .. > JavaDoc: > http://cr.openjdk.java.net/~rriggs/hex-javadoc/java.base/java/util/Hex.html I like it Roger, very nice. A few minor comments/quibbles: Hex: - "Utilities to encode bytes to hex strings and decode hex strings to bytes.” - This

RFR 8251989: Hex encoder and decoder utility

Please review a java.util.Hex API to encode and decode hexadecimal strings to and from byte arrays. Within the JDK and JDK tests there are multiple implementations to encode and decode hexadecimal strings to byte arrays. Hex encoders and decoders support upper or lower case hexadecimal charact