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

2020-12-18 Thread Alan Bateman
On Thu, 17 Dec 2020 13:36:17 GMT, PROgrm_JARvis wrote: > Please review this change moving lookup of MD5 digest in `java.lang.UUID` to > an internal holder class. Are you planning to add a microbenchmark to demonstrate the issue? If there is a holder class for the MessageDigest then its initial

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 la

Re: [jdk16] RFR: 8256693: getAnnotatedReceiverType parameterizes types too eagerly [v2]

2020-12-18 Thread Vicente Romero
On Thu, 17 Dec 2020 10:07:13 GMT, Joel Borggrén-Franck wrote: >> The fix for JDK-8256693 too often produces a ParameterizedType as the result >> of getAnnotatedReceiverType().getType() . A ParameterizedType is necessary >> only when this type or any of its transitive owner types has type >> p

[jdk16] Integrated: 8256693: getAnnotatedReceiverType parameterizes types too eagerly

2020-12-18 Thread Joel Borggrén-Franck
On Wed, 16 Dec 2020 09:41:47 GMT, Joel Borggrén-Franck wrote: > The fix for JDK-8256693 too often produces a ParameterizedType as the result > of getAnnotatedReceiverType().getType() . A ParameterizedType is necessary > only when this type or any of its transitive owner types has type paramete

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

2020-12-18 Thread Alan Bateman
On Fri, 18 Dec 2020 13:27:02 GMT, PROgrm_JARvis wrote: >> Are you planning to add a microbenchmark to demonstrate the issue? >> If there is a holder class for the MessageDigest then its initialisation can >> fail, this would allow the orThrow method to go away > >> If there is a holder class fo

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

2020-12-18 Thread Claes Redestad
On Fri, 18 Dec 2020 13:39:48 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 thin

Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll

2020-12-18 Thread Сергей Цыпанов
Hi Simon, I've removed 'but this method is likely to run significantly faster under most implementations.' so the doc is cleaner now Cheers, Sergey 17.12.2020, 16:38, "Simon Roberts" : > When updating this, might you take the opportunity to remove the ambiguous > antecedent too? The use of "this

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 woul

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: 8193031: Collections.addAll is likely to perform worse than Collection.addAll [v2]

2020-12-18 Thread Сергей Цыпанов
On Thu, 17 Dec 2020 11:07:55 GMT, Rémi Forax wrote: >> Should I then wait for the fix of that bug to remove `@SupressWarnings`? > > I don't think so, the code is fine, for me. I've looked into `com.sun.tools.javac.comp.Check` and it seems the warning is produced here: @Override

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

2020-12-18 Thread Sean Mullan
On Fri, 18 Dec 2020 14:29:00 GMT, PROgrm_JARvis wrote: >> There are pre-existing microbenchmarks for java.security under >> test/micro/org/openjdk/bench/java/security >> >> You can build and run these using `make test TEST=micro:YourBenchmark`. >> Refer to >> [doc/testing.md](https://github.

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, sho

Re: RFR: JDK-8223322: Improve concurrency in jpackage instances

2020-12-18 Thread Andy Herrick
On Thu, 17 Dec 2020 23:26:44 GMT, Alexey Semenyuk wrote: >> Changes requested by asemenyuk (Committer). > > I'd propose to update the test to run more that two concurrent instance of > jpackage command. > > > Changes looks fine, but are we sure that external 3rd party tools used by > jpackag

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

2020-12-18 Thread Claes Redestad
On Fri, 18 Dec 2020 14:42:38 GMT, PROgrm_JARvis wrote: >>> 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_ b

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.co

Re: RFR: JDK-8223322: Improve concurrency in jpackage instances

2020-12-18 Thread Andy Herrick
On Thu, 17 Dec 2020 23:26:44 GMT, Alexey Semenyuk wrote: > > > I'd propose to update the test to run more that two concurrent instance of > jpackage command. yes - I'm planning for a lot more in next revision, your suggestion to use only PackageTest.Action.CREATE will go a long way to making

Re: RFR: JDK-8223322: Improve concurrency in jpackage instances

2020-12-18 Thread Andy Herrick
On Fri, 18 Dec 2020 14:59:06 GMT, Andy Herrick wrote: >> I'd propose to update the test to run more that two concurrent instance of >> jpackage command. > >> >> >> I'd propose to update the test to run more that two concurrent instance of >> jpackage command. > > yes - I'm planning for a lot

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

2020-12-18 Thread Claes Redestad
On Fri, 18 Dec 2020 14:59:10 GMT, PROgrm_JARvis 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://git

Re: RFR: JDK-8223322: Improve concurrency in jpackage instances

2020-12-18 Thread Andy Herrick
On Fri, 18 Dec 2020 15:00:13 GMT, Andy Herrick wrote: >>> >>> >>> I'd propose to update the test to run more that two concurrent instance of >>> jpackage command. >> >> yes - I'm planning for a lot more in next revision, your suggestion to use >> only PackageTest.Action.CREATE will go a long

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

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#Md5Dige

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 t

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

2020-12-18 Thread Sean Mullan
On Fri, 18 Dec 2020 14:42:38 GMT, PROgrm_JARvis 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

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

2020-12-18 Thread Stuart Marks
On Fri, 18 Dec 2020 19:04:36 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

Re: RFR: JDK-8223322: Improve concurrency in jpackage instances

2020-12-18 Thread Alexey Semenyuk
On Fri, 18 Dec 2020 14:59:06 GMT, Andy Herrick wrote: > I have had problems with test infrastructure fixing the workDir based on test > class and method, and then getting IOExceptions as various PackageTest's try > to write the same content to the same work dir I don't think we need changes to

[jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest

2020-12-18 Thread Brent Christian
This change adds some extra test output for NativeLibraryTest, primarily via an update to the ForceGC utility class. It was observed that there was nothing preventing the Cleaner from cleaning the short-lived Object that ForceGC registers before await()/doit()/System.gc() is even called. The n

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest

2020-12-18 Thread Mandy Chung
On Fri, 18 Dec 2020 22:33:11 GMT, Brent Christian wrote: > This change adds some extra test output for NativeLibraryTest, primarily via > an update to the ForceGC utility class. > > It was observed that there was nothing preventing the Cleaner from cleaning > the short-lived Object that ForceG

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest [v2]

2020-12-18 Thread Naoto Sato
On Fri, 18 Dec 2020 23:34:04 GMT, Brent Christian wrote: >> This change adds some extra test output for NativeLibraryTest, primarily via >> an update to the ForceGC utility class. >> >> It was observed that there was nothing preventing the Cleaner from cleaning >> the short-lived Object that F

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest [v2]

2020-12-18 Thread Brent Christian
> This change adds some extra test output for NativeLibraryTest, primarily via > an update to the ForceGC utility class. > > It was observed that there was nothing preventing the Cleaner from cleaning > the short-lived Object that ForceGC registers before > await()/doit()/System.gc() is even ca

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest [v2]

2020-12-18 Thread Brent Christian
On Fri, 18 Dec 2020 22:43:16 GMT, Mandy Chung wrote: >> Brent Christian has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add sleep to ForceGC.await() > > test/lib/jdk/test/lib/util/ForceGC.java line 49: > >> 47: System.gc

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest [v2]

2020-12-18 Thread Mandy Chung
On Fri, 18 Dec 2020 23:37:11 GMT, Brent Christian wrote: >> This change adds some extra test output for NativeLibraryTest, primarily via >> an update to the ForceGC utility class. >> >> It was observed that there was nothing preventing the Cleaner from cleaning >> the short-lived Object that F

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest [v3]

2020-12-18 Thread Brent Christian
> This change adds some extra test output for NativeLibraryTest, primarily via > an update to the ForceGC utility class. > > It was observed that there was nothing preventing the Cleaner from cleaning > the short-lived Object that ForceGC registers before > await()/doit()/System.gc() is even ca

Re: [jdk16] RFR: 8258007: Add instrumentation to NativeLibraryTest [v3]

2020-12-18 Thread Brent Christian
On Fri, 18 Dec 2020 23:31:35 GMT, Naoto Sato wrote: >> Brent Christian has updated the pull request incrementally with one >> additional commit since the last revision: >> >> format() -> println() > > test/lib/jdk/test/lib/util/ForceGC.java line 48: > >> 46: for (int i = 0; i < 1

[jdk16] Integrated: 8258007: Add instrumentation to NativeLibraryTest

2020-12-18 Thread Brent Christian
On Fri, 18 Dec 2020 22:33:11 GMT, Brent Christian wrote: > This change adds some extra test output for NativeLibraryTest, primarily via > an update to the ForceGC utility class. > > It was observed that there was nothing preventing the Cleaner from cleaning > the short-lived Object that ForceG