Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Wed, 30 Mar 2022 00:12:59 GMT, Stuart Marks wrote: >> XenoAmess has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - update jmh >> - refine javadoc; refine implement when expectedSize < 0 > > OK, finally got some time to look at this. Here's a rewrite of the spec > words, at least for HashMap::newHashMap. If this settles down, I'll write the > CSR for this and LHM and WHM. > > /** > * Creates a new, empty HashMap suitable for the expected number of > mappings. > * The returned map uses the default load factor of 0.75, and its initial > capacity is > * generally large enough so that the expected number of mappings can be > added > * without resizing the map. > * > * @param numMappings the expected number of mappings > * @param the type of keys maintained by this map > * @param the type of mapped values > * @return the newly created map > * @throws IllegalArgumentException if numMappings is negative > * @since 19 > */ > > The original wording was taken from CHM, which generally is a reasonable > thing to do. Unfortunately, it's wrong. :-) "Table size" isn't accurate; > HashMap uses "capacity" as the term for the number of buckets (== length of > the internal table array). "Size" means "number of mappings" so its use of > "table size" confuses these two concepts. The CHM wording also uses > "elements" which applies to linear collections; the things inside a map are > usually referred to as "mappings" or "entries". (I guess we should fix up CHM > at some point too.) > > While "expectedSize" isn't inaccurate, it's not tied to the main text, so > I've renamed it to numMappings. > > There are a couple other javadoc style rules operating here. The first > sentence is generally a sentence fragment that is short and descriptive, as > it will be pulled into a summary table. (It's often written as if it were a > sentence that begins "This method..." but those words are elided.) Details > are in the rest of the first paragraph. The text for `@param`, `@return`, and > `@throws` are generally also sentence fragments, and they have no initial > capital letter nor trailing period. > > -- > > On performance and benchmarking: this is a distraction from the main point of > this effort, which is to add an API that allows callers a correct and > convenient way to create a properly-sized HashMap. Any work spent on > optimizing performance is almost certainly wasted. > > First, the benchmark: it's entirely unclear what this is measuring. It's > performing the operation 2^31 times, but it sends the result to a black hole > so that the JIT doesn't eliminate the computation. One of the actual results > is 0.170 ops/sec. This includes both the operation and the black hole, so we > don't actually have any idea what that result represents. _Maybe_ it's > possible to infer some idea of relative performance of the different > operations by comparing the results. It's certainly plausible that the > integer code is faster than the float or double code. But the benchmark > doesn't tell us how long the actual computation takes. > > Second, how significant is the cost of the computation? I'll assert that it's > insignificant. The table length is computed once at HashMap creation time, > and it's amortized over the addition of all the initial entries and > subsequent queries and updates to the HashMap. Any of the computations > (whether integer or floating point) are a handful of nanoseconds. This will > be swamped by the first hashCode computation that causes a cache miss. > > Third: I'll stipulate that the integer computation is probably a few ns > faster than the floating-point computation. But the computation is entirely > non-obvious. To make up for it being non-obvious, there's a big comment that > explains it all. It's not worth doing something that increases performance by > an insignificant amount that also requires a big explanation. > > Finally, note that most of the callers are already doing a floating-point > computation to compute the desired capacity, and it doesn't seem to be a > problem. > > Sorry, you probably spent a bunch of time on this already, but trying to > optimize the performance here just isn't worthwhile. Let's please just stick > with our good old `(int) Math.ceil(numMappings / 0.75)`. > > -- > > There should be regression tests added for the three new methods. I haven't > thought much about this. It might be possible to reuse some of the > infrastructure in the WhiteBoxResizeTest we worked on previously. > > -- > > I think it would be good to include updates to some of the use sites in this > PR. It's often useful to put new APIs into practice. One usually learns > something from the effort. Even though this is a really simple API, looking > at use sites can illuminating, to see how the code reads. This
Re: RFR: 8186958: Need method to create pre-sized HashMap [v8]
> 8186958: Need method to create pre-sized HashMap XenoAmess has updated the pull request incrementally with one additional commit since the last revision: update codes - Changes: - all: https://git.openjdk.java.net/jdk/pull/7928/files - new: https://git.openjdk.java.net/jdk/pull/7928/files/293b949c..fe0f31c8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7928=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7928=06-07 Stats: 61 lines in 3 files changed: 4 ins; 24 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/7928.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7928/head:pull/7928 PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8283726: x86 intrinsics for compare method in Integer and Long [v2]
> Implements x86 intrinsics for compare() method in java.lang.Integer and > java.lang.Long. Vamsi Parasa 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 four additional commits since the last revision: - refactored x86_64.ad code to macro assembly routines - Merge branch 'master' of https://git.openjdk.java.net/jdk into cmp - add JMH benchmarks - 8283726: x86 intrinsics for compare method in Integer and Long - Changes: - all: https://git.openjdk.java.net/jdk/pull/7975/files - new: https://git.openjdk.java.net/jdk/pull/7975/files/b0c3314d..79e4aa50 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7975=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7975=00-01 Stats: 5452 lines in 218 files changed: 3925 ins; 856 del; 671 mod Patch: https://git.openjdk.java.net/jdk/pull/7975.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7975/head:pull/7975 PR: https://git.openjdk.java.net/jdk/pull/7975
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v16]
On Wed, 30 Mar 2022 21:51:16 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > Maurizio Cimadamore has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 31 commits: > > - Merge branch 'master' into foreign-preview > - Tweak FunctionDescriptor::argumentLayouts to return an immutable list > - Fix bad usage of `@link` with primitive array types > - Switch to daemon threads for async upcalls > - Use thread local storage to optimize attach of async threads > - Drop support for Constable from MemoryLayout/FunctionDescriptor > - Merge branch 'master' into foreign-preview > - Revert changes to RunTests.gmk > - Add --enable-preview to micro benchmark java options > - Address more review comments > - ... and 21 more: > https://git.openjdk.java.net/jdk/compare/ce27d9dd...247e5eb5 Java code looks good (i did not go through the tests). As is common no comments, since code was reviewed in smaller steps in the panama-foreign respository. - Marked as reviewed by psandoz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v15]
On Wed, 30 Mar 2022 20:59:34 GMT, Maurizio Cimadamore
wrote:
>> This PR contains the API and implementation changes for JEP-424 [1]. A more
>> detailed description of such changes, to avoid repetitions during the review
>> process, is included as a separate comment.
>>
>> [1] - https://openjdk.java.net/jeps/424
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Tweak FunctionDescriptor::argumentLayouts to return an immutable list
src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java line 73:
> 71: */
> 72: public List argumentLayouts() {
> 73: return Collections.unmodifiableList(argLayouts);
This change doesn’t seem to be necessary, as `FunctionDescriptor` is already
created with `List.of(…)` (or `Stream.toList()` in the case of
`FunctionDescriptor.VariadicFunction`), which produce immutable lists (although
`Stream.toList()` permits `null`s, which
`Stream.collect(Collectors.toImmutableList())` and `List.of(…)` doesn’t).
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8283996: Reduce cost of year and month calculations
On Wed, 30 Mar 2022 12:06:39 GMT, Claes Redestad wrote: > A few integer divisions and multiplications can be replaced with test + > addition, leading to a decent speed-up on java.time microbenchmarks such as > `GetYearBench`. Numbers from my local x86 workstation, seeing similar > speed-up on aarch64 and other x86 setups. > > Baseline: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 18.492 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.121 ± > 0.135 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 18.936 ± > 0.012 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 9.283 ± > 0.222 ops/ms > > Patched: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 20.931 ± > 0.013 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.858 ± > 0.167 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 20.923 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 10.028 ± > 0.182 ops/ms > > > Testing: java.time tests locally, CI tier1+2 ongoing. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8039
Re: RFR: JDK-8277095 : Empty streams create too many objects [v2]
On Tue, 16 Nov 2021 20:53:26 GMT, kabutz wrote: >> This is a draft proposal for how we could improve stream performance for the >> case where the streams are empty. Empty collections are common-place. If we >> iterate over them with an Iterator, we would have to create one small >> Iterator object (which could often be eliminated) and if it is empty we are >> done. However, with Streams we first have to build up the entire pipeline, >> until we realize that there is no work to do. With this example, we change >> Collection#stream() to first check if the collection is empty, and if it is, >> we simply return an EmptyStream. We also have EmptyIntStream, >> EmptyLongStream and EmptyDoubleStream. We have taken great care for these to >> have the same characteristics and behaviour as the streams returned by >> Stream.empty(), IntStream.empty(), etc. >> >> Some of the JDK tests fail with this, due to ClassCastExceptions (our >> EmptyStream is not an AbstractPipeline) and AssertionError, since we can >> call some methods repeatedly on the stream without it failing. On the plus >> side, creating a complex stream on an empty stream gives us upwards of 50x >> increase in performance due to a much smaller object allocation rate. This >> PR includes the code for the change, unit tests and also a JMH benchmark to >> demonstrate the improvement. > > kabutz has updated the pull request incrementally with one additional commit > since the last revision: > > Refactored empty stream implementations to reduce duplicate code and > improved unordered() > Added StreamSupport.empty for primitive spliterators and use that in > Arrays.stream() satisfy the bot with a comment - we are on day 20 now :) @kabutz i hope that was ok? - PR: https://git.openjdk.java.net/jdk/pull/6275
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v16]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 31 commits: - Merge branch 'master' into foreign-preview - Tweak FunctionDescriptor::argumentLayouts to return an immutable list - Fix bad usage of `@link` with primitive array types - Switch to daemon threads for async upcalls - Use thread local storage to optimize attach of async threads - Drop support for Constable from MemoryLayout/FunctionDescriptor - Merge branch 'master' into foreign-preview - Revert changes to RunTests.gmk - Add --enable-preview to micro benchmark java options - Address more review comments - ... and 21 more: https://git.openjdk.java.net/jdk/compare/ce27d9dd...247e5eb5 - Changes: https://git.openjdk.java.net/jdk/pull/7888/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7888=15 Stats: 64862 lines in 366 files changed: 43028 ins; 19321 del; 2513 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Integrated: 8283801: Cleanup confusing String.toString calls
On Sun, 20 Mar 2022 13:20:31 GMT, Andrey Turbanov wrote: > String.toString() calls doesn't make much sense. Only one place, where it > could be used - to generate NPE. But in a few places of JDK codebase it's > called, even when NPE will happen anyway. > I propose to cleanup such places. > Found by IntelliJ IDEA inspection `Redundant 'String' operation`. This pull request has now been integrated. Changeset: b8dd21b7 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/b8dd21b790f36450de9aa0acc56251715b1a33e9 Stats: 14 lines in 4 files changed: 0 ins; 4 del; 10 mod 8283801: Cleanup confusing String.toString calls Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/7878
Re: RFR: 8283801: Cleanup confusing String.toString calls
On Sun, 20 Mar 2022 13:20:31 GMT, Andrey Turbanov wrote: > String.toString() calls doesn't make much sense. Only one place, where it > could be used - to generate NPE. But in a few places of JDK codebase it's > called, even when NPE will happen anyway. > I propose to cleanup such places. > Found by IntelliJ IDEA inspection `Redundant 'String' operation`. Thank you for review! - PR: https://git.openjdk.java.net/jdk/pull/7878
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v15]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Tweak FunctionDescriptor::argumentLayouts to return an immutable list - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/0bcc8664..af41a76c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=13-14 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8283340: Remove support for -Dsun.misc.URLClassPath.disableJarChecking
On Fri, 18 Mar 2022 18:59:16 GMT, Sean Mullan wrote: >> I don't know what the motivation for this change is but there is more to >> this story and I think will require agreement from the security area before >> removing it. > > I agree with @AlanBateman. Please don't proceed with this fix until further > notice. @seanjmullan are there any updates here? Or is the suggestion to drop the PR, because the code is definitely still needed? Thanks! - PR: https://git.openjdk.java.net/jdk/pull/7861
Re: RFR: 8283996: Reduce cost of year and month calculations
On Wed, 30 Mar 2022 12:06:39 GMT, Claes Redestad wrote: > A few integer divisions and multiplications can be replaced with test + > addition, leading to a decent speed-up on java.time microbenchmarks such as > `GetYearBench`. Numbers from my local x86 workstation, seeing similar > speed-up on aarch64 and other x86 setups. > > Baseline: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 18.492 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.121 ± > 0.135 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 18.936 ± > 0.012 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 9.283 ± > 0.222 ops/ms > > Patched: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 20.931 ± > 0.013 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.858 ± > 0.167 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 20.923 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 10.028 ± > 0.182 ops/ms > > > Testing: java.time tests locally, CI tier1+2 ongoing. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8039
Re: RFR: 8283842: TestZoneTextPrinterParser.test_roundTripAtOverlap fails: DateTimeParseException
On Wed, 30 Mar 2022 16:46:40 GMT, Naoto Sato wrote: > Fixes test failures caused by depending on the default locale. Specifying > explicit `Locale.US` will do. Marked as reviewed by scolebourne (Author). - PR: https://git.openjdk.java.net/jdk/pull/8045
Re: RFR: 8283996: Reduce cost of year and month calculations
On Wed, 30 Mar 2022 12:06:39 GMT, Claes Redestad wrote: > A few integer divisions and multiplications can be replaced with test + > addition, leading to a decent speed-up on java.time microbenchmarks such as > `GetYearBench`. Numbers from my local x86 workstation, seeing similar > speed-up on aarch64 and other x86 setups. > > Baseline: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 18.492 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.121 ± > 0.135 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 18.936 ± > 0.012 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 9.283 ± > 0.222 ops/ms > > Patched: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 20.931 ± > 0.013 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.858 ± > 0.167 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 20.923 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 10.028 ± > 0.182 ops/ms > > > Testing: java.time tests locally, CI tier1+2 ongoing. I'm happy, based on the assertion that it produces the same result and is faster. - Marked as reviewed by scolebourne (Author). PR: https://git.openjdk.java.net/jdk/pull/8039
Re: RFR: 8283994: Make Xerces DatatypeException stackless
On Wed, 30 Mar 2022 11:38:59 GMT, Aleksey Shipilev wrote: > See bug report for more details. This change improves > SPECjvm2008:xml.validation for about +3%: > > > baseline: 298.353 ± 1.008 ops/min > patched: 309.912 ± 1.347 ops/min > > Of course, the real improvements might be even higher, as exception might be > thrown from much deeper call hierarchy. > > Additional testing: > - [x] Linux x86_64 fastdebug, `jaxp_all` > - [x] Linux x86_64 fastdebug, `javax/xml` Marked as reviewed by joehw (Reviewer). src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/dv/DatatypeException.java line 2: > 1: /* > 2: * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights > reserved. Update the LastModified tag below as well. Otherwise, looks good. - PR: https://git.openjdk.java.net/jdk/pull/8036
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v14]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix bad usage of `@link` with primitive array types - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/43dc6be3..0bcc8664 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=12-13 Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8283842: TestZoneTextPrinterParser.test_roundTripAtOverlap fails: DateTimeParseException
On Wed, 30 Mar 2022 16:46:40 GMT, Naoto Sato wrote: > Fixes test failures caused by depending on the default locale. Specifying > explicit `Locale.US` will do. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8045
Integrated: 8283470: Update java.lang.invoke.VarHandle to use sealed classes
On Wed, 23 Mar 2022 16:27:29 GMT, Mandy Chung wrote: > This patch changes VarHandle and its implementation hierarchy to use sealed > classes. All VarHandle permitted classes are package-private and either > final or sealed abstract classes. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8283540 This pull request has now been integrated. Changeset: e61ccfba Author:Mandy Chung URL: https://git.openjdk.java.net/jdk/commit/e61ccfba7fa747c24e34a7539871a34630693af5 Stats: 58 lines in 5 files changed: 42 ins; 5 del; 11 mod 8283470: Update java.lang.invoke.VarHandle to use sealed classes Reviewed-by: darcy, psandoz - PR: https://git.openjdk.java.net/jdk/pull/7926
Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library
On Wed, 30 Mar 2022 13:21:41 GMT, Jaikiran Pai wrote:
>> A small improvement to `RawNativeLibraries`. `RawNativeLibraries::load`
>> returns the same `NativeLibrary` instance of a given path if it's called
>> multiple times. This means that multiple clients have to coordinate that the
>> last one using the loaded library is the one to close the library by calling
>> `RawNativeLibraries::unload`; otherwise, an exception may be thrown.
>>
>> This patch changes `RawNativeLibraries::load` to delegate to the underlying
>> platform-specific library loading mechanism e.g. dlopen/dlclose that keeps
>> the reference count. So each call to `RawNativeLibraries::load` and
>> `unload` is simply a call to dlopen and dlclose respectively. Each client
>> of calling `RawNativeLibraries::load` is responsible for calling
>> `RawNativeLibraries::unload`. This will be consistent with the current
>> behavior when you call `load` and `unload` with a different
>> `RawNativeLibraries` instance.
>
> src/java.base/share/classes/jdk/internal/loader/RawNativeLibraries.java line
> 49:
>
>> 47: public final class RawNativeLibraries {
>> 48: final Set libraries =
>> ConcurrentHashMap.newKeySet();
>> 49: final Class caller;
>
> Hello Mandy,
> Apart from the `caller` being used for checks while constructing a
> `RawNativeLibraries` in `RawNativeLibraries.newInstance(MethodHandles.Lookup
> trustedCaller)`, I don't see this instance member being used anywhere. Do you
> think we need to store this as an instance member?
I keep it as a record and for debugging use in case.
-
PR: https://git.openjdk.java.net/jdk/pull/8022
Re: RFR: 8283842: TestZoneTextPrinterParser.test_roundTripAtOverlap fails: DateTimeParseException
On Wed, 30 Mar 2022 16:46:40 GMT, Naoto Sato wrote: > Fixes test failures caused by depending on the default locale. Specifying > explicit `Locale.US` will do. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8045
Re: RFR: 8283996: Reduce cost of year and month calculations
On Wed, 30 Mar 2022 16:08:15 GMT, Brian Burkhalter wrote: > Looks all right assuming tests pass. Thanks! Tier1+2 testing passed. - PR: https://git.openjdk.java.net/jdk/pull/8039
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostly changes >> file headers etc and another one, smaller, that make changes to the code to >> adapt it to our needs, JDK version etc. In this second commit one can see >> that two classes that were removed by the automatically generated change >> have been copied back: >> >> jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter >> jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter >> >> This is because package: `jdk.jfr.internal.instrument` needs them. >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > fixing files missing new line at the end JFR parts look OK. - Marked as reviewed by egahlin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8000
RFR: 8283842: TestZoneTextPrinterParser.test_roundTripAtOverlap fails: DateTimeParseException
Fixes test failures caused by depending on the default locale. Specifying explicit `Locale.US` will do. - Commit messages: - 8283842: TestZoneTextPrinterParser.test_roundTripAtOverlap fails: DateTimeParseException Changes: https://git.openjdk.java.net/jdk/pull/8045/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8045=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283842 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8045.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8045/head:pull/8045 PR: https://git.openjdk.java.net/jdk/pull/8045
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostly changes >> file headers etc and another one, smaller, that make changes to the code to >> adapt it to our needs, JDK version etc. In this second commit one can see >> that two classes that were removed by the automatically generated change >> have been copied back: >> >> jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter >> jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter >> >> This is because package: `jdk.jfr.internal.instrument` needs them. >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > fixing files missing new line at the end The changes look reasonable to me - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostly changes >> file headers etc and another one, smaller, that make changes to the code to >> adapt it to our needs, JDK version etc. In this second commit one can see >> that two classes that were removed by the automatically generated change >> have been copied back: >> >> jdk.internal.org.objectweb.asm.commons.RemappingMethodAdapter >> jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter >> >> This is because package: `jdk.jfr.internal.instrument` needs them. >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > fixing files missing new line at the end anymore comments on this one? Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8000
Re: RFR: 5087440: java.io bulk read(...) end-of-stream return value descriptions ambiguous [v2]
On Wed, 30 Mar 2022 16:28:16 GMT, Brian Burkhalter wrote: >> Minimal version of possible fixes: make the bulk read `@return` verbiage >> consistent in the `java.io` package. > > Brian Burkhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. The pull request > contains one new commit since the last revision: > > 5087440: java.io bulk read(...) end-of-stream return value descriptions > ambiguous Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8044
Re: RFR: 5087440: java.io bulk read(...) end-of-stream return value descriptions ambiguous [v2]
> Minimal version of possible fixes: make the bulk read `@return` verbiage > consistent in the `java.io` package. Brian Burkhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 5087440: java.io bulk read(...) end-of-stream return value descriptions ambiguous - Changes: - all: https://git.openjdk.java.net/jdk/pull/8044/files - new: https://git.openjdk.java.net/jdk/pull/8044/files/360a5a9c..3de7caa8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8044=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8044=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8044.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8044/head:pull/8044 PR: https://git.openjdk.java.net/jdk/pull/8044
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature
On Wed, 30 Mar 2022 10:31:59 GMT, Xiaohong Gong wrote:
> Currently the vector load with mask when the given index happens out of the
> array boundary is implemented with pure java scalar code to avoid the IOOBE
> (IndexOutOfBoundaryException). This is necessary for architectures that do
> not support the predicate feature. Because the masked load is implemented
> with a full vector load and a vector blend applied on it. And a full vector
> load will definitely cause the IOOBE which is not valid. However, for
> architectures that support the predicate feature like SVE/AVX-512/RVV, it can
> be vectorized with the predicated load instruction as long as the indexes of
> the masked lanes are within the bounds of the array. For these architectures,
> loading with unmasked lanes does not raise exception.
>
> This patch adds the vectorization support for the masked load with IOOBE
> part. Please see the original java implementation (FIXME: optimize):
>
>
> @ForceInline
> public static
> ByteVector fromArray(VectorSpecies species,
>byte[] a, int offset,
>VectorMask m) {
> ByteSpecies vsp = (ByteSpecies) species;
> if (offset >= 0 && offset <= (a.length - species.length())) {
> return vsp.dummyVector().fromArray0(a, offset, m);
> }
>
> // FIXME: optimize
> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
> return vsp.vOp(m, i -> a[offset + i]);
> }
>
> Since it can only be vectorized with the predicate load, the hotspot must
> check whether the current backend supports it and falls back to the java
> scalar version if not. This is different from the normal masked vector load
> that the compiler will generate a full vector load and a vector blend if the
> predicate load is not supported. So to let the compiler make the expected
> action, an additional flag (i.e. `usePred`) is added to the existing
> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
> "false" for the normal load. And the compiler will fail to intrinsify if the
> flag is "true" and the predicate load is not supported by the backend, which
> means that normal java path will be executed.
>
> Also adds the same vectorization support for masked:
> - fromByteArray/fromByteBuffer
> - fromBooleanArray
> - fromCharArray
>
> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
> on the x86 AVX-512 system:
>
> Benchmark before After Units
> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>
> Similar performance gain can also be observed on 512-bit SVE system.
src/hotspot/share/opto/vectorIntrinsics.cpp line 1234:
> 1232: bool use_predicate = false;
> 1233: if (is_store) {
> 1234: // Masked vector store always uses the predicated store.
Can masked store be implemented as Load + Blend + Store?
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 5087440: (ch spec) java.io, nio bulk read(...) end-of-stream return value descriptions ambiguous
On Wed, 30 Mar 2022 16:05:03 GMT, Brian Burkhalter wrote: > Minimal version of possible fixes: make the bulk read `@return` verbiage > consistent in the `java.io` package. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8044
Re: RFR: 8283996: Reduce cost of year and month calculations
On Wed, 30 Mar 2022 12:06:39 GMT, Claes Redestad wrote: > A few integer divisions and multiplications can be replaced with test + > addition, leading to a decent speed-up on java.time microbenchmarks such as > `GetYearBench`. Numbers from my local x86 workstation, seeing similar > speed-up on aarch64 and other x86 setups. > > Baseline: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 18.492 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.121 ± > 0.135 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 18.936 ± > 0.012 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 9.283 ± > 0.222 ops/ms > > Patched: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 20.931 ± > 0.013 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.858 ± > 0.167 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 20.923 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 10.028 ± > 0.182 ops/ms > > > Testing: java.time tests locally, CI tier1+2 ongoing. Looks all right assuming tests pass. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8039
RFR: 5087440: (ch spec) java.io,nio bulk read(...) end-of-stream return value descriptions ambiguous
Minimal version of possible fixes: make the bulk read `@return` verbiage consistent in the `java.io` package. - Commit messages: - 5087440: (ch spec) java.io,nio bulk read(...) end-of-stream return value descriptions ambiguous Changes: https://git.openjdk.java.net/jdk/pull/8044/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8044=00 Issue: https://bugs.openjdk.java.net/browse/JDK-5087440 Stats: 14 lines in 3 files changed: 5 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/8044.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8044/head:pull/8044 PR: https://git.openjdk.java.net/jdk/pull/8044
Integrated: 8283715: Update ObjectStreamClass to be final
On Tue, 29 Mar 2022 01:07:33 GMT, Stuart Marks wrote: > Pretty much just what it says. This pull request has now been integrated. Changeset: ae57258b Author:Stuart Marks URL: https://git.openjdk.java.net/jdk/commit/ae57258b46b8b6953148cd8cf71faf75eef118da Stats: 10 lines in 2 files changed: 2 ins; 1 del; 7 mod 8283715: Update ObjectStreamClass to be final Reviewed-by: darcy, jpai, mchung, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/8009
Re: RFR: 8283805: [REDO] JDK 19 L10n resource files update - msgdrop 10
On Wed, 30 Mar 2022 00:43:49 GMT, Alisen Chung wrote: > redo of 8280400 I believe `src/jdk.localedata/share/classes/sun/util/resources/ext/CurrencyNames_zh_CN.properties` and `test/jdk/sun/text/resources/LocaleData` have to be adjusted according to the l10n changes. - PR: https://git.openjdk.java.net/jdk/pull/8026
Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library
On Tue, 29 Mar 2022 20:05:50 GMT, Mandy Chung wrote:
> A small improvement to `RawNativeLibraries`. `RawNativeLibraries::load`
> returns the same `NativeLibrary` instance of a given path if it's called
> multiple times. This means that multiple clients have to coordinate that the
> last one using the loaded library is the one to close the library by calling
> `RawNativeLibraries::unload`; otherwise, an exception may be thrown.
>
> This patch changes `RawNativeLibraries::load` to delegate to the underlying
> platform-specific library loading mechanism e.g. dlopen/dlclose that keeps
> the reference count. So each call to `RawNativeLibraries::load` and `unload`
> is simply a call to dlopen and dlclose respectively. Each client of calling
> `RawNativeLibraries::load` is responsible for calling
> `RawNativeLibraries::unload`. This will be consistent with the current
> behavior when you call `load` and `unload` with a different
> `RawNativeLibraries` instance.
src/java.base/share/classes/jdk/internal/loader/RawNativeLibraries.java line 49:
> 47: public final class RawNativeLibraries {
> 48: final Set libraries =
> ConcurrentHashMap.newKeySet();
> 49: final Class caller;
Hello Mandy,
Apart from the `caller` being used for checks while constructing a
`RawNativeLibraries` in `RawNativeLibraries.newInstance(MethodHandles.Lookup
trustedCaller)`, I don't see this instance member being used anywhere. Do you
think we need to store this as an instance member?
-
PR: https://git.openjdk.java.net/jdk/pull/8022
Re: RFR: 8253569: javax.xml.catalog.Catalog.matchURI() implementation should reset state variables [v2]
On Wed, 30 Mar 2022 00:46:49 GMT, Joe Wang wrote: >> Resets state of a Catalog instance on each matching call so that it can be >> reused. Adjusts CatalogResolver's resolve routine so that it continues to >> observes the PREFER feature in a resolution process. Without the adjustment, >> PreferFeatureTest would fail. >> >> Mach5 XML tests passed: >> https://mach5.us.oracle.com/mdash/jobs/huizwang-open-20220329-0148-30563542 > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > remove variable, check instance instead. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8018
RFR: 8283996: Reduce cost of year and month calculations
A few integer divisions and multiplications can be replaced with test + addition, leading to a decent speed-up on java.time microbenchmarks such as `GetYearBench`. Numbers from my local x86 workstation, seeing similar speed-up on aarch64 and other x86 setups. Baseline: BenchmarkMode Cnt Score Error Units GetYearBench.getYearFromMillisZoneOffsetthrpt 15 18.492 ± 0.017 ops/ms GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.121 ± 0.135 ops/ms GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 18.936 ± 0.012 ops/ms GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 9.283 ± 0.222 ops/ms Patched: BenchmarkMode Cnt Score Error Units GetYearBench.getYearFromMillisZoneOffsetthrpt 15 20.931 ± 0.013 ops/ms GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.858 ± 0.167 ops/ms GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 20.923 ± 0.017 ops/ms GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 10.028 ± 0.182 ops/ms Testing: java.time tests locally, CI tier1+2 ongoing. - Commit messages: - Reduce cost of year and month calculations Changes: https://git.openjdk.java.net/jdk/pull/8039/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8039=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283996 Stats: 12 lines in 2 files changed: 6 ins; 1 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8039/head:pull/8039 PR: https://git.openjdk.java.net/jdk/pull/8039
Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library
On Tue, 29 Mar 2022 20:05:50 GMT, Mandy Chung wrote: > A small improvement to `RawNativeLibraries`. `RawNativeLibraries::load` > returns the same `NativeLibrary` instance of a given path if it's called > multiple times. This means that multiple clients have to coordinate that the > last one using the loaded library is the one to close the library by calling > `RawNativeLibraries::unload`; otherwise, an exception may be thrown. > > This patch changes `RawNativeLibraries::load` to delegate to the underlying > platform-specific library loading mechanism e.g. dlopen/dlclose that keeps > the reference count. So each call to `RawNativeLibraries::load` and `unload` > is simply a call to dlopen and dlclose respectively. Each client of calling > `RawNativeLibraries::load` is responsible for calling > `RawNativeLibraries::unload`. This will be consistent with the current > behavior when you call `load` and `unload` with a different > `RawNativeLibraries` instance. Marked as reviewed by jvernee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8022
RFR: 8283994: Make Xerces DatatypeException stackless
See bug report for more details. This change improves SPECjvm2008:xml.validation for about +3%: baseline: 298.353 ± 1.008 ops/min patched: 309.912 ± 1.347 ops/min Of course, the real improvements might be even higher, as exception might be thrown from much deeper call hierarchy. Additional testing: - [x] Linux x86_64 fastdebug, `jaxp_all` - [x] Linux x86_64 fastdebug, `javax/xml` - Commit messages: - Fix Changes: https://git.openjdk.java.net/jdk/pull/8036/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8036=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283994 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8036.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8036/head:pull/8036 PR: https://git.openjdk.java.net/jdk/pull/8036
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature
On Wed, 30 Mar 2022 10:31:59 GMT, Xiaohong Gong wrote:
> Currently the vector load with mask when the given index happens out of the
> array boundary is implemented with pure java scalar code to avoid the IOOBE
> (IndexOutOfBoundaryException). This is necessary for architectures that do
> not support the predicate feature. Because the masked load is implemented
> with a full vector load and a vector blend applied on it. And a full vector
> load will definitely cause the IOOBE which is not valid. However, for
> architectures that support the predicate feature like SVE/AVX-512/RVV, it can
> be vectorized with the predicated load instruction as long as the indexes of
> the masked lanes are within the bounds of the array. For these architectures,
> loading with unmasked lanes does not raise exception.
>
> This patch adds the vectorization support for the masked load with IOOBE
> part. Please see the original java implementation (FIXME: optimize):
>
>
> @ForceInline
> public static
> ByteVector fromArray(VectorSpecies species,
>byte[] a, int offset,
>VectorMask m) {
> ByteSpecies vsp = (ByteSpecies) species;
> if (offset >= 0 && offset <= (a.length - species.length())) {
> return vsp.dummyVector().fromArray0(a, offset, m);
> }
>
> // FIXME: optimize
> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
> return vsp.vOp(m, i -> a[offset + i]);
> }
>
> Since it can only be vectorized with the predicate load, the hotspot must
> check whether the current backend supports it and falls back to the java
> scalar version if not. This is different from the normal masked vector load
> that the compiler will generate a full vector load and a vector blend if the
> predicate load is not supported. So to let the compiler make the expected
> action, an additional flag (i.e. `usePred`) is added to the existing
> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
> "false" for the normal load. And the compiler will fail to intrinsify if the
> flag is "true" and the predicate load is not supported by the backend, which
> means that normal java path will be executed.
>
> Also adds the same vectorization support for masked:
> - fromByteArray/fromByteBuffer
> - fromBooleanArray
> - fromCharArray
>
> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
> on the x86 AVX-512 system:
>
> Benchmark before After Units
> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>
> Similar performance gain can also be observed on 512-bit SVE system.
AVX has `vmaskmovpd` and `vmaskmovps` for masked loads and stores, which do not
required predicate vectors. I think the implementation should make it possible
to take advantage of these instructions. Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283805: [REDO] JDK 19 L10n resource files update - msgdrop 10
On Wed, 30 Mar 2022 00:43:49 GMT, Alisen Chung wrote: > redo of 8280400 Since this is identical to the original fix, I would expect the same Tier2 test failure as reported in [JDK-8283804](https://bugs.openjdk.java.net/browse/JDK-8283804). - PR: https://git.openjdk.java.net/jdk/pull/8026
RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature
Currently the vector load with mask when the given index happens out of the
array boundary is implemented with pure java scalar code to avoid the IOOBE
(IndexOutOfBoundaryException). This is necessary for architectures that do not
support the predicate feature. Because the masked load is implemented with a
full vector load and a vector blend applied on it. And a full vector load will
definitely cause the IOOBE which is not valid. However, for architectures that
support the predicate feature like SVE/AVX-512/RVV, it can be vectorized with
the predicated load instruction as long as the indexes of the masked lanes are
within the bounds of the array. For these architectures, loading with unmasked
lanes does not raise exception.
This patch adds the vectorization support for the masked load with IOOBE part.
Please see the original java implementation (FIXME: optimize):
@ForceInline
public static
ByteVector fromArray(VectorSpecies species,
byte[] a, int offset,
VectorMask m) {
ByteSpecies vsp = (ByteSpecies) species;
if (offset >= 0 && offset <= (a.length - species.length())) {
return vsp.dummyVector().fromArray0(a, offset, m);
}
// FIXME: optimize
checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
return vsp.vOp(m, i -> a[offset + i]);
}
Since it can only be vectorized with the predicate load, the hotspot must check
whether the current backend supports it and falls back to the java scalar
version if not. This is different from the normal masked vector load that the
compiler will generate a full vector load and a vector blend if the predicate
load is not supported. So to let the compiler make the expected action, an
additional flag (i.e. `usePred`) is added to the existing "loadMasked"
intrinsic, with the value "true" for the IOOBE part while "false" for the
normal load. And the compiler will fail to intrinsify if the flag is "true" and
the predicate load is not supported by the backend, which means that normal
java path will be executed.
Also adds the same vectorization support for masked:
- fromByteArray/fromByteBuffer
- fromBooleanArray
- fromCharArray
The performance for the new added benchmarks improve about `1.88x ~ 30.26x` on
the x86 AVX-512 system:
Benchmark before After Units
LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
Similar performance gain can also be observed on 512-bit SVE system.
-
Commit messages:
- 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate
feature
Changes: https://git.openjdk.java.net/jdk/pull/8035/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8035=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8283667
Stats: 821 lines in 43 files changed: 314 ins; 117 del; 390 mod
Patch: https://git.openjdk.java.net/jdk/pull/8035.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8035/head:pull/8035
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Tue, 29 Mar 2022 12:45:51 GMT, Volker Simonis wrote:
> > Hello Volker, An additional thing that we might have to consider here is
> > whether adding this javadoc change to `InflaterInputStream` is ever going
> > to "show up" to end user applications. What I mean is, I think in many
> > cases the end user applications won't even know they are dealing with an
> > `InflaterInputStream`. For example, the following code:
> > ```
> > ZipFile zf = ...
> > ZipEntry ze = zf.getEntry("some-file");
> > InputStream is = zf.getInputStream(ze);
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > As we see above, none of these APIs talk about `InflaterInputStream` (the
> > return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
> > users won't be able to view this spec change. Perhaps we should also add
> > some note in the `ZipFile.getInpustream()` API to make a mention of
> > this potential difference in behaviour of the returned stream?
>
> You are right with your observation and I'll be happy to add a corresponding
> comment if @LanceAndersen and @AlanBateman agree. Please let me know what you
> think?
Hi Volker,
I believe Jai raises a valid point given these javadocs probably have had
limited updates if any since the API was originally added.We should look at
ZipInputStream and GZipInputStream as well if we decide to update the
ZipFile::getInputStream(where we could borrow some wording from the
ZipInputStream class description as a start to some word smithing).
As Roger points out we will need a release note for this change as well.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8265768 [aarch64] Use glibc libm impl for dlog,dlog10,dexp iff 2.29 or greater on AArch64.
On Tue, 25 May 2021 15:32:40 GMT, gregcawthorne wrote: >> Glibc 2.29 onwards provides optimised versions of log,log10,exp. >> These functions have an accuracy of 0.9ulp or better in glibc >> 2.29. >> >> Therefore this patch adds code to parse, store and check >> the runtime glibcs version in os_linux.cpp/hpp. >> This is then used to select the glibcs implementation of >> log, log10, exp at runtime for c1 and c2, iff we have >> glibc 2.29 or greater. >> >> This will ensure OpenJDK can benefit from future improvements >> to glibc. >> >> Glibc adheres to the ieee754 standard, unless stated otherwise >> in its spec. >> >> As there are no stated exceptions in the current glibc spec >> for dlog, dlog10 and dexp, we can assume they currently follow >> ieee754 (which testing confirms). As such, future version of >> glibc are unlikely to lose this compliance with ieee754 in >> future. >> >> W.r.t performance this patch sees ~15-30% performance improvements for >> log and log10, with ~50-80% performance improvements for exp for the >> common input ranged (which output real numbers). However for the NaN >> and inf output ranges we see a slow down of up to a factor of 2 for >> some functions and architectures. >> >> Due to this being the uncommon case we assert that this is a >> worthwhile tradeoff. > > greg.cawtho...@arm.com > > Should work @gregcawthorne any plans to re-open and fix this? - PR: https://git.openjdk.java.net/jdk/pull/3510
Integrated: 8283800: Simplify String.indexOf/lastIndexOf calls
On Sun, 20 Mar 2022 12:45:34 GMT, Andrey Turbanov wrote: > In a few places String.indexOf/lastIndexOf methods are called with default > parameter for index: `0` for `indexOf`, length() for `lastIndexOf`. > I propose to cleanup such calls. It makes code a bit easier to read. In case > of `indexOf` it even could be faster, as there is separate intrinsic for > `indexOf` call without index argument. This pull request has now been integrated. Changeset: 9bb916db Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/9bb916db0a6af2c6476c0bfbc55c1bb8721b4354 Stats: 14 lines in 6 files changed: 0 ins; 0 del; 14 mod 8283800: Simplify String.indexOf/lastIndexOf calls Reviewed-by: xuelei, bpb, lmesnik - PR: https://git.openjdk.java.net/jdk/pull/7877
Integrated: 8283846: Remove unused jdk.internal.reflect.SignatureIterator
On Tue, 29 Mar 2022 09:15:01 GMT, Andrey Turbanov wrote: > It was no longer used due to JDK-4479184 long ago. This pull request has now been integrated. Changeset: b323f54f Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/b323f54feef13a47bb02af608eb1f6474692d905 Stats: 81 lines in 1 file changed: 0 ins; 81 del; 0 mod 8283846: Remove unused jdk.internal.reflect.SignatureIterator Reviewed-by: bpb, mchung, iris - PR: https://git.openjdk.java.net/jdk/pull/8013
Re: RFR: 8283715: Update ObjectStreamClass to be final [v2]
On Wed, 30 Mar 2022 00:43:57 GMT, Jaikiran Pai wrote: >> Stuart Marks has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Adjust nonfinal CSMs and rework error reporting in CheckCSMs.java test > > Hello Stuart, the test change looks fine to me. Just a minor note - the > copyright year of the test will need an update. @jaikiran Thanks, I've updated the copyright year. I will integrate when I can keep an eye on the subsequent builds. - PR: https://git.openjdk.java.net/jdk/pull/8009
Re: RFR: 8283715: Update ObjectStreamClass to be final [v3]
On Wed, 30 Mar 2022 06:17:54 GMT, Stuart Marks wrote: >> Pretty much just what it says. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright year. Marked as reviewed by jpai (Committer). - PR: https://git.openjdk.java.net/jdk/pull/8009
Re: RFR: 8283715: Update ObjectStreamClass to be final [v3]
> Pretty much just what it says. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Update copyright year. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8009/files - new: https://git.openjdk.java.net/jdk/pull/8009/files/77b6d79f..7ca24f1d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8009=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8009=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8009.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8009/head:pull/8009 PR: https://git.openjdk.java.net/jdk/pull/8009
