Re: RFR: 8303427: Fixpath confused unix root contains "/jdk"
On Tue, 29 Aug 2023 00:16:46 GMT, Erik Joelsson wrote: > On Windows, when a directory exists in the "unix" root with the same name as > a directory in the "test" dir, fixpath will corrupt test arguments to jtreg > (and possibly other arguments as well). Fixpath sees a string like this: > > test/jdk/foo > > It looks for the first `/` and checks if the first element following that, > until the next `/`, is an existing directory in the unix filesystem root. In > this case, the reporter had a directory named "/jdk" which satisfies this > heuristic check. This makes fixpath assume that the `/jdk/foo` part is an > absolute unix path that needs to be rewritten to a Windows path. > > My suggested fix is to look at the prefix part, "test" in this case, and see > if that itself is a valid path in the current working directory, as that > would indicate that the string is intended to be a relative path. > > I think we also need to account for possible prefixes with `:` and `=` here > to handle a string like: > > jtreg:test/jdk/foo > > In that case we need to remove anything up to the last `:` before we try to > match it as a relative directory. (Same thing applies to `=`) > > Changing the heuristics of fixpath is rather sensitive and risky. I would > appreciate help from people using Windows with trying this patch with some of > your regular workflows. Works normally on both master and the experimental JDK-8288293 branch - PR Comment: https://git.openjdk.org/jdk/pull/15461#issuecomment-1696803731
Re: RFR: 8303427: Fixpath confused unix root contains "/jdk"
On Tue, 29 Aug 2023 00:16:46 GMT, Erik Joelsson wrote: > On Windows, when a directory exists in the "unix" root with the same name as > a directory in the "test" dir, fixpath will corrupt test arguments to jtreg > (and possibly other arguments as well). Fixpath sees a string like this: > > test/jdk/foo > > It looks for the first `/` and checks if the first element following that, > until the next `/`, is an existing directory in the unix filesystem root. In > this case, the reporter had a directory named "/jdk" which satisfies this > heuristic check. This makes fixpath assume that the `/jdk/foo` part is an > absolute unix path that needs to be rewritten to a Windows path. > > My suggested fix is to look at the prefix part, "test" in this case, and see > if that itself is a valid path in the current working directory, as that > would indicate that the string is intended to be a relative path. > > I think we also need to account for possible prefixes with `:` and `=` here > to handle a string like: > > jtreg:test/jdk/foo > > In that case we need to remove anything up to the last `:` before we try to > match it as a relative directory. (Same thing applies to `=`) > > Changing the heuristics of fixpath is rather sensitive and risky. I would > appreciate help from people using Windows with trying this patch with some of > your regular workflows. The GitHub Actions failures look terrifying, but all of them are just a failing JTReg download for some reason (Not related to the change). Will recompile with this change and report back as soon as I can - PR Comment: https://git.openjdk.org/jdk/pull/15461#issuecomment-1696737213
RFR: 8303427: Fixpath confused if jdk source tree located in "/jdk/jdk"
On Windows, when a directory exists in the "unix" root with the same name as a directory in the "test" dir, fixpath will corrupt test arguments to jtreg (and possibly other arguments as well). Fixpath sees a string like this: test/jdk/foo It looks for the first `/` and checks if the first element following that, until the next `/`, is an existing directory in the unix filesystem root. In this case, the reporter had a directory named "/jdk" which satisfies this heuristic check. This makes fixpath assume that the `/jdk/foo` part is an absolute unix path that needs to be rewritten to a Windows path. My suggested fix is to look at the prefix part, "test" in this case, and see if that itself is a valid path in the current working directory, as that would indicate that the string is intended to be a relative path. I think we also need to account for possible prefixes with `:` and `=` here to handle a string like: jtreg:test/jdk/foo In that case we need to remove anything up to the last `:` before we try to match it as a relative directory. (Same thing applies to `=`) Changing the heuristics of fixpath is rather sensitive and risky. I would appreciate help from people using Windows with trying this patch with some of your regular workflows. - Commit messages: - JDK-8303427 Changes: https://git.openjdk.org/jdk/pull/15461/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15461&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8303427 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15461.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15461/head:pull/15461 PR: https://git.openjdk.org/jdk/pull/15461
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v29]
On Mon, 28 Aug 2023 22:42:50 GMT, Srinivas Vamsi Parasa
wrote:
>> make/modules/java.base/Lib.gmk line 239:
>>
>>> 237:
>>>
>>> 238:
>>> 239: ifeq ($(call isTargetOs, linux)+$(call isTargetCpu,
>>> x86_64)+$(INCLUDE_COMPILER2), true+true+true)
>>
>> Is there a reason for this to only be supported on Linux?
>
> Hi Erik,
>
> The reason this PR is focused on Linux is because the AVX512 sort and
> partitioning routines are based on Intel’s x86-simd-library
> (https://github.com/intel/x86-simd-sort) which was originally developed with
> GCC as the target compiler. Thus, this PR has restricted itself to Linux as
> the code was tested using GCC/Linux platforms.
> Additionally, the x86_64 library is compiled for AVX512 using file specific
> compilation pragmas (`#pragma GCC target("avx512dq", "avx512f")`). This
> feature is absent for Windows/MSVC++ compiler.”
>
> Thanks,
> Vamsi
If it's tied to GCC as well, then we should probably include that in the
condition here unless it's also expected to work with Clang. (`TOOLCHAIN_TYPE`
= `gcc`)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14227#discussion_r1308050702
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v30]
On Mon, 28 Aug 2023 21:27:25 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX512 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and >> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the >> performance data below. >> >> >> **Arrays.sort performance data using JMH benchmarks for arrays with random >> data** >> >> |Arrays.sort benchmark | Array Size | Baseline >> (us/op)| AVX512 Sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | >> |ArraysSort.doubleSort | 10 | 0.034 | 0.035 >> | 1.0 | >> |ArraysSort.doubleSort | 25 | 0.116 | 0.089 >> | 1.3 | >> |ArraysSort.doubleSort | 50 | 0.282 | 0.291 >> | 1.0 | >> |ArraysSort.doubleSort | 75 | 0.474 | 0.358 >> | 1.3 | >> |ArraysSort.doubleSort | 100 | 0.654 | 0.623 >> | 1.0 | >> |ArraysSort.doubleSort | 1000| 9.274 | 6.331 >> | 1.5 | >> |ArraysSort.doubleSort | 1 | 323.339 | 71.228 >> | **4.5** | >> |ArraysSort.doubleSort | 10 | 4471.871| >> 1002.748| **4.5** | >> |ArraysSort.doubleSort | 100 | 51660.742 | >> 12921.295 | **4.0** | >> |ArraysSort.floatSort| 10 | 0.045 | 0.046 >> | 1.0 | >> |ArraysSort.floatSort| 25 | 0.103 | 0.084 >> | 1.2 | >> |ArraysSort.floatSort| 50 | 0.285 | 0.33 >> | 0.9 | >> |ArraysSort.floatSort| 75 | 0.492 | 0.346 >> | 1.4 | >> |ArraysSort.floatSort| 100 | 0.597 | 0.326 >> | 1.8 | >> |ArraysSort.floatSort| 1000| 9.811 | 5.294 >> | 1.9 | >> |ArraysSort.floatSort| 1 | 323.955 | 50.547 >> | **6.4** | >> |ArraysSort.floatSort| 10 | 4326.38 | 731.152 >> | **5.9** | >> |ArraysSort.floatSort| 100 | 52413.88| >> 8409.193| **6.2** | >> |ArraysSort.intSort | 10 | 0.033 | 0.033 >> | 1.0 | >> |ArraysSort.intSort | 25 | 0.086 | 0.051 >> | 1.7 | >> |ArraysSort.intSort | 50 | 0.236 | 0.151 >> | 1.6 | >> |ArraysSort.intSort | 75 | 0.416 | 0.332 >> | 1.3 | >> |ArraysSort.intSort | 100 | 0.63| 0.521 >> | 1.2 | >> |ArraysSort.intSort | 1000| 10.518 | 4.698 >> | 2.2 | >> |ArraysSort.intSort | 1 | 309.659 | 42.518 >> | **7.3** | >> |ArraysSort.intSort | 10 | 4130.917| >> 573.956 | **7.2** | >> |ArraysSort.intSort | 100 | 49876.307 | >> 6712.812| **7.4** | >> |ArraysSort.longSort | 10 | 0.036 | 0.037 >> | 1.0 | >> |ArraysSort.longSort | 25 | 0.094 | 0.08 >> | 1.2 | >> |ArraysSort.longSort | 50 | 0.218 | 0.227 >> | 1.0 | >> |ArraysSort.longSort | 75 | 0.466 | 0.402 >> | 1.2 | >> |ArraysSort.longSort | 100 | 0.76| 0.58 >> | 1.3 | >> |ArraysSort.longSort | 1000| 10.449 | 6 > > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Clean up parameters passed to arrayPartition; update the check to load > library make/modules/java.base/Lib.gmk line 240: > 238: > 239: ifeq ($(call isTargetOs, linux)+$(call isTargetCpu, > x86_64)+$(INCLUDE_COMPILER2), true+true+true) > 240: $(eval $(call SetupJdkLibrary, BUILD_LIB_X86_64, \ As this is a C++ lib, consider using g++ for linking by setting: TOOLCHAIN := TOOLCHAIN_LINK_CXX make/modules/java.base/Lib.gmk line 241: > 239: ifeq ($(call isTargetOs, linux)+$(call isTargetCpu, > x86_64)+$(INCLUDE_COMPILER2), true+true+true) > 240: $(eval $(call SetupJdkLibrary, BUILD_LIB_X86_64, \ > 241: NAME := x86_64,
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v30]
On Mon, 28 Aug 2023 21:27:25 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX512 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and >> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the >> performance data below. >> >> >> **Arrays.sort performance data using JMH benchmarks for arrays with random >> data** >> >> |Arrays.sort benchmark | Array Size | Baseline >> (us/op)| AVX512 Sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | >> |ArraysSort.doubleSort | 10 | 0.034 | 0.035 >> | 1.0 | >> |ArraysSort.doubleSort | 25 | 0.116 | 0.089 >> | 1.3 | >> |ArraysSort.doubleSort | 50 | 0.282 | 0.291 >> | 1.0 | >> |ArraysSort.doubleSort | 75 | 0.474 | 0.358 >> | 1.3 | >> |ArraysSort.doubleSort | 100 | 0.654 | 0.623 >> | 1.0 | >> |ArraysSort.doubleSort | 1000| 9.274 | 6.331 >> | 1.5 | >> |ArraysSort.doubleSort | 1 | 323.339 | 71.228 >> | **4.5** | >> |ArraysSort.doubleSort | 10 | 4471.871| >> 1002.748| **4.5** | >> |ArraysSort.doubleSort | 100 | 51660.742 | >> 12921.295 | **4.0** | >> |ArraysSort.floatSort| 10 | 0.045 | 0.046 >> | 1.0 | >> |ArraysSort.floatSort| 25 | 0.103 | 0.084 >> | 1.2 | >> |ArraysSort.floatSort| 50 | 0.285 | 0.33 >> | 0.9 | >> |ArraysSort.floatSort| 75 | 0.492 | 0.346 >> | 1.4 | >> |ArraysSort.floatSort| 100 | 0.597 | 0.326 >> | 1.8 | >> |ArraysSort.floatSort| 1000| 9.811 | 5.294 >> | 1.9 | >> |ArraysSort.floatSort| 1 | 323.955 | 50.547 >> | **6.4** | >> |ArraysSort.floatSort| 10 | 4326.38 | 731.152 >> | **5.9** | >> |ArraysSort.floatSort| 100 | 52413.88| >> 8409.193| **6.2** | >> |ArraysSort.intSort | 10 | 0.033 | 0.033 >> | 1.0 | >> |ArraysSort.intSort | 25 | 0.086 | 0.051 >> | 1.7 | >> |ArraysSort.intSort | 50 | 0.236 | 0.151 >> | 1.6 | >> |ArraysSort.intSort | 75 | 0.416 | 0.332 >> | 1.3 | >> |ArraysSort.intSort | 100 | 0.63| 0.521 >> | 1.2 | >> |ArraysSort.intSort | 1000| 10.518 | 4.698 >> | 2.2 | >> |ArraysSort.intSort | 1 | 309.659 | 42.518 >> | **7.3** | >> |ArraysSort.intSort | 10 | 4130.917| >> 573.956 | **7.2** | >> |ArraysSort.intSort | 100 | 49876.307 | >> 6712.812| **7.4** | >> |ArraysSort.longSort | 10 | 0.036 | 0.037 >> | 1.0 | >> |ArraysSort.longSort | 25 | 0.094 | 0.08 >> | 1.2 | >> |ArraysSort.longSort | 50 | 0.218 | 0.227 >> | 1.0 | >> |ArraysSort.longSort | 75 | 0.466 | 0.402 >> | 1.2 | >> |ArraysSort.longSort | 100 | 0.76| 0.58 >> | 1.3 | >> |ArraysSort.longSort | 1000| 10.449 | 6 > > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Clean up parameters passed to arrayPartition; update the check to load > library Thanks for considering all the review comments and fixing them. The PR looks good to me. @PaulSandoz Could you please review the Java part? - Marked as reviewed by sviswanathan (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14227#pullrequestreview-1599251153 PR Comment: https://git.openjdk.org/jdk/pull/14227#issuecomment-1696551426
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v29]
On Fri, 25 Aug 2023 13:20:09 GMT, Erik Joelsson wrote:
>> Srinivas Vamsi Parasa has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove unnecessary import in Arrays.java
>
> make/modules/java.base/Lib.gmk line 239:
>
>> 237:
>>
>> 238:
>> 239: ifeq ($(call isTargetOs, linux)+$(call isTargetCpu,
>> x86_64)+$(INCLUDE_COMPILER2), true+true+true)
>
> Is there a reason for this to only be supported on Linux?
Hi Erik,
The reason this PR is focused on Linux is because the AVX512 sort and
partitioning routines are based on Intel’s x86-simd-library
(https://github.com/intel/x86-simd-sort) which was originally developed with
GCC as the target compiler. Thus, this PR has restricted itself to Linux as the
code was tested using GCC/Linux platforms.
Additionally, the x86_64 library is compiled for AVX512 using file specific
compilation pragmas (`#pragma GCC target("avx512dq", "avx512f")`). This feature
is absent for Windows/MSVC++ compiler.”
Thanks,
Vamsi
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14227#discussion_r1308027470
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v29]
On Fri, 25 Aug 2023 22:04:45 GMT, Vladimir Kozlov wrote:
>> Srinivas Vamsi Parasa has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove unnecessary import in Arrays.java
>
> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 4143:
>
>> 4141: log_info(library)("Loaded library %s, handle " INTPTR_FORMAT,
>> JNI_LIB_PREFIX "x86_64" JNI_LIB_SUFFIX, p2i(libx86_64));
>> 4142:
>> 4143: if (UseAVX > 2 && VM_Version::supports_avx512dq()) {
>
> This check should be done before you locate and load library
Please see the check moved to before loading the library in the latest commit.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14227#discussion_r1307962504
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v30]
> The goal is to develop faster sort routines for x86_64 CPUs by taking > advantage of AVX512 instructions. This enhancement provides an order of > magnitude speedup for Arrays.sort() using int, long, float and double arrays. > > This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and upto > ~4.5x improvement for 64-bit datatypes (long, double) as shown in the > performance data below. > > > **Arrays.sort performance data using JMH benchmarks for arrays with random > data** > > | Arrays.sort benchmark | Array Size | Baseline > (us/op)| AVX512 Sort (us/op) | Speedup | > | --- | --- | --- | --- | --- > | > | ArraysSort.doubleSort | 10 | 0.034 | 0.035 > | 1.0 | > | ArraysSort.doubleSort | 25 | 0.116 | 0.089 > | 1.3 | > | ArraysSort.doubleSort | 50 | 0.282 | 0.291 > | 1.0 | > | ArraysSort.doubleSort | 75 | 0.474 | 0.358 > | 1.3 | > | ArraysSort.doubleSort | 100 | 0.654 | 0.623 > | 1.0 | > | ArraysSort.doubleSort | 1000| 9.274 | 6.331 > | 1.5 | > | ArraysSort.doubleSort | 1 | 323.339 | 71.228 > | **4.5** | > | ArraysSort.doubleSort | 10 | 4471.871| > 1002.748| **4.5** | > | ArraysSort.doubleSort | 100 | 51660.742 | > 12921.295 | **4.0** | > | ArraysSort.floatSort| 10 | 0.045 | 0.046 > | 1.0 | > | ArraysSort.floatSort| 25 | 0.103 | 0.084 > | 1.2 | > | ArraysSort.floatSort| 50 | 0.285 | 0.33 > | 0.9 | > | ArraysSort.floatSort| 75 | 0.492 | 0.346 > | 1.4 | > | ArraysSort.floatSort| 100 | 0.597 | 0.326 > | 1.8 | > | ArraysSort.floatSort| 1000| 9.811 | 5.294 > | 1.9 | > | ArraysSort.floatSort| 1 | 323.955 | 50.547 > | **6.4** | > | ArraysSort.floatSort| 10 | 4326.38 | 731.152 > | **5.9** | > | ArraysSort.floatSort| 100 | 52413.88| > 8409.193| **6.2** | > | ArraysSort.intSort | 10 | 0.033 | 0.033 > | 1.0 | > | ArraysSort.intSort | 25 | 0.086 | 0.051 > | 1.7 | > | ArraysSort.intSort | 50 | 0.236 | 0.151 > | 1.6 | > | ArraysSort.intSort | 75 | 0.416 | 0.332 > | 1.3 | > | ArraysSort.intSort | 100 | 0.63| 0.521 > | 1.2 | > | ArraysSort.intSort | 1000| 10.518 | 4.698 > | 2.2 | > | ArraysSort.intSort | 1 | 309.659 | 42.518 > | **7.3** | > | ArraysSort.intSort | 10 | 4130.917| > 573.956 | **7.2** | > | ArraysSort.intSort | 100 | 49876.307 | > 6712.812| **7.4** | > | ArraysSort.longSort | 10 | 0.036 | 0.037 > | 1.0 | > | ArraysSort.longSort | 25 | 0.094 | 0.08 > | 1.2 | > | ArraysSort.longSort | 50 | 0.218 | 0.227 > | 1.0 | > | ArraysSort.longSort | 75 | 0.466 | 0.402 > | 1.2 | > | ArraysSort.longSort | 100 | 0.76| 0.58 > | 1.3 | > | ArraysSort.longSort | 1000| 10.449 | 6.239 > | 1.7 | > | ArraysSort.longSort | 1 | 307.074 | 70.284 > | **4.4** | > | ArraysSor... Srinivas Vamsi Parasa has updated the pull request incrementally with one additional commit since the last revision: Clean up parameters passed to arrayPartition; update the check to load library - Changes: - all: https://git.openjdk.org/jdk/pull/14227/files - new: https://git.openjdk.org/jdk/pull/14227/files/e44f11a6..9642d852 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14227&range=29 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14227&range=28-29 Stats: 56 lines in 7 files changed: 17 ins; 19 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/14227.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14227/head:pull/14227 PR: https://git.openjdk.org/jdk/pull/14227
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v26]
On Fri, 25 Aug 2023 01:52:32 GMT, Sandhya Viswanathan wrote: >> pivotIndices array is being passed as a parameter to the partition intrinsic >> as it is updated in-place with the new pivot indices after partitioning. The >> Unsafe.ARRAY_INT_BASE_OFFSET is being used in libary_call.cpp to get the >> address of pivotIndices. > > As PivotIndices is local to the DualPivotQuickSort and is always going to be > int array, there are other ways to compute the address in library_call.cpp > without having to pass an additional argument. As suggested, the code was updated to no longer pass the offset(Unsafe.ARRAY_INT_BASE_OFFSET) for pivotIndices array. Please see the code in the latest commit. - PR Review Comment: https://git.openjdk.org/jdk/pull/14227#discussion_r1307964768
Integrated: 8315060: Out of tree incremental build fails with ccache
On Fri, 25 Aug 2023 22:31:31 GMT, Erik Joelsson wrote: > After [JDK-8313374](https://bugs.openjdk.org/browse/JDK-8313374), out of tree > incremental builds with ccache started failing. The rewriting of the > generated dependency (*.d) files creates object file paths with `/../` in > them which make does not match to other rules. I don't think we should apply > the rewriting when the build dir is outside CCACHE_BASEDIR. > > During review of [JDK-8313374](https://bugs.openjdk.org/browse/JDK-8313374) I > also noted that the CCACHE_BASEDIR should point to WORKSPACE_ROOT rather than > TOPDIR for ccache to work uniformly for OracleJDK and OpenJDK builds. > > Both of these issues are addressed with this patch. I have tried various > scenarios and it seems to be working for me, but would appreciate some more > verification by people using ccache. This pull request has now been integrated. Changeset: 69d1feb8 Author:Erik Joelsson URL: https://git.openjdk.org/jdk/commit/69d1feb83f0e1f411f3b62f74e1a488f0dd29b15 Stats: 9 lines in 2 files changed: 6 ins; 0 del; 3 mod 8315060: Out of tree incremental build fails with ccache Reviewed-by: kbarrett, dholmes - PR: https://git.openjdk.org/jdk/pull/15434
Re: RFR: 8315060: Out of tree incremental build fails with ccache
On Fri, 25 Aug 2023 22:31:31 GMT, Erik Joelsson wrote: > After [JDK-8313374](https://bugs.openjdk.org/browse/JDK-8313374), out of tree > incremental builds with ccache started failing. The rewriting of the > generated dependency (*.d) files creates object file paths with `/../` in > them which make does not match to other rules. I don't think we should apply > the rewriting when the build dir is outside CCACHE_BASEDIR. > > During review of [JDK-8313374](https://bugs.openjdk.org/browse/JDK-8313374) I > also noted that the CCACHE_BASEDIR should point to WORKSPACE_ROOT rather than > TOPDIR for ccache to work uniformly for OracleJDK and OpenJDK builds. > > Both of these issues are addressed with this patch. I have tried various > scenarios and it seems to be working for me, but would appreciate some more > verification by people using ccache. Thanks for reviews! - PR Comment: https://git.openjdk.org/jdk/pull/15434#issuecomment-1696397652
Re: building individual test issue
Hi, I believe this was due to building OpenJDK from Debian original tarball that does not contain bundled libraries. Building from it requires applying Debian patches via `quilt push -a` and using configure arguments from `debian/rules`. Best Regards, Vladimir. On Tue, Aug 29, 2023 at 1:02 AM David Holmes wrote: > > On 28/08/2023 9:54 pm, Mykhaylo Lodygin wrote: > > Hello colleagues, > > I've build JDK locally successfully, but got several tests "FAILED". > > What did you build? images? what about test-images? > > > Tried to run one of the failing test separately with "make test > > TEST="". > > Were unable to do so due to build error - the compiler was unfamiliar to > > include path to winscard.h, fixed with configuration option: > > bash configure --with-extra-cflags="-I/usr/include/PCSC" > > What test(s), on what platform? > > > next error was incapability of linker to link with libjpeg - fixed with > > adding --with-libjpeg=system to configure. > > the issue was solved, but a new one was added with missing libgif. > > I was curious and tried a test that "Passed", thinking that "failure" > > could had happen due to inability to build the test. Tried > > sun/util/calendar/zi/Beyond2037.java but got a very similar error of > > inability to link with liblcms2-2. > > I'm flabbergasted, hence my questions are: does "make test" target is > > expected to rebuild JDK's code? does my approach of adding options to > > configure script is proper one? How come a test could be "Passed" while > > running it separately results in inability to build the test? > > As Erik indicated we need more information/details. > > Cheers, > David > > > > Thank you. > > Mykhaylo Lodygin
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v22]
On Fri, 25 Aug 2023 18:59:47 GMT, Srinivas Vamsi Parasa wrote: >>> Improvements are nice but it would not pay off if you have big regressions. >>> I can accept 0.9x but 0.4x - 0.8x regressions should be investigated and >>> implementation adjusted to avoid them. >> >> Hello Vladimir (@vnkozlov) , >> >> As per your suggestion, the implementation was adjusted to address the >> regressions caused for STAGGER and REPEATED type of input data patterns. >> Please see below the new JMH performance data using the adjusted >> implementation. >> >> In the new implementation, we don't call the AVX512 sort intrinsic at the >> top level (`Arrays.sort()`) . Instead, we take a decomposed or a 'building >> blocks' approach where we only intrinsify (using AVX512 instructions) the >> partitioning and small array sort functions used in the current baseline ( >> `DualPivotQuickSort.sort()` ). Since the current baseline has logic to >> detect and sort special input patterns like STAGGER, we fallback to the >> current baseline instead of using AVX512 partitioning and sorting (which >> works best for RANDOM, REPEATED and SHUFFLE patterns). >> >> Data below shows `Arrays.sort()` performance comparison between the current >> **Java baseline (DPQS)** vs. **AVX512 sort** (this PR) using the >> `ArraysSort.java` JMH >> [benchmark](https://github.com/openjdk/jdk/pull/13568/files#diff-dee51b13bd1872ff455cec2f29255cfd25014a5dd33dda55a2fc68638c3dd4b2) >> provided in the PR for [JDK-8266431: Dual-Pivot Quicksort improvements >> (Radix sort)](https://github.com/openjdk/jdk/pull/13568/files#top) ( #13568) >> >> - The following command line was used to run the benchmarks: ` java -jar >> $JDK_HOME/build/linux-x86_64-server-release/images/test/micro/benchmarks.jar >> -jvmArgs "-XX:CompileThreshold=1 -XX:-TieredCompilation" ArraysSort` >> - The scores shown are the average time (us/op), thus lower is better. The >> last column towards the right shows the speedup. >> >> >> |Benchmark | Mode| Size| Baseline DPQS >> (us/op) | AVX512 partitioning & sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | --- | >> |ArraysSort.Double.testSort | RANDOM | 800 | >> 6.7 | 4.8 | 1.39x | >> |ArraysSort.Double.testSort | RANDOM | 7000| >> 234.1 | 51.5| **4.55x** | >> |ArraysSort.Double.testSort | RANDOM | 5 | >> 2155.9 | 470.0 | **4.59x** | >> |ArraysSort.Double.testSort | RANDOM | 30 | >> 15076.3 | 3391.3 | **4.45x** | >> |ArraysSort.Double.testSort | RANDOM | 200 | >> 116445.5| 27491.7 | **4.24x** | >> |ArraysSort.Double.testSort | REPEATED| 800 >> | 2.3 | 1.7 | 1.35x | >> |ArraysSort.Double.testSort | REPEATED| 7000 >> | 23.3| 12... > >> @vamsi-parasa I submitted our testing of latest v28 version. It found issue >> in `ArraysSort.java` new benchmark file. You missed `,`after year in >> copyright line: >> >> ``` >> * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. >> ``` > > Thank you, Vladimir! Hi @vamsi-parasa , I did some runs on linux by disabling intrinsification of _arraySort and _arrayPartition since these are not handled for windows and non-x86 targets, just to measure the impact of java side code re-factoring.    Please find the results, may I request you to kindly verify performance number on windows to be sure that there are no performance degradation. Best Regards, Jatin - PR Comment: https://git.openjdk.org/jdk/pull/14227#issuecomment-1696209731
Integrated: 8315062: [GHA] get-bootjdk action should return the abolute path
On Fri, 25 Aug 2023 22:32:13 GMT, Xin Liu wrote:
> We are running hotspot:tier2 on github action(GHA). I have difficulty to
> execute this test on GHA:
> Runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java
>
> In GHA, the java property is gained from environment variable BOOT_JDK.
> BOOT_JDK=${{ steps.bootjdk.outputs.path }}
>
> it is in turn from action get-bootjdk. now it returns a relative path. The
> aforementioned test has trouble to find the executable if the path relative.
> I would recommend changing the action get-bootjdk. we just return the
> absolute path.
This pull request has now been integrated.
Changeset: 99ea8bf2
Author:Xin Liu
URL:
https://git.openjdk.org/jdk/commit/99ea8bf2b962011e57d02a93217d65d7259e8f80
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
8315062: [GHA] get-bootjdk action should return the abolute path
Reviewed-by: clanger, erikj
-
PR: https://git.openjdk.org/jdk/pull/15435
Re: RFR: 8314753: Remove support for @beaninfo, @ToDo, @since.unbundled, and @Note [v2]
On Tue, 22 Aug 2023 14:55:18 GMT, Pavel Rappo wrote: >> Please review this trivial PR. > > Pavel Rappo has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Merge branch 'master' into 8314753 > - Initial commit Marked as reviewed by azvegint (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15385#pullrequestreview-1598378564
Re: building individual test issue
On 28/08/2023 9:54 pm, Mykhaylo Lodygin wrote: Hello colleagues, I've build JDK locally successfully, but got several tests "FAILED". What did you build? images? what about test-images? Tried to run one of the failing test separately with "make test TEST="". Were unable to do so due to build error - the compiler was unfamiliar to include path to winscard.h, fixed with configuration option: bash configure --with-extra-cflags="-I/usr/include/PCSC" What test(s), on what platform? next error was incapability of linker to link with libjpeg - fixed with adding --with-libjpeg=system to configure. the issue was solved, but a new one was added with missing libgif. I was curious and tried a test that "Passed", thinking that "failure" could had happen due to inability to build the test. Tried sun/util/calendar/zi/Beyond2037.java but got a very similar error of inability to link with liblcms2-2. I'm flabbergasted, hence my questions are: does "make test" target is expected to rebuild JDK's code? does my approach of adding options to configure script is proper one? How come a test could be "Passed" while running it separately results in inability to build the test? As Erik indicated we need more information/details. Cheers, David Thank you. Mykhaylo Lodygin
Re: building individual test issue
Hello Mykhaylo Lodygin, What you describe sounds strange. If you successfully built the JDK and ran tests, then nothing should be rebuilt when trying to run tests again. What operating system are you running this on? Which JDK src did you clone? What was your original configure command line? /Erik On 8/28/23 04:54, Mykhaylo Lodygin wrote: Hello colleagues, I've build JDK locally successfully, but got several tests "FAILED". Tried to run one of the failing test separately with "make test TEST="". Were unable to do so due to build error - the compiler was unfamiliar to include path to winscard.h, fixed with configuration option: bash configure --with-extra-cflags="-I/usr/include/PCSC" next error was incapability of linker to link with libjpeg - fixed with adding --with-libjpeg=system to configure. the issue was solved, but a new one was added with missing libgif. I was curious and tried a test that "Passed", thinking that "failure" could had happen due to inability to build the test. Tried sun/util/calendar/zi/Beyond2037.java but got a very similar error of inability to link with liblcms2-2. I'm flabbergasted, hence my questions are: does "make test" target is expected to rebuild JDK's code? does my approach of adding options to configure script is proper one? How come a test could be "Passed" while running it separately results in inability to build the test? Thank you. Mykhaylo Lodygin
Re: RFR: 8315062: [GHA] get-bootjdk action should return the abolute path
On Fri, 25 Aug 2023 22:32:13 GMT, Xin Liu wrote:
> We are running hotspot:tier2 on github action(GHA). I have difficulty to
> execute this test on GHA:
> Runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java
>
> In GHA, the java property is gained from environment variable BOOT_JDK.
> BOOT_JDK=${{ steps.bootjdk.outputs.path }}
>
> it is in turn from action get-bootjdk. now it returns a relative path. The
> aforementioned test has trouble to find the executable if the path relative.
> I would recommend changing the action get-bootjdk. we just return the
> absolute path.
Marked as reviewed by erikj (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/15435#pullrequestreview-1598214130
building individual test issue
Hello colleagues, I've build JDK locally successfully, but got several tests "FAILED". Tried to run one of the failing test separately with "make test TEST="". Were unable to do so due to build error - the compiler was unfamiliar to include path to winscard.h, fixed with configuration option: bash configure --with-extra-cflags="-I/usr/include/PCSC" next error was incapability of linker to link with libjpeg - fixed with adding --with-libjpeg=system to configure. the issue was solved, but a new one was added with missing libgif. I was curious and tried a test that "Passed", thinking that "failure" could had happen due to inability to build the test. Tried sun/util/calendar/zi/Beyond2037.java but got a very similar error of inability to link with liblcms2-2. I'm flabbergasted, hence my questions are: does "make test" target is expected to rebuild JDK's code? does my approach of adding options to configure script is proper one? How come a test could be "Passed" while running it separately results in inability to build the test? Thank you. Mykhaylo Lodygin
Re: RFR: 8315020: The macro definition for LoongArch64 zero build is not accurate. [v2]
On Sun, 27 Aug 2023 12:50:31 GMT, Fei Yang wrote: >> Ao Qi has updated the pull request incrementally with one additional commit >> since the last revision: >> >> rename LOONGARCH and move RISCV32 to zero > > Marked as reviewed by fyang (Reviewer). Thank you, @RealFYang. - PR Comment: https://git.openjdk.org/jdk/pull/15428#issuecomment-1695515580
Integrated: 8315020: The macro definition for LoongArch64 zero build is not accurate.
On Fri, 25 Aug 2023 10:17:38 GMT, Ao Qi wrote: > The `LOONGARCH` is not a macro that must be defined. It is not a C/C++ > pre-defined macro and is also not pre-defined in libraries such as libffi. > We'd better use a macro defined in jdk. This pull request has now been integrated. Changeset: 725ec0ce Author:Ao Qi Committer: Fei Yang URL: https://git.openjdk.org/jdk/commit/725ec0ce1b463b21cd4c5287cf4ccbee53ec7349 Stats: 8 lines in 2 files changed: 4 ins; 2 del; 2 mod 8315020: The macro definition for LoongArch64 zero build is not accurate. Reviewed-by: erikj, fyang - PR: https://git.openjdk.org/jdk/pull/15428
Integrated: 8314762: Make {@Incubating} conventional
On Tue, 22 Aug 2023 12:19:42 GMT, Pavel Rappo wrote:
> Please review this trivial PR.
This pull request has now been integrated.
Changeset: 0901d75e
Author:Pavel Rappo
URL:
https://git.openjdk.org/jdk/commit/0901d75e074322c5a8d55e3c72c4cba4291fb00c
Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod
8314762: Make {@Incubating} conventional
Reviewed-by: jjg, iris, chegar
-
PR: https://git.openjdk.org/jdk/pull/15387
Re: RFR: 8315062: [GHA] get-bootjdk action should return the abolute path
On Fri, 25 Aug 2023 22:32:13 GMT, Xin Liu wrote:
> We are running hotspot:tier2 on github action(GHA). I have difficulty to
> execute this test on GHA:
> Runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchiveUpgrade.java
>
> In GHA, the java property is gained from environment variable BOOT_JDK.
> BOOT_JDK=${{ steps.bootjdk.outputs.path }}
>
> it is in turn from action get-bootjdk. now it returns a relative path. The
> aforementioned test has trouble to find the executable if the path relative.
> I would recommend changing the action get-bootjdk. we just return the
> absolute path.
LGTM
-
Marked as reviewed by clanger (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15435#pullrequestreview-1597867929
Re: RFR: JDK-8313764: Offer JVM HS functionality to shared lib load operations done by the JDK codebase [v2]
On Mon, 28 Aug 2023 02:26:30 GMT, David Holmes wrote: > Sorry but looking at the hotspot part of this I do not like the code in > jvm.cpp at all - it is far too messy. I expected to see a simple interface to > os::dlopen which then handles all the platform specific issues. > > I'm also somewhat dubious about having all the JDK code use a VM interface > for this, The usual coupling to the VM is to just provide native > implementations of core library methods, not to have the VM provide a general > purpose utility interface like dynamic library loading. Hi David, so would you prefer to call into 'os::dll_load' (probably you meant that) instead ? That would indeed make the coding smaller and less 'messy' . However 'os::dll_load' has no flags parameter but uses a default, have to check if this might be a problem ... - PR Comment: https://git.openjdk.org/jdk/pull/15264#issuecomment-1695142975
