Re: RFR: 8282178: Replace simple iterations of Map.entrySet with Map.forEach calls

2022-02-23 Thread PROgrm_JARvis
On Wed, 23 Feb 2022 22:33:42 GMT, liach  wrote:

> Replaces simple `for (Map.Entry entry : map.entrySet())` with 
> `map.forEach((k, v) ->)` calls. This change is better for thread-safety and 
> reduces allocation for some map implementations.
> 
> A more in-depth description of benefits is available at 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-February/086201.html
>  and at the JBS issue itself.
> 
> A [jmh 
> comparison](https://jmh.morethan.io/?sources=https://gist.githubusercontent.com/liach/0c0f79f0c0b9b78f474d65cda2c5f7b5/raw/4f2a160c51164aefdfac6ab5a19bdbc8c65f5fcf/base-results.json,https://gist.githubusercontent.com/liach/0c0f79f0c0b9b78f474d65cda2c5f7b5/raw/4f2a160c51164aefdfac6ab5a19bdbc8c65f5fcf/head-results.json)
>  on the performance of the existing `HashMapBench` shows that the performance 
> of `putAll` for `HashMap` has not significantly changed.

Changes requested by jarviscr...@github.com (no known OpenJDK username).

src/java.base/share/classes/java/util/AbstractMap.java line 881:

> 879:  * used before indy and lambda expressions are ready in initPhase1.
> 880:  * Regular code can safely use {@code map::put} instead.
> 881:  */

Looks like `@param ` and `@param ` are missing although Javadoc-comment 
is used.

-

PR: https://git.openjdk.java.net/jdk/pull/7601


Integrated: 8277606: String(String) constructor could copy hashIsZero

2021-11-30 Thread PROgrm_JARvis
On Mon, 22 Nov 2021 22:59:21 GMT, PROgrm_JARvis  wrote:

> This is a trivial fix to make `String(String)` constructor copy the value of 
> `hashIsZero` field.

This pull request has now been integrated.

Changeset: e30e6767
Author:Petr Portnov 
Committer: Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk/commit/e30e67670981ee905724787c109b7b7fd2b70b42
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod

8277606: String(String) constructor could copy hashIsZero

Reviewed-by: redestad, rriggs

-

PR: https://git.openjdk.java.net/jdk/pull/6511


Re: RFR: 8277606: String(String) constructor could copy hashIsZero

2021-11-30 Thread PROgrm_JARvis
On Mon, 22 Nov 2021 22:59:21 GMT, PROgrm_JARvis  wrote:

> This is a trivial fix to make `String(String)` constructor copy the value of 
> `hashIsZero` field.

Hi there, could anyone please sponsor this change if it is still applicable?

Thanks in advance!

-

PR: https://git.openjdk.java.net/jdk/pull/6511


RFR: 8277606: String(String) constructor could copy hashIsZero

2021-11-22 Thread PROgrm_JARvis
This is a trivial fix to make `String(String)` constructor copy the value of 
`hashIsZero` field.

-

Commit messages:
 - fix: copy `hashIsZero` in `String#String(String)`

Changes: https://git.openjdk.java.net/jdk/pull/6511/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6511=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277606
  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6511.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6511/head:pull/6511

PR: https://git.openjdk.java.net/jdk/pull/6511


Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v3]

2021-11-20 Thread PROgrm_JARvis
On Sat, 20 Nov 2021 04:40:22 GMT, Stuart Marks  wrote:

> Regarding the slot limit in `StringConcatFactory`, it's not clear to me the 
> limit of 200 is normative or is merely an implementation note. The limit of 
> 200 slots seems to be arbitrary and shouldn't be baked into the spec. Perhaps 
> this limit can be removed if the splitting logic is moved into 
> `StringConcatFactory`.

Too me it looks as if it something that has to be considered as part of the 
specification, at least in the current state.
The 
[Javadoc](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/StringConcatFactory.html#makeConcat(java.lang.invoke.MethodHandles.Lookup,java.lang.String,java.lang.invoke.MethodType))
 explicitly states that:

> <...> the following linkage invariants must hold:
> - The number of parameter slots in `concatType` is less than or equal to 200
> <...>
> **Throws**:
> `StringConcatException` - If any of the linkage invariants described here are 
> violated, or the lookup context does not have private access privileges.

If it was an unspecified implementation detail, than arbitrary compilers (at 
least if we speak about `javac`) would have no formal reason to stick to this 
limit instead of using max available number of slots, which currently would 
lead to runtime `StringConcatException`.

For this reason the current state (IMO) should be considered the part of the 
specification and thus it seems reasonable to either:
- make it available to other code (via a public constant, at least)
- remove the magical number limit (as suggested by @stuart-marks and @cl4es) so 
that compilers no longer have to depend on a semi-specified magical number
- add an analog of current `StringConcatFactory` to some other place 
(`java.lang.runtime` seems to be a nice place, considering how most new 
`invokedynamic`-related bootstraps live there) but make it more strictly 
specified and keep the current `StringConcatFactory` untouched (simply 
delegating to the new one)

---

I understand that this is mostly out of this PR's scope and should definitely 
be discussed more wisely but I guess that it's worth starting this discussion 
here where it can be seen as an issue.

-

PR: https://git.openjdk.java.net/jdk/pull/6403


Re: RFR: 8261847: performace of java.lang.Record::toString should be improved

2021-11-17 Thread PROgrm_JARvis
On Tue, 16 Nov 2021 05:24:38 GMT, Vicente Romero  wrote:

> Please review this PR which aims to optimize the implementation of the 
> `toString` method we provide for records. A benchmark comparing the 
> implementation we are providing for records with lombok found out that lombok 
> is much faster mainly because our implementation uses `String::format`. This 
> fix is basically delegating on StringConcatFactory::makeConcatWithConstants 
> which is faster.
> 
> TIA
> 
> This is the result of the benchmark comparing records to lombok with vanilla 
> JDK:
> 
> Benchmark  Mode  CntScoreError  Units
> MyBenchmark.base   avgt40.849 ±  0.111  ns/op
> MyBenchmark.equals_record  avgt47.343 ±  2.740  ns/op
> MyBenchmark.equals_value   avgt46.644 ±  1.920  ns/op
> MyBenchmark.record_hash_code   avgt45.763 ±  3.882  ns/op
> MyBenchmark.record_to_string   avgt4  262.626 ± 12.574  ns/op 
>   <-- Before
> MyBenchmark.value_class_to_string  avgt4   30.325 ± 21.389  ns/op
> MyBenchmark.value_hash_codeavgt45.048 ±  3.936  ns/op
> 
> 
> after this patch:
> 
> Benchmark  Mode  Cnt   Score   Error  Units
> MyBenchmark.base   avgt4   0.680 ± 0.185  ns/op
> MyBenchmark.equals_record  avgt4   5.599 ± 1.348  ns/op
> MyBenchmark.equals_value   avgt4   5.718 ± 4.633  ns/op
> MyBenchmark.record_hash_code   avgt4   4.628 ± 4.368  ns/op
> MyBenchmark.record_to_string   avgt4  26.791 ± 1.817  ns/op   
>   <--- After
> MyBenchmark.value_class_to_string  avgt4  35.473 ± 2.626  ns/op
> MyBenchmark.value_hash_codeavgt4   6.152 ± 5.101  ns/op

By the way, considering the fact that the limit of `StringConcatFactory` (i.e. 
`200` arguments) is explicitly specified (at least, it is mentioned in its 
Javadoc) can we request to have it available to code (e.g. as a `public static 
final` constant) so that other code in JDK (like this PR) and foreign code can 
rely on it?

For example, I have to duplicate this value based on Javadoc 
[here](https://github.com/JarvisCraft/padla/blob/118c89e078cfc8e54faf154f2f4e658604159ab0/ultimate-messenger/src/main/java/ru/progrm_jarvis/ultimatemessenger/format/model/AsmTextModelFactory.java#L366)
 while there could have been something similar to 
`StringConcatFactory#MAX_ARGUMENTS`.

-

PR: https://git.openjdk.java.net/jdk/pull/6403


Integrated: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()

2021-11-08 Thread PROgrm_JARvis
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis  wrote:

> This is trivial fix of 
> [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686)
>  which replaces manually-computed `int`-based `long` hash-code.
> 
> Because `Long#hashCode(long)` uses other hashing function than the one 
> currently used here:
> 
> https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442
> 
> the value of `hashCode()` will produce different result, however this does 
> not seem to be a breaking change as `UUID#hashCode()` is not specified.
> 
> ---
> 
> Note: the comment to the bug states:
> 
>> Moved to JDK for further discussions of the proposed enhancement.
> 
> But as there seemed to be no corresponding discussion in core-libs-dev and 
> the change looks trivial, I considered that it is appropriate to open a PR 
> which (if needed) may be used for discussion (especially, considering the 
> fact that its comments get mirrored to the ML).

This pull request has now been integrated.

Changeset: cc2cac13
Author:Petr Portnov 
Committer: Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk/commit/cc2cac130cc28730a30d2e1d76bcb6ec8bc0b580
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod

8274686: java.util.UUID#hashCode() should use Long.hashCode()

Reviewed-by: rriggs

-

PR: https://git.openjdk.java.net/jdk/pull/5811


Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()

2021-11-07 Thread PROgrm_JARvis
On Mon, 4 Oct 2021 21:13:02 GMT, PROgrm_JARvis  wrote:

> This is trivial fix of 
> [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686)
>  which replaces manually-computed `int`-based `long` hash-code.
> 
> Because `Long#hashCode(long)` uses other hashing function than the one 
> currently used here:
> 
> https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442
> 
> the value of `hashCode()` will produce different result, however this does 
> not seem to be a breaking change as `UUID#hashCode()` is not specified.
> 
> ---
> 
> Note: the comment to the bug states:
> 
>> Moved to JDK for further discussions of the proposed enhancement.
> 
> But as there seemed to be no corresponding discussion in core-libs-dev and 
> the change looks trivial, I considered that it is appropriate to open a PR 
> which (if needed) may be used for discussion (especially, considering the 
> fact that its comments get mirrored to the ML).

Hi there, just a friendly reminder on this PR (as @bridgekeeper has attempted 
to mark it as stale).

Is there anything what should be done in order to have this PR integrated or is 
it just waiting for its time?

-

PR: https://git.openjdk.java.net/jdk/pull/5811


RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()

2021-10-04 Thread PROgrm_JARvis
This is trivial fix of 
[JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) 
which replaces manually-computed `int`-based `long` hash-code.

Because `Long#hashCode(long)` uses other hashing function than the one 
currently used here:

https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442

the value of `hashCode()` will produce different result, however this does not 
seem to be a breaking change as `UUID#hashCode()` is not specified.

---

Note: the comment to the bug states:

> Moved to JDK for further discussions of the proposed enhancement.

But as I there seemed to be no corresponding discussion in core-libs-dev and 
the change looks trivial, I considered that it is appropriate to open a PR 
which (if needed) may be used for discussion (especially, considering the fact 
that its comments get mirrored to the ML).

-

Commit messages:
 - fix: use `Long#hashCode(long)` for `UUID#hashCode()`

Changes: https://git.openjdk.java.net/jdk/pull/5811/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5811=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8274686
  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5811.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5811/head:pull/5811

PR: https://git.openjdk.java.net/jdk/pull/5811


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2021-02-15 Thread PROgrm_JARvis
On Wed, 10 Feb 2021 14:08:22 GMT, PROgrm_JARvis 
 wrote:

>>> Hi Claes,
>>> Would flattening the state of MD5 bring any further improvements?
>>> [plevart@92bf48f](https://github.com/plevart/jdk/commit/92bf48ff58f0ce9648e49466dbf1befebbf49083)
>> 
>> I think it might, marginally, but it seemed to me that the implCompress0 MD5 
>> intrinsic is using `state` so I've not tried that yet since rewriting and 
>> checking the intrinsic code is a lot of work. (I've blogged about my 
>> experiments in this area and mentioned this issue in passing: 
>> https://cl4es.github.io/2021/01/04/Investigating-MD5-Overheads.html#accidental-constraints)
>
>> 
>> 
>> @JarvisCraft This pull request has been inactive for more than 4 weeks and 
>> will be automatically closed if another 4 weeks passes without any activity. 
>> To avoid this, simply add a new comment to the pull request. Feel free to 
>> ask for assistance if you need help with progressing this pull request 
>> towards integration!

> @JarvisCraft Can you re-run your benchmarks in light of the changes that 
> @cl4es pointed out in the previous comment? We think this PR can be closed.

My apologies for the late reply,

I'll try re-running these with the merged branch ASAP (the delay is due to me 
changing the workstation thus having some job to reconfigure the environment).

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2021-02-10 Thread PROgrm_JARvis
On Thu, 7 Jan 2021 17:09:14 GMT, Claes Redestad  wrote:

>> Hi Claes,
>> Would flattening the state of MD5 bring any further improvements?
>> https://github.com/plevart/jdk/commit/92bf48ff58f0ce9648e49466dbf1befebbf49083
>
>> Hi Claes,
>> Would flattening the state of MD5 bring any further improvements?
>> [plevart@92bf48f](https://github.com/plevart/jdk/commit/92bf48ff58f0ce9648e49466dbf1befebbf49083)
> 
> I think it might, marginally, but it seemed to me that the implCompress0 MD5 
> intrinsic is using `state` so I've not tried that yet since rewriting and 
> checking the intrinsic code is a lot of work. (I've blogged about my 
> experiments in this area and mentioned this issue in passing: 
> https://cl4es.github.io/2021/01/04/Investigating-MD5-Overheads.html#accidental-constraints)

> 
> 
> @JarvisCraft This pull request has been inactive for more than 4 weeks and 
> will be automatically closed if another 4 weeks passes without any activity. 
> To avoid this, simply add a new comment to the pull request. Feel free to ask 
> for assistance if you need help with progressing this pull request towards 
> integration!

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-20 Thread PROgrm_JARvis
On Sun, 20 Dec 2020 19:48:43 GMT, Alan Bateman  wrote:

> I have to say that introducing a ThreadLocal here seems like a step in the 
> wrong direction. With a ThreadLocal, if I read this correctly, a 
> MessageDigest will be cached with each thread that ever calls this API, and 
> it won't be garbage collected until the thread dies. Some threads live 
> indefinitely, e.g., the ones in the fork-join common pool. Is this what we 
> want? Is UUID creation so frequent that each thread needs its own, or could 
> thread safety be handled using a simple locking protocol?

Fair enough; the solution proposed by Claes in #1855 seems to be a better 
alternative.

Currently, I will keep this PR open but I expect this to be superseded by the 
latter.

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 15:48:52 GMT, PROgrm_JARvis 
 wrote:

>> Might be fun to try, but it looks like rewriting to have MD5 to only use 
>> transient state will be a significant effort, and might just end up 
>> shuffling over allocations from `getInstance` to `digest`, which could 
>> regress code that reuse a digest instance (in a thread safe manner).
>
>> Might be fun to try, but it looks like rewriting to have MD5 to only use 
>> transient state will be a significant effort, and might just end up 
>> shuffling over allocations from `getInstance` to `digest`, which could 
>> regress code that reuse a digest instance (in a thread safe manner).
> 
> I've thought about using `ThreadLocal` at holder level and my naive benchmark 
> shows significant improvement in speed compared to current implementation: 
> https://gist.github.com/JarvisCraft/61cd45f83aea7f33cba9510e523a5c4e
> 
> As of the state from single thread's perspective, it seems to be safe to 
> reuse `MessageDigest` in this case:
> 
>> The `digest` method can be called once for a given number of updates. After 
>> `digest` has been called, the MessageDigest object is reset to its 
>> initialized state.

I've pushed the change utilizing `ThreadLocal` and my benchmark to correspond 
to the discussion.

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached [v3]

2020-12-18 Thread PROgrm_JARvis
> Please review this change moving lookup of MD5 digest in `java.lang.UUID` to 
> an internal holder class.

PROgrm_JARvis has updated the pull request incrementally with two additional 
commits since the last revision:

 - 8258588: add Md5MessageDigestLookup benchmark
 - 8258588: make UUID#Md5Digest thread-safe using ThreadLocal

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1821/files
  - new: https://git.openjdk.java.net/jdk/pull/1821/files/37db4c56..1abb7cfe

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1821=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1821=01-02

  Stats: 87 lines in 2 files changed: 77 ins; 5 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1821.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1821/head:pull/1821

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 15:24:21 GMT, Claes Redestad  wrote:

> Might be fun to try, but it looks like rewriting to have MD5 to only use 
> transient state will be a significant effort, and might just end up shuffling 
> over allocations from `getInstance` to `digest`, which could regress code 
> that reuse a digest instance (in a thread safe manner).

I've thought about using `ThreadLocal` at holder level and my naive benchmark 
shows significant improvement in speed compared to current implementation: 
https://gist.github.com/JarvisCraft/61cd45f83aea7f33cba9510e523a5c4e

As of the state from single thread's perspective, it seems to be safe to reuse 
`MessageDigest` in this case:

> The `digest` method can be called once for a given number of updates. After 
> `digest` has been called, the MessageDigest object is reset to its 
> initialized state.

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 14:55:08 GMT, Claes Redestad  wrote:

> A more general issue is that this patch assumes the `MessageDigest` object 
> returned is statically shareable, which implies it being stateless and 
> thread-safe.
> 
> This doesn't seem to be the case. See 
> [MD5.java](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/provider/MD5.java)
>  and the 
> [DigestBase.java](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/provider/DigestBase.java)
>  base class, which both have mutating buffers for doing the digest.

Wow, this is interesting. In this case I will check if there is a way to 
implement a thread-safe mechanism there. Will report here once there is some 
result.

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 14:37:53 GMT, Sean Mullan  wrote:

> MD5 and DES were removed as SE requirements in JDK 14. See 
> https://bugs.openjdk.java.net/browse/JDK-8214483 for more information. 
> However, there are no plans to remove the implementations from the JDK at 
> this time.

In this case, should a bug report be filled to require specifying behaviour for 
`UUID#nameUUIDFromBytes(byte[])` in case of MD5 not being present?

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 14:01:19 GMT, Claes Redestad  wrote:

> I'd suggest starting with a simple micro that zooms in on 
> MessageDigest.getInstance("MD5"), maybe like so:

Thanks, that's what I wanted to hear.
I will implement it now.

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 13:39:48 GMT, Alan Bateman  wrote:

> Can you look in test/micro for existing examples?

Yes, of course, the question is more about the following: should I simply cover 
`UUID#nameUUIDFromBytes(byte[])` by the benchmark or should I rather write a 
comparison benchmark which would compare finding the MessageDigest on every 
iteration against using the holder for this purpose (without any direct 
reference to `UUID` class).

> It might be that the right place to cache is in `MessageDigest` rather than 
> `UUID` so that other code can benefit too.

`UUID.Md5Digest` may be moved to `MessageDigest` but this seems to be a case to 
implement something similar to this in the way similar to `StandardCharsets`.

> One other point is that Standard Algorithms specifies that MD5 is supported 
> by MessageDigest so the JDK would be broken it could not be found. If the 
> eventual patch is in UUID then this aspect will need a bit of cleanup.

I've looked through [Standard Algorithms section for 
MessageDigest](https://docs.oracle.com/en/java/javase/15/docs/specs/security/standard-names.html#messagedigest-algorithms)
 and is says

> Algorithm names that *can* be specified

And the javadoc of `MessageDigest` says:
> Every implementation of the Java platform is required to support the 
> following standard `MessageDigest` algorithms:
> - `SHA-1`
> - `SHA-256`

So I cannot find any requirement for `MD5` to be present. Although I believe 
that every implementation does provide it, it may be essential to either 
specify it or describe the behavior for its absence in case of `UUID`'s usage.

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-18 Thread PROgrm_JARvis
On Fri, 18 Dec 2020 09:10:26 GMT, Alan Bateman  wrote:

> If there is a holder class for the MessageDigest then its initialisation can 
> fail, this would allow the orThrow method to go away

As of allowing `Md5Digest` instead of explicitly throwing the exception from 
`orThrow` I think that the latter is more appropriate as in case of allowing 
the class initialization to fail all non-first calls will lead to 
`NoClassDefFoundError` which will be counterintuitive from users' perspective 
although the behaviour is simply unspecified for this in contract of 
`nameUUIDFromBytes` (Javadoc of `MessageDigest` requires the existence of 
`SHA-1` and `SHA-256` but nothing is said about `MD5` (should a bug-report be 
raised for the purpose of specifying this detail in the Javadoc?).

> Are you planning to add a microbenchmark to demonstrate the issue?

As for testing I am ready to write a benchmark but I am unsure what is the 
canonical way to write the one comparing the value before and after changeset. 
Or should it simply be a comparison of the very approach used here, not exactly 
the changed method (i.e. writing the benchmarks using the try/catch variant and 
holder variant and comparing these)? Could you please give any example of such 
if there are some.

Thanks in advance!

-

PR: https://git.openjdk.java.net/jdk/pull/1821


Re: RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached [v2]

2020-12-17 Thread PROgrm_JARvis
> Please review this change moving lookup of MD5 digest in `java.lang.UUID` to 
> an internal holder class.

PROgrm_JARvis has updated the pull request incrementally with one additional 
commit since the last revision:

  8258588: add missing " UUIDs" to comment of UUID$Md5Digest

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1821/files
  - new: https://git.openjdk.java.net/jdk/pull/1821/files/73eb2c4f..37db4c56

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1821=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1821=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1821.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1821/head:pull/1821

PR: https://git.openjdk.java.net/jdk/pull/1821


RFR: 8258588: MD5 MessageDigest in java.util.UUID should be cached

2020-12-17 Thread PROgrm_JARvis
Please review this change moving lookup of MD5 digest in `java.lang.UUID` to an 
internal holder class.

-

Commit messages:
 - 8258588: MD5 MessageDigest in java.util.UUID should be cached

Changes: https://git.openjdk.java.net/jdk/pull/1821/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1821=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258588
  Stats: 33 lines in 1 file changed: 26 ins; 6 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1821.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1821/head:pull/1821

PR: https://git.openjdk.java.net/jdk/pull/1821