Re: RFR: 8273000: Remove WeakReference-based class initialisation barrier implementation
On Wed, 25 Aug 2021 22:05:24 GMT, Vladimir Ivanov wrote: > Get rid of WeakReference-based logic in > DirectMethodHandle::checkInitialized() and reimplement it with > `Unsafe::ensureClassInitialized()`/`shouldBeInitialized()`. > > The key observation is that `Unsafe::ensureClassInitialized()` does not block > the initializing thread. > > Also, removed `Unsafe::shouldBeInitialized()` in > `DMH::shouldBeInitialized(MemberName)` to save on calling into the VM. > `Unsafe::ensureClassInitialized()` already has a fast-path check which checks > whether the class is fully initialized or not. > > Testing: tier1 - tier6 src/java.base/share/classes/java/lang/invoke/DirectMethodHandle.java line 385: > 383: Class defc = member.getDeclaringClass(); > 384: UNSAFE.ensureClassInitialized(defc); // initialization barrier; > blocks unless called by the initializing thread > 385: return !UNSAFE.shouldBeInitialized(defc); // keep the barrier if > the initialization is still in progress I think some more elaborate commentary about the possibility of this being called while of defc is already on the call stack, would be worthwhile - the existing comments are a little too subtle IMO. UNSAFE.ensureClassInitialized(defc); // Once we get here either defc was fully initialized by another thread, or // defc was already being initialized by the current thread. In the latter case // the barrier must remain. We can detect this simply by checking if initialization // is still needed. return !UNSAFE.shouldBeInitialized(defc); - PR: https://git.openjdk.java.net/jdk/pull/5258
Re: RFR: 8272541: Incorrect overflow test in Toom-Cook branch of BigInteger multiplication
On Mon, 16 Aug 2021 21:00:00 GMT, Brian Burkhalter wrote: > Please consider this change which would modify a conditional `a + b > c` > where the left side variables are `int`s and the right side is > `(long)Integer.MAX_VALUE + 1`. The change is to cast the left side variables > to `long`s. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5130
Integrated: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
On Mon, 23 Aug 2021 21:01:48 GMT, Andrey Turbanov wrote: > Collections.sort is just a wrapper, so it is better to use an instance method > directly. This pull request has now been integrated. Changeset: d732c309 Author:Andrey Turbanov Committer: Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/d732c3091fea0a7c6d6767227de89002564504e5 Stats: 27 lines in 10 files changed: 0 ins; 8 del; 19 mod 8272863: Replace usages of Collections.sort with List.sort call in public java modules Reviewed-by: serb, dfuchs, naoto - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform [v2]
On Wed, 4 Aug 2021 07:25:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8269373 bug fixes? >> >> These tests call java.lang.ProcessBuilder in direct, so not used jtreg >> command option. To run non-localized tests, -Duser.language=en and >> -Duser.country=US options should be added in ProcessBuilder. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > 8269373: use test opts for process arguments I think implicitly expecting locales to be set to en-US by specifying `test.vm.opts` is fragile which would introduce test instability. In fact, looking at some of the tests, e.g., `HelpFlagsTest` at line 332, the intention is to silently exit in case it is not English. I think it is what the test is intended and it is a bug if it fails with other locales. - PR: https://git.openjdk.java.net/jdk/pull/4594
Re: RFR: 8272861: Add a micro benchmark for vector api [v4]
> This pull request adds a micro benchmark for Vector API. > The Black Scholes algorithm is implemented with and without Vector API. > We see about ~6x gain with Vector API for this micro benchmark using 256 bit > vectors. Sandhya Viswanathan has updated the pull request incrementally with one additional commit since the last revision: No need to normalize nextFloat - Changes: - all: https://git.openjdk.java.net/jdk/pull/5234/files - new: https://git.openjdk.java.net/jdk/pull/5234/files/5b4abbf9..df22def3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5234=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5234=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5234/head:pull/5234 PR: https://git.openjdk.java.net/jdk/pull/5234
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading
On Wed, 25 Aug 2021 09:31:51 GMT, Vladimir Ivanov wrote:
> `MethodHandle.asTypeCache` keeps a strong reference to adapted `MethodHandle`
> and it can introduce a class loader leak through its `MethodType`.
>
> Proposed fix introduces a 2-level cache (1 element each) where 1st level can
> only contain `MethodHandle`s which are guaranteed to not introduce any
> dependencies on new class loaders compared to the original `MethodHandle`.
> 2nd level is backed by a `SoftReference` and is used as a backup when the
> result of `MethodHandle.asType()` conversion can't populate the higher level
> cache.
>
> The fix is based on [the
> work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/)
> made by Peter Levart @plevart back in 2015.
>
> Testing: tier1 - tier6
Looks good.
I guess it is not that common for the soft ref to get instantiated i.e. for the
case of the ~common class loader of `type`, MTC say, and the ~common classoader
of `newType`, NMTC say, then NMTC is not an ancestor of MTC.
It's possible that `asTypeCache` and `asTypeSoftCache` could both be non-null
i.e. we don't null out one of them, buy does not seem a problem.
src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 869:
> 867: }
> 868: at = asTypeUncached(newType);
> 869: return setAsTypeCache(at);
The following may be a little clearer
Suggestion:
MethodHandle at = asTypeCached(newType);
return at != null
? at
: setAsTypeCache(asTypeUncached(newType));
src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 953:
> 951:
> 952: /* Determine whether {@code descendant} keeps {@code ancestor} alive
> through the loader delegation chain. */
> 953: private static boolean keepsAlive(ClassLoader ancestor, ClassLoader
> descendant) {
Might be clearer to name the method by what it is e.g. isAncestor
// Returns true if ancestor can be found descendant's delegation chain.
-
Marked as reviewed by psandoz (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5246
Integrated: 8272473: Parsing epoch seconds at a DST transition with a non-UTC parser is wrong
On Mon, 23 Aug 2021 16:42:03 GMT, Naoto Sato wrote: > Please review the fix to the subject issue. When instant seconds and zone > co-exist in parsed data, instant seconds was not resolved correctly from them. This pull request has now been integrated. Changeset: fe7d7088 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/fe7d70886cc9985478c5810eff0790648a9aae41 Stats: 14 lines in 2 files changed: 8 ins; 0 del; 6 mod 8272473: Parsing epoch seconds at a DST transition with a non-UTC parser is wrong Reviewed-by: joehw, rriggs, iris, lancea, scolebourne - PR: https://git.openjdk.java.net/jdk/pull/5225
Re: what does the spec say about file paths that are too long?
On 26/08/2021 15:43, Alan Snyder wrote: As I said, I think it's great that we agree an exception should be thrown when an over-length path is used in an actual file operation. However, would it not be better to have that in the specification, rather than relying on the opinions of individual engineers expressed in a thread on a mailing list? And would it not also be better if there were test cases? You are welcome to propose additional tests if you want. I also think it would be good to have a specific exception to use for such cases. It would to have optional (see the "Optional Specific Exceptions" sections of the javadoc) as you can't guarantee that ENAMETOOLONG/equivalent would happen in all cases. Regarding path resolving, I am not surprised that the JDK may have to resolve paths before use in some cases. Your original comment seemed to imply that *all* uses were resolved by the JDK first, and that would surprise me. Granted that path operations cannot in general predict when a path will be of usable length in a file operation, the question remains whether path operations should report over-length paths in those cases where it can be determined. Is it not the case that some OS APIs have hard limits on path length (unrelated to limits imposed by the file system itself)? For a file provider using such an API, would throwing an exception when that limit is exceeded be a good idea or a bad idea? I don't think this is feasible or a compatible change. -Alan
Re: RFR: 8273000: Remove WeakReference-based class initialisation barrier implementation
On Wed, 25 Aug 2021 22:05:24 GMT, Vladimir Ivanov wrote: > Get rid of WeakReference-based logic in > DirectMethodHandle::checkInitialized() and reimplement it with > `Unsafe::ensureClassInitialized()`/`shouldBeInitialized()`. > > The key observation is that `Unsafe::ensureClassInitialized()` does not block > the initializing thread. > > Also, removed `Unsafe::shouldBeInitialized()` in > `DMH::shouldBeInitialized(MemberName)` to save on calling into the VM. > `Unsafe::ensureClassInitialized()` already has a fast-path check which checks > whether the class is fully initialized or not. > > Testing: tier1 - tier6 That's a nice cleanup to a tricky area (one of the few used to trigger an update a final field). In effect we were already relying on that behavior in the `ClassValue` computation. May i suggest that we add some JavaDoc to `ensureClassInitialized` describing the two cases of the calling thread is the initialization thread or not. - Marked as reviewed by psandoz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5258
Re: RFR: 8272861: Add a micro benchmark for vector api [v3]
On Tue, 24 Aug 2021 20:49:52 GMT, Sandhya Viswanathan
wrote:
>> This pull request adds a micro benchmark for Vector API.
>> The Black Scholes algorithm is implemented with and without Vector API.
>> We see about ~6x gain with Vector API for this micro benchmark using 256 bit
>> vectors.
>
> Sandhya Viswanathan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Make constants as static final
Very nice, just one minor comment (no need for another review), thanks for
contributing this.
test/micro/org/openjdk/bench/jdk/incubator/vector/BlackScholes.java line 60:
> 58:
> 59: float randFloat(float low, float high) {
> 60:float val = rand.nextFloat()/Float.MAX_VALUE;
`nextFloat` returns a PSR between 0 and 1., so no need to divide by
`Float.MAX_VALUE` ?
-
Marked as reviewed by psandoz (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5234
Re: what does the spec say about file paths that are too long?
As I said, I think it's great that we agree an exception should be thrown when an over-length path is used in an actual file operation. However, would it not be better to have that in the specification, rather than relying on the opinions of individual engineers expressed in a thread on a mailing list? And would it not also be better if there were test cases? I also think it would be good to have a specific exception to use for such cases. Regarding path resolving, I am not surprised that the JDK may have to resolve paths before use in some cases. Your original comment seemed to imply that *all* uses were resolved by the JDK first, and that would surprise me. Granted that path operations cannot in general predict when a path will be of usable length in a file operation, the question remains whether path operations should report over-length paths in those cases where it can be determined. Is it not the case that some OS APIs have hard limits on path length (unrelated to limits imposed by the file system itself)? For a file provider using such an API, would throwing an exception when that limit is exceeded be a good idea or a bad idea? Alan > On Aug 26, 2021, at 3:39 AM, Alan Bateman wrote: > > On 25/08/2021 23:12, Alan Snyder wrote: >> Lacking any new data, I guess it is fair to assume that there is no >> specification for the behavior of methods that use file paths that are too >> long, and presumably no tests, either. >> >> So the next question is whether there should be such a specification. >> >> I think there should be a specification because I would like to be able to >> use file paths without having to defend against possible unwanted bad >> effects when the paths are too long. By analogy, more like array indexing >> than integer overflow. >> >> If there is to be a specification, should it be at the method level? That >> would be best, I think. >> >> For example, the Path.toAbsolutePath() method in principle could return a >> path that is “too long” even if the original path is fine. Should an >> exception be raised at that point or only when the absolute path is used? >> This distinction was not possible with File, but it may be possible with >> Path, given the association with file providers. >> >> (Regarding the comment from Alan B., is it the case that file paths are >> necessarily resolved before use? That would surprise me.) > As I said, you should see an I/O exception if you attempt to access the file > system with a file path that is too long to locate a file. There are a couple > of APIs that return a boolean rather than throw and they should all fail > (usually by returning false) if the file path is too long. If you do find a > case where you think the file path is "silently truncated" then please bring > it up so we can see if this is a JDK or operating system issue. There was > some exploration, in the JDK 7 time frame, into defining APIs that expose > limits but it gets unapproachable very quickly due to handling by specific > operating systems and encoding/normalization at the low-level file system > level. > > I see your other mails asking if resolving or combining paths should fail. > That is not feasible in general. Bernd mentioned some of the issues. If you > add sym or hard links to the discussion then you will quickly see that you > don't actually know which file system or file will be accessed until you > attempt the access. You also mentioned being surprised that the JDK may have > to "resolve" paths. It has to in some cases, the most obvious being long > paths on Windows that need transformation and a prefix to order to generate > the file path for the Windows call. > > -Alan. >
Re: Proposal: JDK-8231640 - (prop) Canonical property storage
Hello Roger, Sorry, I just read the top part of your reply the last time and didn't realize there were inline comments. I just noticed them. Replying inline. On 24/08/21 8:14 pm, Roger Riggs wrote: Hi Jaikiran, Thanks for taking this on and getting it started. One use case of canonical storage is repeatable builds. It would be useful to identify the uses in the JDK that would need to be changed to use the new function. On 8/24/21 10:07 AM, Jaikiran Pai wrote: The java.util.Properties class allows the properties to be written out to a stream or through a writer. In its current form, the specification of these APIs state that a comment comprising of the current date is always written out. The spec doesn't make any guarantees about the order of the properties when they are written out. There have been requests asking to make these APIs more deterministic. These requests, from what I have seen, mainly ask for: - A way to disable writing out the date comment - A way to write out the properties in a deterministic and reproducible way There have been discussions in the mailing list in the past which have been captured in JDK-8231640[1]. In these discussions, there has been an inclination to not touch the current existing API implementations and instead introduce new API(s) to achieve the proposed use cases. Before starting off with an implementation, I wanted to try and get some inputs on what the new API(s) would look like and what the scope of such a work should be. Right now, the Properties class has 2 "store" APIs: public void store(Writer writer, String comments) throws IOException public void store(OutputStream out, String comments) throws IOException I don't think two methods are needed, its easy enough for the caller to adapt an OutputStream to a Writer (OutputStreamWriter) and take control of the encoding, so the OutputStream version is not essential. That's a good point and makes sense. Speaking of optional comments, should the APIs accept an instance of java.util.Optional for the comments parameter. Perhaps: public void storeCanonical(Writer writer, Optional comments) throws IOException public void storeCanonical(OutputStream out, Optional comments) throws IOException Optional is overkill here, using null for no comment is conventional and matches the current usage in the store(..) methods. Okay. Not using Optional sounds fine. Coming to the part where we write out the properties, these APIs will write out the properties in the lexicographical order of the property keys. An additional enhancement perhaps could be to allow users to pass in an optional java.util.Comparator instance to provide for application specific ordering of the property keys while being written out by these APIs. I am not too sure if we should introduce that. Any inputs? If we do introduce it, we would end up with 4 new APIs: public void storeCanonical(Writer writer, Optional comments) throws IOException public void storeCanonical(OutputStream out, Optional comments) throws IOException public void storeCanonical(Writer writer, Optional comments, Comparator keyOrderer) throws IOException public void storeCanonical(OutputStream out, Optional comments, Comparator keyOrderer) throws IOException Canonical usually already means a non-variable encoding, that seems the inconsistent with providing a Comparator. From the inputs received so far, there hasn't been a real use case where a custom user provided Comparator would be of genuine help. So I don't plan to look more into this aspect. However, it should be a goal that properties stored with storeCanonical can be loaded with load(). Agreed. Is that worth it? Finally, the other semantics, like the property key value separators, how/where newlines are inserted, what character encoding is used etc... will continue to match with the current semantics of the "store" APIs. If a client has the need for a custom format, its quite easy to iterate over the contents, sorting if desires and writing the format itself. A custom format would not be usable with Properties.load. Simpler is better, Agreed. -Jaikiran
Re: Proposal: JDK-8231640 - (prop) Canonical property storage
Hello Magnus, On 25/08/21 5:33 pm, Magnus Ihse Bursie wrote: ... One thing I do remember is the JDK build (through the make files) would have certain Java code it would call to do some build steps. Is there a easy way to find all such build related Java files within the JDK? I would like to see if there are any references/calls to this method from those build files. I am doing some searches myself, but knowing where to search will give me more confidence that I haven't missed out any. The java buildtool sources are located in the "make" directory, more specifically in "make/jdk/src", "make/langtools/src" and "make/src" (yeah, I know -- a cleanup is way overdue). I did a quick search now but could not find any references to Properties.store(). Thank you for that. I went ahead and verified it again and like you note, I don't see any usages in this code. I'm trying to remember if we create properties during the build... We have several instances where .properties files in the Java source code are converted to hard-coded classes (for performance), and other where .properties files are copied verbatim. Ah, right, they are "cleaned" beforehand. We used to do this by a Java program, but nowadays they are mangled by sed. I think replacing that sed script with a trivial Java program doing storeCanonical() would be on the list of things to do. I can assist with that, when you are starting to get your implementation done. Thank you. Once the initial draft version is ready, I will contact you. We might also write something as part of the jlink process that gets embedded in the resulting jimage; not quite sure about that. You should find that code in the normal src/ codebase though. I had a look at the the jlink and image building code and I haven't found any references/usages in this area. -Jaikiran
Re: Proposal: JDK-8231640 - (prop) Canonical property storage
On 25/08/2021 15:57, Jaikiran Pai wrote: : Introducing an overloaded "store" which takes the timestamp or implicitly using the SOURCE_DATE_EPOCH environment variable would mean that the Properties.store(...) APIs will continue to always write out a Date representation into the outputstream. That effectively means that there will continue to be no way to disable writing out a (date) comment. More specifically, even if a user doesn't explicitly specify a comment, we would end up writing a default (date) comment. Do we want that? Or while we are doing these changes, should we introduce this ability of disabling writing out these date comments by default? That, IMO, can only be done by introducing new APIs instead of trying to slightly change the spec and the implementation of the current "store" APIs. After all, if any callers do want a date (and a reproducible one at that), they could always do so by passing that as a value for the "comment" parameter of these (new) "storeXXX" APIs. Properties save/store has always emitted a comment line with the current date/time, it goes back to JDK 1.0. It's unfortunate that newer store methods didn't re-examine this, also unfortunate that it continued with 8859-1. In the discussion on jdk-dev about reproducibility then I think we concluded that we don't want to change the behavior of existing methods, hence the discussion on introducing a new method. An new overload of store would give the maximum flexibility to new code but it would require programs used in builds to use it consistently. It's possible that libraries or tools that are using Properties::store have no idea that they will ultimately be used in a system that is trying to produce reproducible builds. So I have some sympathy to the argument that there should a way to omit the date or emit it as the Unix epoch time or a fixed value. This would mean changing the spec to allow for some implementation means to do this, and maybe an implNote that documents that it reads SOURCE_DATE_EPOCH. I think Roger has misgivings about this. So let's list the options and the pros/cons and see if we can converge on an approach. I think, based on the discussion/inputs so far, there's clarity on this part that changing the current implementation of "store" to write out the property keys in a specific order would be a good thing, irrespective of whether or not we introduce new APIs to deal with the date comment aspect. Daniel had an interesting point of logging.properties file and the order that would need to be maintained for those, but like he noted in that same reply, I don't think that's a use case to consider for the "store" APIs, since they never previously supported/guaranteed that use case. Yes, I think we should be okay there. I suspect the iteration order has changed once or twice already, e.g. when Properties was re-implemented to use a CHM. -Alan
Re: what does the spec say about file paths that are too long?
On 25/08/2021 23:12, Alan Snyder wrote: Lacking any new data, I guess it is fair to assume that there is no specification for the behavior of methods that use file paths that are too long, and presumably no tests, either. So the next question is whether there should be such a specification. I think there should be a specification because I would like to be able to use file paths without having to defend against possible unwanted bad effects when the paths are too long. By analogy, more like array indexing than integer overflow. If there is to be a specification, should it be at the method level? That would be best, I think. For example, the Path.toAbsolutePath() method in principle could return a path that is “too long” even if the original path is fine. Should an exception be raised at that point or only when the absolute path is used? This distinction was not possible with File, but it may be possible with Path, given the association with file providers. (Regarding the comment from Alan B., is it the case that file paths are necessarily resolved before use? That would surprise me.) As I said, you should see an I/O exception if you attempt to access the file system with a file path that is too long to locate a file. There are a couple of APIs that return a boolean rather than throw and they should all fail (usually by returning false) if the file path is too long. If you do find a case where you think the file path is "silently truncated" then please bring it up so we can see if this is a JDK or operating system issue. There was some exploration, in the JDK 7 time frame, into defining APIs that expose limits but it gets unapproachable very quickly due to handling by specific operating systems and encoding/normalization at the low-level file system level. I see your other mails asking if resolving or combining paths should fail. That is not feasible in general. Bernd mentioned some of the issues. If you add sym or hard links to the discussion then you will quickly see that you don't actually know which file system or file will be accessed until you attempt the access. You also mentioned being surprised that the JDK may have to "resolve" paths. It has to in some cases, the most obvious being long paths on Windows that need transformation and a prefix to order to generate the file path for the Windows call. -Alan.
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v6]
On Wed, 25 Aug 2021 07:40:56 GMT, Nick Gasson wrote: > I've run the benchmark on several different machines and didn't see any > performance regressions, and the speed-up for longer strings looks quite > good. I also ran jtreg tier1-3 with no new failures so I think this is ok. > > If you fix the Windows build I'll approve it. But please wait for another > review, preferably from @theRealAph. OK, Thank you very much! > Note that JDK-8269559 (#5129) is also adding a JMH benchmark for this > intrinsic: it would be good if we could merge them, either now or later. The JMH benchmark added by JDK-8269559 (#5129) can cover our test items (compareToLL and compareToUU), and can show the improvement of our patch, so we decided to delete our JMH benchmark in the next commit. The test results using that JMH benchmark in JDK-8269559 are as follows: Raspberry Pi 4B base: Benchmark (delta) (size) Mode Cnt Score Error Units StringCompareToDifferentLength.compareToLL2 24 avgt3 2.310 ? 0.050 ms/op StringCompareToDifferentLength.compareToLL2 36 avgt3 2.818 ? 0.185 ms/op StringCompareToDifferentLength.compareToLL2 72 avgt3 3.151 ? 0.215 ms/op StringCompareToDifferentLength.compareToLL2 128 avgt3 4.171 ? 1.320 ms/op StringCompareToDifferentLength.compareToLL2 256 avgt3 6.169 ? 0.653 ms/op StringCompareToDifferentLength.compareToLL2 512 avgt3 10.911 ? 0.175 ms/op StringCompareToDifferentLength.compareToLU2 24 avgt3 3.312 ? 0.102 ms/op StringCompareToDifferentLength.compareToLU2 36 avgt3 4.162 ? 0.032 ms/op StringCompareToDifferentLength.compareToLU2 72 avgt3 5.705 ? 0.152 ms/op StringCompareToDifferentLength.compareToLU2 128 avgt3 9.301 ? 0.749 ms/op StringCompareToDifferentLength.compareToLU2 256 avgt3 16.507 ? 1.353 ms/op StringCompareToDifferentLength.compareToLU2 512 avgt3 30.160 ? 0.377 ms/op StringCompareToDifferentLength.compareToUL2 24 avgt3 3.366 ? 0.280 ms/op StringCompareToDifferentLength.compareToUL2 36 avgt3 4.308 ? 0.037 ms/op StringCompareToDifferentLength.compareToUL2 72 avgt3 5.674 ? 0.210 ms/op StringCompareToDifferentLength.compareToUL2 128 avgt3 9.358 ? 0.158 ms/op StringCompareToDifferentLength.compareToUL2 256 avgt3 16.165 ? 0.158 ms/op StringCompareToDifferentLength.compareToUL2 512 avgt3 29.857 ? 0.277 ms/op StringCompareToDifferentLength.compareToUU2 24 avgt3 3.149 ? 0.209 ms/op StringCompareToDifferentLength.compareToUU2 36 avgt3 3.157 ? 0.102 ms/op StringCompareToDifferentLength.compareToUU2 72 avgt3 4.415 ? 0.073 ms/op StringCompareToDifferentLength.compareToUU2 128 avgt3 6.244 ? 0.224 ms/op StringCompareToDifferentLength.compareToUU2 256 avgt3 11.032 ? 0.080 ms/op StringCompareToDifferentLength.compareToUU2 512 avgt3 20.942 ? 3.973 ms/op opt： Benchmark (delta) (size) Mode Cnt Score Error Units StringCompareToDifferentLength.compareToLL2 24 avgt3 2.319 ? 0.121 ms/op StringCompareToDifferentLength.compareToLL2 36 avgt3 2.820 ? 0.096 ms/op StringCompareToDifferentLength.compareToLL2 72 avgt3 2.511 ? 0.024 ms/op StringCompareToDifferentLength.compareToLL2 128 avgt3 3.496 ? 0.382 ms/op StringCompareToDifferentLength.compareToLL2 256 avgt3 5.215 ? 0.210 ms/op StringCompareToDifferentLength.compareToLL2 512 avgt3 7.772 ? 0.448 ms/op StringCompareToDifferentLength.compareToLU2 24 avgt3 3.432 ? 0.249 ms/op StringCompareToDifferentLength.compareToLU2 36 avgt3 4.156 ? 0.052 ms/op StringCompareToDifferentLength.compareToLU2 72 avgt3 5.735 ? 0.043 ms/op StringCompareToDifferentLength.compareToLU2 128 avgt3 9.215 ? 0.394 ms/op StringCompareToDifferentLength.compareToLU2 256 avgt3 16.373 ? 0.515 ms/op StringCompareToDifferentLength.compareToLU2 512 avgt3 29.906 ? 0.245 ms/op StringCompareToDifferentLength.compareToUL2 24 avgt3 3.361 ? 0.116 ms/op StringCompareToDifferentLength.compareToUL2 36 avgt3 4.253 ? 0.061 ms/op StringCompareToDifferentLength.compareToUL2 72 avgt3 5.744 ? 0.082 ms/op StringCompareToDifferentLength.compareToUL2 128 avgt3 9.167 ? 0.343 ms/op StringCompareToDifferentLength.compareToUL2 256 avgt3 16.591 ? 0.999 ms/op
Re: RFR: 8272473: Parsing epoch seconds at a DST transition with a non-UTC parser is wrong
On Mon, 23 Aug 2021 16:42:03 GMT, Naoto Sato wrote: > Please review the fix to the subject issue. When instant seconds and zone > co-exist in parsed data, instant seconds was not resolved correctly from them. LGTM - Marked as reviewed by scolebourne (Author). PR: https://git.openjdk.java.net/jdk/pull/5225
Re: RFR: 8273000: Remove WeakReference-based class initialisation barrier implementation
On Thu, 26 Aug 2021 02:48:38 GMT, David Holmes wrote: > I'm unclear exactly what that statement is meant to indicate. `DirectMethodHandle::checkInitialized()` is dual-purpose: it implements class initialization barrier and reports whether the class is fully initialized, so the barrier can be safely elided. The call to `Unsafe::ensureClassInitialized()` blocks until initialization is over when the current thread is not the initializing one. But when call to `Unsafe::ensureClassInitialized()` finished, there are 2 cases possible: * the class is fully initialized; * the class is being initialized and the current thread is the initializing one. In the former case, it's safe to remove the barrier while in the latter the barrier is still required. Original implementation implemented that in an explicit manner by using `WeakReference`s to record the initializing thread. But a pair of `Unsafe::ensureClassInitialized()` and `Unsafe::shouldBeInitialized()` calls provides equivalent functionality and is much simpler at the same time. - PR: https://git.openjdk.java.net/jdk/pull/5258
Re: RFR: 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform [v2]
On Wed, 4 Aug 2021 07:25:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8269373 bug fixes? >> >> These tests call java.lang.ProcessBuilder in direct, so not used jtreg >> command option. To run non-localized tests, -Duser.language=en and >> -Duser.country=US options should be added in ProcessBuilder. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > 8269373: use test opts for process arguments I think the current tests force the US locale, and false positives are test failures in non-US locale environments. This fix does not change the test results in the US locale, but allows it to work in non-US locale environments. I can't think of a false positive problem with this fix, but what specific cases do you think are the problems? - PR: https://git.openjdk.java.net/jdk/pull/4594
Integrated: 8272836: Limit run time for java/lang/invoke/LFCaching tests
On Mon, 23 Aug 2021 11:33:35 GMT, Aleksey Shipilev wrote: > See the RFE for discussion. > > Current PR improves the test time like this: > > > $ make run-test TEST=java/lang/invoke/LFCaching/ > > # Before > real 3m51.608s > user 5m21.612s > sys 0m5.391s > > # After > real 1m13.606s > user 2m26.827s > sys 0m4.761s This pull request has now been integrated. Changeset: a3308af0 Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/a3308af0605bf936d9a9fb7093787a315ccc1e2a Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8272836: Limit run time for java/lang/invoke/LFCaching tests Reviewed-by: redestad, iignatyev - PR: https://git.openjdk.java.net/jdk/pull/5214
Re: RFR: 8272836: Limit run time for java/lang/invoke/LFCaching tests [v2]
On Wed, 25 Aug 2021 16:31:50 GMT, Aleksey Shipilev wrote: >> See the RFE for discussion. >> >> Current PR improves the test time like this: >> >> >> $ make run-test TEST=java/lang/invoke/LFCaching/ >> >> # Before >> real 3m51.608s >> user 5m21.612s >> sys 0m5.391s >> >> # After >> real 1m13.606s >> user 2m26.827s >> sys 0m4.761s > > Aleksey Shipilev has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Make the timeout depend on global timeout with lower multiplier > - Merge branch 'master' into JDK-8272836-perf-test-lfcaching > - 8272836: Optimize run time for java/lang/invoke/LFCaching tests Thanks! - PR: https://git.openjdk.java.net/jdk/pull/5214
Re: RFR: 8272600: (test) Use native "sleep" in Basic.java [v2]
On Wed, 25 Aug 2021 21:45:57 GMT, Roger Riggs wrote: >> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified >> unexpected messages from a child Java VM >> as the cause of the test failure. Attempts to control the output of the >> child VM have failed, the VM is unrepentant . >> >> There is no functionality in the child except to wait long enough for the >> test to finish and the child is destroyed. >> The fix is to switch from using a Java child to using a native child; a new >> executable `sleepmillis`. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Revised to use a native program for sleep LGTM - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5239
