Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Thu, 20 Jun 2024 21:53:25 GMT, Justin Lu wrote:
> > It requires append(int), but the Appendable has no such method.
>
> If via `Appendable` we don't have access to `append(int)`, can we simply
> `append(String.valueOf(int))`. And similarly for `append(char[], int, int)`,
> can we `append(String.valueOf(char[], int, int))`.
>
> According to the method descriptions in `AbstractStringBuilder`, this should
> behaviorally be the same. That way we don't have to add any new classes, and
> can continue with the generic implementation.
If do that ,there is some performance degradation.
Benchmark Mode Cnt
Score Error Units
AppendableTest.tesAppendableAppendCharAsciiavgt 50
33.628 ± 0.263 ns/op
AppendableTest.tesAppendableAppendCharMixNoAsciiChars avgt 50
33.522 ± 0.245 ns/op
AppendableTest.testAppendableAppendInt avgt 50
32.602 ± 0.186 ns/op
AppendableTest.testStringBufferAppendCharArrayAsciiavgt 50
31.028 ± 0.134 ns/op
AppendableTest.testStringBufferAppendCharArrayMixNoAsciiChars avgt 50
31.075 ± 0.158 ns/op
AppendableTest.testStringBufferAppendInt avgt 50
25.706 ± 0.268 ns/op
@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@State(Scope.Thread)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class AppendableTest {
private StringBuffer sb;
private Appendable appendable;
private char[] asciiChars;
private char[] mixNoAsciiChars;
@Setup(Level.Iteration)
public void setup() {
sb = new StringBuffer();
appendable = sb;
asciiChars = "Criticism appears to Anatole France the most recent and
possibly the ultimate evolution".toCharArray();
mixNoAsciiChars = "The 测试数据 above mentioned two volumes of poetry were
followed by many works in prose, which we shall notice. France’s critical
writings".toCharArray();
}
@Benchmark
public void testStringBufferAppendInt() throws InterruptedException {
sb.append(12345890);
}
@Benchmark
public void testAppendableAppendInt() throws InterruptedException,
IOException {
appendable.append(String.valueOf(12345890));
}
@Benchmark
public void testStringBufferAppendCharArrayAscii() throws
InterruptedException {
sb.append(asciiChars, 40, 18);
}
@Benchmark
public void tesAppendableAppendCharAscii() throws InterruptedException,
IOException {
appendable.append(new String(asciiChars, 40, 18));
}
@Benchmark
public void testStringBufferAppendCharArrayMixNoAsciiChars() throws
InterruptedException {
sb.append(mixNoAsciiChars, 40, 18);
}
@Benchmark
public void tesAppendableAppendCharMixNoAsciiChars() throws
InterruptedException, IOException {
appendable.append(new String(mixNoAsciiChars, 40, 18));
}
}
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2181851223
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v3]
On Thu, 20 Jun 2024 20:30:36 GMT, Steven Loomis wrote: > CLDR is aware of this change but hasn't added it yet. We're mid cycle so it > won't be much more than metadata this round due to timing. > https://unicode-org.atlassian.net/issues/CLDR-17751 if you want to track. Out of curiosity, what's the rationale behind the currency name change from "Zimbabwe Dollar" (ISO 4217 definition) to "Zimbabwean Dollar"? - PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1648324116
[jdk23] RFR: 8334333: MissingResourceCauseTestRun.java fails if run by root
Hi all, This pull request contains a backport of commit [de8ee977](https://github.com/openjdk/jdk/commit/de8ee97718d7e12b541b310cf5b67f3e10e91ad9) from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. The commit being backported was authored by SendaoYan on 20 Jun 2024 and was reviewed by Naoto Sato and Justin Lu. Thanks! - Commit messages: - Backport de8ee97718d7e12b541b310cf5b67f3e10e91ad9 Changes: https://git.openjdk.org/jdk/pull/19817/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19817&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8334333 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19817.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19817/head:pull/19817 PR: https://git.openjdk.org/jdk/pull/19817
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows make/common/native/Link.gmk line 72: > 70: define CreateStaticLibrary > 71: # Include partial linking when building the static library with clang > on linux > 72: ifeq ($(call isTargetOs, linux macosx), true) Is this mainly for `clang` support for now? - PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1648293391
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows make/modules/java.desktop/lib/AwtLibraries.gmk line 257: > 255: JDK_LIBS := libawt java.base:libjava, \ > 256: LIBS_unix := $(LIBDL) $(LIBM) $(X_LIBS) -lX11 -lXext -lXi > -lXrender \ > 257: -lXtst, \ Same question as for the STATIC_LIB_EXCLUDE_OBJS change with `libjli`. Are the duplicate symbol issues resolved by symbol hiding? I think it's still better to not include those .o files to avoid unnecessary footprint overhead in the binary. - PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1648292220
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows make/modules/java.base/lib/CoreLibraries.gmk line 148: > 146: $(LIBJLI_EXTRA_FILE_LIST)) > 147: > 148: # Do not include these libz objects in the static libjli library. Why this is no longer needed? - PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1648290693
Re: RFR: 8334333: MissingResourceCauseTestRun.java fails if run by root [v4]
On Tue, 18 Jun 2024 07:41:35 GMT, SendaoYan wrote: >> Hi all, >> Testcase >> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTestRun.java` >> run fails with root user privileged. I think it's necessary to skip this >> testcase when user is root. >> Why run the jtreg test by root user? It's because during rpmbuild process >> for linux distribution of JDK, root user is the default user to build the >> openjdk, also is the default user to run the `make test-tier1`, this PR make >> this testcase more robustness. >> The change has been verified, only change the testcase, no risk. > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > add bug id 8334333 to jtreg tag @bug Thanks for the sponsor. - PR Comment: https://git.openjdk.org/jdk/pull/19732#issuecomment-2181783165
Re: RFR: 8333268: Fixes for static build [v2]
On Tue, 18 Jun 2024 17:57:29 GMT, Magnus Ihse Bursie wrote:
>> Magnus Ihse Bursie 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 seven
>> additional commits since the last revision:
>>
>> - Merge branch 'master' into static-linking-progress
>> - Merge branch 'master' into static-linking-progress
>> - Move the exported JVM_IsStaticallyLinked to a better location
>> - Use runtime lookup of static vs dynamic instead of #ifdef STATIC_BUILD
>> - Copy fix for init_system_properties_values on linux
>> - Make sure we do not try to build static libraries on Windows
>> - 8333268: Fixes for static build
>
> src/hotspot/os/linux/os_linux.cpp line 605:
>
>> 603:
>> 604: // Get rid of /{client|server|hotspot}, if binary is libjvm.so.
>> 605: // Or, cut off /.
>
> @jianglizhou This code is based on changes in the Hermetic Java repo, but I
> do not fully understand neither the comment nor what the purpose is. If you
> could explain this a bit I'd be grateful.
The specific related commit in the hermetic Java branch is
https://github.com/openjdk/leyden/commit/53aa8f0cf418ab5f435a4b9996c7754fb8505d4b.
The change in os_linux.cpp here is to make sure that the libjvm.so related
path manipulation is conditionally done only. The check at line 599 looks for
"/libjvm.so" substring, so we only chop off (`*pslash = `\0` at line 601) that
part when necessary. In the static JDK case, there is no `libjvm.so` and the
path string is `/bin/javastatic`, which should not be affected.
Otherwise, it could fail.
I found the code was not very easy to follow when running into problems and
fixing for static support. So I added a bit more comments in the code here. The
comment above about `/{client|server|hotspot}` was there originally. I think we
no longer have those directories. We can cleanup that later, since it needs
some more testing.
@magicus, thanks a lot for extracting/reworking/cleaning-up the static Java
changes from the hermetic Java branch. That's a substantial amount of work!
I have one quick comment about the removal of `STATIC_LIB_EXCLUDE_OBJS`
changes. Will post it as separate comment for the related code.
I'll also look closely of the vm & jdk changes and compare those with the
changes in the hermetic Java branch this week.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1648283151
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Wed, 19 Jun 2024 02:05:28 GMT, lingjun-cg wrote: > It requires append(int), but the Appendable has no such method. If via `Appendable` we don't have access to `append(int)`, can we simply `append(String.valueOf(int))`. And similarly for `append(char[], int, int)`, can we `append(String.valueOf(char[], int, int))`. According to the method descriptions in `AbstractStringBuilder`, this should behaviorally be the same. That way we don't have to add any new classes, and can continue with the generic implementation. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2181620101
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v3]
On Thu, 20 Jun 2024 19:47:56 GMT, Naoto Sato wrote: >> I see your point. CLDR takes precedence over ISO4217 regarding currencies >> here? > > That is more consistent IMO CLDR is aware of this change but hasn't added it yet. We're mid cycle so it won't be much more than metadata this round due to timing. https://unicode-org.atlassian.net/issues/CLDR-17751 if you want to track. - PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1648094193
[jdk23] RFR: 8333854: IllegalAccessError with proxies after JDK-8332457
Please review this patch, which is a backport of the fix in #19615 to JDK 23. This is not a clean patch, because the old patch was done on JDK-8333479 (#19585) which was absent in JDK 23; however, the conflicts were small, and the only real changes were that `methodTypeDesc` and `classDesc` from `ConstantUtils` were not available, so the original approaches were retained. There is also import adjustments. Testing: tier 1-3 - Commit messages: - 8333854: IllegalAccessError with proxies after JDK-8332457 Changes: https://git.openjdk.org/jdk/pull/19815/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19815&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8333854 Stats: 238 lines in 2 files changed: 161 ins; 9 del; 68 mod Patch: https://git.openjdk.org/jdk/pull/19815.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19815/head:pull/19815 PR: https://git.openjdk.org/jdk/pull/19815
Re: RFR: 8334653: ISO 4217 Amendment 177 Update
On Thu, 20 Jun 2024 19:51:51 GMT, Naoto Sato wrote: > The point I wanted to make is that the text file only contains the "list one" > part of the ISO 4217. Renamed the file name to "ISO4217-list-one.txt" - PR Comment: https://git.openjdk.org/jdk/pull/19813#issuecomment-2181478826
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v3]
> Please review this PR which incorporates the ISO 4217 Amendment 177 Update. > > Specifically, the introduction of the new currency, Zimbabwe Gold. Justin Lu has updated the pull request incrementally with one additional commit since the last revision: reflect review: change txt name - Changes: - all: https://git.openjdk.org/jdk/pull/19813/files - new: https://git.openjdk.org/jdk/pull/19813/files/761d8efc..58c09005 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19813&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19813&range=01-02 Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19813.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19813/head:pull/19813 PR: https://git.openjdk.org/jdk/pull/19813
[jdk23] RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed
844: JMX attaching of Subject does not work when security manager not allowed - Commit messages: - Backport bcf4bb4882e06d8c52f6eb4e9c4e027ba0622c5f Changes: https://git.openjdk.org/jdk/pull/19810/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19810&range=00 Issue: https://bugs.openjdk.org/browse/JDK-844 Stats: 162 lines in 17 files changed: 113 ins; 3 del; 46 mod Patch: https://git.openjdk.org/jdk/pull/19810.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19810/head:pull/19810 PR: https://git.openjdk.org/jdk/pull/19810
Re: RFR: 8334653: ISO 4217 Amendment 177 Update
On Thu, 20 Jun 2024 19:01:29 GMT, Justin Lu wrote: > > It's not about this update, but it may be better to rename the > > `tablea1.txt` data file to `list-one.txt` or something along the line. The > > name `tablea1` is obsolete, and it is now called "List one: Currency, fund > > and precious metal codes" > > https://www.six-group.com/en/products-services/financial-information/data-standards.html > > Without further context, it is hard to infer meaning from `tablea1.txt` or > `list-one.txt`. I have renamed it to `ISO4217-codes.txt`, which I think is > much more straight forward and provides immediate info. The point I wanted to make is that the text file only contains the "list one" part of the ISO 4217. Looks like "list two" is a subset of list one (as they are funds codes), but "list three", which lists historic codes, is definitely not covered in that file. I am ok with the suggested name, but then we should include some comments mentioning this. - PR Comment: https://git.openjdk.org/jdk/pull/19813#issuecomment-2181418086
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v2]
On Thu, 20 Jun 2024 19:01:26 GMT, Justin Lu wrote: >> src/java.base/share/classes/sun/util/resources/CurrencyNames.properties line >> 516: >> >>> 514: zmk=Zambian Kwacha >>> 515: zwd=Zimbabwean Dollar (1980-2008) >>> 516: zwg=Zimbabwe Gold >> >> It's interesting to see the difference `Zimbabwe` and `Zimbabwean` here. The >> latter is from CLDR and the name for ZWG is not yet available, so we may >> need to pull the name from CLDR later. > > I see your point. CLDR takes precedence over ISO4217 regarding currencies > here? That is more consistent IMO - PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1648054882
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: add extra test for missing root modules - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/75c9a6af..a4723494 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=05-06 Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 10:00:33 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
A `jdk.internal.access.StringBuf` that `AbstractStringBuilder` implements may
work, right?
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2181402131
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee wrote: >> This PR adds a new JDK tool, called `jnativescan`, that can be used to find >> code that accesses native functionality. Currently this includes `native` >> method declarations, and methods marked with `@Restricted`. >> >> The tool accepts a list of class path and module path entries through >> `--class-path` and `--module-path`, and a set of root modules through >> `--add-modules`, as well as an optional target release with `--release`. >> >> The default mode is for the tool to report all uses of `@Restricted` >> methods, and `native` method declaration in a tree-like structure: >> >> >> app.jar (ALL-UNNAMED): >> main.Main: >> main.Main::main(String[])void references restricted methods: >> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment >> main.Main::m()void is a native method declaration >> >> >> The `--print-native-access` option can be used print out all the module >> names of modules doing native access in a comma separated list. For class >> path code, this will print out `ALL-UNNAMED`. >> >> Testing: >> - `langtools_jnativescan` tests. >> - Running the tool over jextract's libclang bindings, which use the FFM API, >> and thus has a lot of references to `@Restricted` methods. >> - tier 1-3 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > update man page header to be consisten with the others I've massaged the parsing code to where the help message now looks like this: ... Option Description -- --- -?, -h, --help help --add-modules List of root modules to scan --class-path The class path as used at runtime --module-path The module path as used at runtime --print-native-access print a comma separated list of modules that may perform native access operations. ALL-UNNAMED is used to indicate unnamed modules. --release The runtime version that will run the application --version Print version information and exit I'm not sure if joptsimple has a way to display the option arguments like `[,]` to indicate multiple options (at least I couldn't find it immediately). - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181397654
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v6]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: review comments Alan - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/06f53e31..75c9a6af Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=04-05 Stats: 98 lines in 2 files changed: 59 ins; 26 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v3]
On Thu, 6 Jun 2024 12:46:36 GMT, jengebr wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to size zero, or are otherwise maintained empty, so cloning is unnecessary. >> >> Results from the included JMH benchmark: >> Before: >> >> BenchmarkMode Cnt >> Score Error Units >> CopyOnWriteArrayListBenchmark.clear avgt5 >> 74.487 ± 1.793 ns/op >> CopyOnWriteArrayListBenchmark.clearEmpty avgt5 >> 27.918 ± 0.759 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 >> 16.656 ± 0.375 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 >> 15.415 ± 0.489 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 >> 21.608 ± 0.363 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 >> 15.374 ± 0.260 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 >> 15.688 ± 0.350 ns/op >> CopyOnWriteArrayListBenchmark.readInstance avgt 10 >> 2625.125 ± 71.802 ns/op >> CopyOnWriteArrayListBenchmark.readInstanceEmpty avgt 10 >> 2607.447 ± 46.400 ns/op >> >> >> After: >> >> BenchmarkMode Cnt >> Score Error Units >> CopyOnWriteArrayListBenchmark.clear avgt5 >> 75.365 ± 2.092 ns/op >> CopyOnWriteArrayListBenchmark.clearEmpty avgt5 >> 20.803 ± 0.539 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 >> 16.808 ± 0.582 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 >> 12.980 ± 0.418 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 >> 21.627 ± 0.173 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 >> 12.864 ± 0.408 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 >> 12.931 ± 0.255 ns/op >> CopyOnWriteArrayListBenchmark.readInstance avgt 10 >> 2615.500 ± 30.771 ns/op >> CopyOnWriteArrayListBenchmark.readInstanceEmpty avgt 10 >> 2583.892 ± 62.086 ns/op > > jengebr has updated the pull request incrementally with one additional commit > since the last revision: > > Adding benchmarks for readObject Thank you! I've expanded coverage to remove(int) as well. > One thing that surprises me is the change to remove(Object) to handle the > case where the last remaining element is removed. Does that help the > application that prompted this change? Part of me wonders if remove(int) > needs to do the same as someone will show up sometime to ask. The application benefits most from the lower footprint of the empty constructors, but I tried to expand the optimization throughout the class. Clearly I missed a spot, thank you for catching it. - PR Comment: https://git.openjdk.org/jdk/pull/19527#issuecomment-2181366938
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v4]
> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless > cloning of Object[0] instances. This cloning is intended to prevent callers > from changing array contents, but many `CopyOnWriteArrayList`s are allocated > to size zero, or are otherwise maintained empty, so cloning is unnecessary. > > Results from the included JMH benchmark: > Before: > > BenchmarkMode Cnt > Score Error Units > CopyOnWriteArrayListBenchmark.clear avgt5 > 74.487 ± 1.793 ns/op > CopyOnWriteArrayListBenchmark.clearEmpty avgt5 > 27.918 ± 0.759 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 > 16.656 ± 0.375 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 > 15.415 ± 0.489 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 > 21.608 ± 0.363 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 > 15.374 ± 0.260 ns/op > CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 > 15.688 ± 0.350 ns/op > CopyOnWriteArrayListBenchmark.readInstance avgt 10 > 2625.125 ± 71.802 ns/op > CopyOnWriteArrayListBenchmark.readInstanceEmpty avgt 10 > 2607.447 ± 46.400 ns/op > > > After: > > BenchmarkMode Cnt > Score Error Units > CopyOnWriteArrayListBenchmark.clear avgt5 > 75.365 ± 2.092 ns/op > CopyOnWriteArrayListBenchmark.clearEmpty avgt5 > 20.803 ± 0.539 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 > 16.808 ± 0.582 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 > 12.980 ± 0.418 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 > 21.627 ± 0.173 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 > 12.864 ± 0.408 ns/op > CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 > 12.931 ± 0.255 ns/op > CopyOnWriteArrayListBenchmark.readInstance avgt 10 > 2615.500 ± 30.771 ns/op > CopyOnWriteArrayListBenchmark.readInstanceEmpty avgt 10 > 2583.892 ± 62.086 ns/op jengebr has updated the pull request incrementally with one additional commit since the last revision: Expanding coverage of remove() - Changes: - all: https://git.openjdk.org/jdk/pull/19527/files - new: https://git.openjdk.org/jdk/pull/19527/files/b1920f7a..2ff93ab6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19527&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19527&range=02-03 Stats: 35 lines in 2 files changed: 35 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19527.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19527/head:pull/19527 PR: https://git.openjdk.org/jdk/pull/19527
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v2]
On Thu, 20 Jun 2024 18:21:41 GMT, Naoto Sato wrote: >> Justin Lu has updated the pull request incrementally with one additional >> commit since the last revision: >> >> reflect review comments > > src/java.base/share/classes/sun/util/resources/CurrencyNames.properties line > 516: > >> 514: zmk=Zambian Kwacha >> 515: zwd=Zimbabwean Dollar (1980-2008) >> 516: zwg=Zimbabwe Gold > > It's interesting to see the difference `Zimbabwe` and `Zimbabwean` here. The > latter is from CLDR and the name for ZWG is not yet available, so we may need > to pull the name from CLDR later. I see your point. CLDR takes precedence over ISO4217 regarding currencies here? > src/java.base/share/data/currency/CurrencyData.properties line 585: > >> 583: ZM=ZMW >> 584: # ZIMBABWE >> 585: ZW=ZWL;2024-08-31-22-00-00;ZWG > > JDK switches the currency at the beginning of the transition period. So in > this case, it's 6/25. So we don't need to even specify the transition at all, > as the next build drop is beyond that date. Good to know. Removed the cutover dates. - PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1648013380 PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1648013361
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v2]
> Please review this PR which incorporates the ISO 4217 Amendment 177 Update. > > Specifically, the introduction of the new currency, Zimbabwe Gold. Justin Lu has updated the pull request incrementally with one additional commit since the last revision: reflect review comments - Changes: - all: https://git.openjdk.org/jdk/pull/19813/files - new: https://git.openjdk.org/jdk/pull/19813/files/37d1bed4..761d8efc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19813&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19813&range=00-01 Stats: 11 lines in 5 files changed: 1 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/19813.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19813/head:pull/19813 PR: https://git.openjdk.org/jdk/pull/19813
Re: RFR: 8334653: ISO 4217 Amendment 177 Update
On Thu, 20 Jun 2024 18:36:22 GMT, Naoto Sato wrote: > It's not about this update, but it may be better to rename the `tablea1.txt` > data file to `list-one.txt` or something along the line. The name `tablea1` > is obsolete, and it is now called "List one: Currency, fund and precious > metal codes" > https://www.six-group.com/en/products-services/financial-information/data-standards.html Without further context, it is hard to infer meaning from `tablea1.txt` or `list-one.txt`. I have renamed it to `ISO4217-codes.txt`, which I think is much more straight forward and provides immediate info. - PR Comment: https://git.openjdk.org/jdk/pull/19813#issuecomment-2181344921
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Tue, 18 Jun 2024 10:00:33 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
I did not mean to introduce a public API for this change (if we do, the fix
cannot be backported). I thought we could define a package private one, but
based on your observation, it may get messier... So I agree that falling back
to `StringBuf` is the way to go, IMO.
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2181339942
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 16:13:04 GMT, Alan Bateman wrote: > Another thing is that using joptsimple gives up a bit of control, e.g. the > help output shows the parameter for --class-path as `` where the java > launcher and other tools will show "path" or "class path". Same thing with > `--release` where it shows `` where jar, javac, and other tools will > say "release". Not important for now, just comments from trying out the tool. It seems that it is possible in joptsimple to attach a description to the argument as well, and get something like: --module-pathThe class path as used at runtime It looks like we could also customize the argument type and get something like: --module-path (we'd just need to add a `ModulePath` class with a `valueOf(String)` method) - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181315973
RFR: 8334441: Mark tests in jdk_security_infra group as manual
Updated all the tests that depend on external infrastructure services as manual. These tests may fail with external reasons, for instance - change in CA test portal, certificate status updates, or network issues. - Commit messages: - Update comments - comment explaining why these are manual - update ProblemList - 8334441: Mark tests in jdk_security_infra group as manual Changes: https://git.openjdk.org/jdk/pull/19814/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19814&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8334441 Stats: 160 lines in 10 files changed: 5 ins; 2 del; 153 mod Patch: https://git.openjdk.org/jdk/pull/19814.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19814/head:pull/19814 PR: https://git.openjdk.org/jdk/pull/19814
Re: RFR: 8334653: ISO 4217 Amendment 177 Update
On Thu, 20 Jun 2024 18:01:39 GMT, Justin Lu wrote: > Please review this PR which incorporates the ISO 4217 Amendment 177 Update. > > Specifically, the introduction of the new currency, Zimbabwe Gold. It's not about this update, but it may be better to rename the `tablea1.txt` data file to `list-one.txt` or something along the line. The name `tablea1` is obsolete, and it is now called "List one: Currency, fund and precious metal codes" https://www.six-group.com/en/products-services/financial-information/data-standards.html - PR Comment: https://git.openjdk.org/jdk/pull/19813#issuecomment-2181303334
Re: RFR: 8334653: ISO 4217 Amendment 177 Update
On Thu, 20 Jun 2024 18:01:39 GMT, Justin Lu wrote: > Please review this PR which incorporates the ISO 4217 Amendment 177 Update. > > Specifically, the introduction of the new currency, Zimbabwe Gold. src/java.base/share/classes/sun/util/resources/CurrencyNames.properties line 516: > 514: zmk=Zambian Kwacha > 515: zwd=Zimbabwean Dollar (1980-2008) > 516: zwg=Zimbabwe Gold It's interesting to see the difference `Zimbabwe` and `Zimbabwean` here. The latter is from CLDR and the name for ZWG is not yet available, so we may need to pull the name from CLDR later. src/java.base/share/data/currency/CurrencyData.properties line 585: > 583: ZM=ZMW > 584: # ZIMBABWE > 585: ZW=ZWL;2024-08-31-22-00-00;ZWG JDK switches the currency at the beginning of the transition period. So in this case, it's 6/25. So we don't need to even specify the transition at all, as the next build drop is beyond that date. test/jdk/java/util/Currency/tablea1.txt line 279: > 277: YE YER 886 2 > 278: ZM ZMW 967 2 > 279: ZW ZWL 932 2 2024-08-31-22-00-00 ZWG 924 > 2 Same as above - PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1647972810 PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1647967701 PR Review Comment: https://git.openjdk.org/jdk/pull/19813#discussion_r1647967861
Integrated: 8334333: MissingResourceCauseTestRun.java fails if run by root
On Sat, 15 Jun 2024 09:56:53 GMT, SendaoYan wrote: > Hi all, > Testcase > `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTestRun.java` > run fails with root user privileged. I think it's necessary to skip this > testcase when user is root. > Why run the jtreg test by root user? It's because during rpmbuild process for > linux distribution of JDK, root user is the default user to build the > openjdk, also is the default user to run the `make test-tier1`, this PR make > this testcase more robustness. > The change has been verified, only change the testcase, no risk. This pull request has now been integrated. Changeset: de8ee977 Author:SendaoYan Committer: Naoto Sato URL: https://git.openjdk.org/jdk/commit/de8ee97718d7e12b541b310cf5b67f3e10e91ad9 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod 8334333: MissingResourceCauseTestRun.java fails if run by root Reviewed-by: naoto, jlu - PR: https://git.openjdk.org/jdk/pull/19732
RFR: 8334653: ISO 4217 Amendment 177 Update
Please review this PR which incorporates the ISO 4217 Amendment 177 Update. Specifically, the introduction of the new currency, Zimbabwe Gold. - Commit messages: - init Changes: https://git.openjdk.org/jdk/pull/19813/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19813&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8334653 Stats: 14 lines in 4 files changed: 2 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/19813.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19813/head:pull/19813 PR: https://git.openjdk.org/jdk/pull/19813
Withdrawn: 8329760: Add indexOf(Predicate filter) to java.util.List interface
On Fri, 5 Apr 2024 00:00:58 GMT, Evemose wrote:
> **Subject**
> Addition of Predicate-based `indexOf` and `lastIndexOf` methods to
> `java.util.List`
>
> **Motivation**
> The motivation behind this proposal is to enhance the functionality of the
> `List` interface by providing a more flexible way to find the index of an
> element. Currently, the `indexOf` and `lastIndexOf` methods only accept an
> object as a parameter. This limits the flexibility of these methods as they
> can only find the index of exact object matches.
>
> The proposed methods would accept a `Predicate` as a parameter, allowing
> users to define a condition that the desired element must meet. This would
> provide a more flexible and powerful way to find the index of an element in a
> list.
>
> Here is a brief overview of the changes made in this pull request:
>
> 1. Added the `indexOf(Predicate filter)` method to the `List`
> interface.
> 2. Added the `lastIndexOf(Predicate filter)` method to the `List`
> interface.
> 3. Implemented these methods in all non-abstract classes that implement the
> `List` interface.
>
> The changes have been thoroughly tested to ensure they work as expected and
> do not introduce any regressions. The test cases cover a variety of scenarios
> to ensure the robustness of the implementation.
>
> For example, consider the following test case:
>
> List list = new ArrayList<>();
> list.add("Object one");
> list.add("NotObject two");
> list.add("NotObject three");
>
> int index1 = list.indexOf(s -> s.contains("ct t"));
> System.out.println(index1); // Expected output: 1
> int index2 = list.lastIndexOf(s -> s.startsWith("NotObject"));
> System.out.println(index2); // Expected output: 2
>
>
> Currently, to achieve the same result, we would have to use a more verbose
> approach:
>
> int index1 = IntStream.range(0, list.size())
> .filter(i -> list.get(i).contains("ct t"))
> .findFirst()
> .orElse(-1);
> System.out.println(index1); // Output: 1
> int index2 = IntStream.range(0, list.size())
> .filter(i -> list.get(i).startsWith("NotObject"))
> .reduce((first, second) -> second)
> .orElse(-1);
> System.out.println(index2); // Output: 2
>
>
> I believe these additions would greatly enhance the functionality and
> flexibility of the `List` interface, making it more powerful and
> user-friendly. I look forward to your feedback and am open to making any
> necessary changes based on your suggestions.
>
> Thank you for considering this proposal.
>
> Best regards
>
> PS: In ...
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.org/jdk/pull/18639
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 17:40:25 GMT, Alan Bateman wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update man page header to be consisten with the others > > src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java > line 113: > >> 111: // Class-Path attribute specifies that jars that >> 112: // are not found are simply ignored. Do the same >> here >> 113: classPathJars.offer(otherJar); > > The expansion of the class path looks right but the Class-Path attribute is > awkward to deal with as its relative URI rather than a file path. More on > this this later. Not important for now but the expansion will probably need to a "visited" set to detect cycles (yes, it is possible). - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647934012
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee wrote: >> This PR adds a new JDK tool, called `jnativescan`, that can be used to find >> code that accesses native functionality. Currently this includes `native` >> method declarations, and methods marked with `@Restricted`. >> >> The tool accepts a list of class path and module path entries through >> `--class-path` and `--module-path`, and a set of root modules through >> `--add-modules`, as well as an optional target release with `--release`. >> >> The default mode is for the tool to report all uses of `@Restricted` >> methods, and `native` method declaration in a tree-like structure: >> >> >> app.jar (ALL-UNNAMED): >> main.Main: >> main.Main::main(String[])void references restricted methods: >> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment >> main.Main::m()void is a native method declaration >> >> >> The `--print-native-access` option can be used print out all the module >> names of modules doing native access in a comma separated list. For class >> path code, this will print out `ALL-UNNAMED`. >> >> Testing: >> - `langtools_jnativescan` tests. >> - Running the tool over jextract's libclang bindings, which use the FFM API, >> and thus has a lot of references to `@Restricted` methods. >> - tier 1-3 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > update man page header to be consisten with the others src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line 113: > 111: // Class-Path attribute specifies that jars that > 112: // are not found are simply ignored. Do the same here > 113: classPathJars.offer(otherJar); The expansion of the class path looks right but the Class-Path attribute is awkward to deal with as its relative URI rather than a file path. More on this this later. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647929185
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v6]
On Thu, 20 Jun 2024 12:06:43 GMT, Severin Gehwolf wrote:
>> Please review this enhancement to the container detection code which allows
>> it to figure out whether the JVM is actually running inside a container
>> (`podman`, `docker`, `crio`), or with some other means that enforces
>> memory/cpu limits by means of the cgroup filesystem. If neither of those
>> conditions hold, the JVM runs in not containerized mode, addressing the
>> issue described in the JBS tracker. For example, on my Linux system
>> `is_containerized() == false" is being indicated with the following trace
>> log line:
>>
>>
>> [0.001s][debug][os,container] OSContainer::init: is_containerized() = false
>> because no cpu or memory limit is present
>>
>>
>> This state is being exposed by the Java `Metrics` API class using the new
>> (still JDK internal) `isContainerized()` method. Example:
>>
>>
>> java -XshowSettings:system --version
>> Operating System Metrics:
>> Provider: cgroupv1
>> System not containerized.
>> openjdk 23-internal 2024-09-17
>> OpenJDK Runtime Environment (fastdebug build
>> 23-internal-adhoc.sgehwolf.jdk-jdk)
>> OpenJDK 64-Bit Server VM (fastdebug build
>> 23-internal-adhoc.sgehwolf.jdk-jdk, mixed mode, sharing)
>>
>>
>> The basic property this is being built on is the observation that the cgroup
>> controllers typically get mounted read only into containers. Note that the
>> current container tests assert that `OSContainer::is_containerized() ==
>> true` in various tests. Therefore, using the heuristic of "is any memory or
>> cpu limit present" isn't sufficient. I had considered that in an earlier
>> iteration, but many container tests failed.
>>
>> Overall, I think, with this patch we improve the current situation of
>> claiming a containerized system being present when it's actually just a
>> regular Linux system.
>>
>> Testing:
>>
>> - [x] GHA (risc-v failure seems infra related)
>> - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2
>> (including gtests)
>> - [x] Some manual testing using cri-o
>>
>> Thoughts?
>
> Severin Gehwolf has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Remove problem listing of PlainRead which is reworked here
Seems okay. I don't understand the depths of V1 vs V2 controller files as well
as you do, @jerboaa , but I trust you there. The mechanics seem fine.
src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 373:
> 371: * (11) mount source:matched with '%*s' and discarded
> 372: * (12) super options: matched with '%*s' and discarded
> 373: */
Thanks for the good comment. That scanf line is a brain teaser.
src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 422:
> 420: * (12) super options: matched with '%s' and captured in
> 'tmpcgroups'
> 421: */
> 422: if (sscanf(p, "%*d %*d %*d:%*d %s %s %s%*[^-]- %s %*s %s", tmproot,
> tmpmount, mount_opts, tmp_fs_type, tmpcgroups) == 5) {
The only difference to v1 is that we parse the super options (12), right? Could
we factor out the parsing into a helper function? Or, alternatively, at least
`#define` the scanf format somewhere up top, including the nice comment, and
reuse that format string?
-
Marked as reviewed by stuefe (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18201#pullrequestreview-2130943896
PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1647881202
PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1647925120
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Wed, 19 Jun 2024 21:13:33 GMT, Maurizio Cimadamore wrote: >> What do you suggest? Just a note in the error message that exploded >> modules/class paths are not supported? > > Something like that yes An altermative is to use ResolvedModule::reference to get a ModuleReference, then use its open method to open the contents of the module to get a ModuleReader. That will give you a stream over the names of all resources in the module. It will work for all exploded and packaged modules. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647911987
Integrated: 8334490: Normalize string with locale invariant `toLowerCase()`
On Tue, 18 Jun 2024 18:23:12 GMT, Naoto Sato wrote: > The test library `jdk.test.lib.Platform` uses no-arg `toLowerCase()` for > string comparison, which should be avoided. This pull request has now been integrated. Changeset: 265a0f55 Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/265a0f5547d0ddb220391aef679c122768f02a00 Stats: 7 lines in 1 file changed: 1 ins; 0 del; 6 mod 8334490: Normalize string with locale invariant `toLowerCase()` Reviewed-by: jlu, dfuchs, lancea, rriggs - PR: https://git.openjdk.org/jdk/pull/19775
Re: RFR: 8334490: Normalize string with locale invariant `toLowerCase()`
On Tue, 18 Jun 2024 18:23:12 GMT, Naoto Sato wrote: > The test library `jdk.test.lib.Platform` uses no-arg `toLowerCase()` for > string comparison, which should be avoided. Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/19775#issuecomment-2181150302
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Tue, 18 Jun 2024 16:35:10 GMT, Jorn Vernee wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update man page header to be consisten with the others
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
> line 93:
>
>> 91: }
>> 92:
>> 93: public PlatformDescription getPlatformTrusted(String platformName) {
>
> I noticed that `getPlatform` was not throwing an exception if the release was
> not valid, which then later results in an NPE. I've added an explicit check
> here instead. The caller can then catch the exception.
I see the "options" arg is not used so maybe another approach here is a one-arg
getPlatform that throws PlatformNotSupported and then migrate the other usages
at another time.
(This is just a passing comment, not important for the core of this feature).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647877795
Integrated: 8333358: java/io/IO/IO.java test fails intermittently
On Mon, 10 Jun 2024 12:29:32 GMT, Pavel Rappo wrote: > Please review this fix for an intermittent test failure. > > On some configurations, the default `expect` timeout of 10 seconds is > insufficient. It is increased to 20; it's hard to imagine a configuration for > which that new value would still be insufficient, but we'll see. > > Aside from that, test-generated diagnostics are improved: the version of the > `expect` command and the duration of each test method are recorded. > `output.exp` is modified for robustness and clear indication of the timeout > condition. > > I was reminded, out-of-band, that test timeouts should be scaled with > `test.timeout.factor`. I propose to integrate this PR first and then > separately update all the affected tests to scale their `expect` timeouts. This pull request has now been integrated. Changeset: 1b1dba80 Author:Pavel Rappo URL: https://git.openjdk.org/jdk/commit/1b1dba8082969244effa86ac03c6053b3b0ddc43 Stats: 69 lines in 3 files changed: 63 ins; 2 del; 4 mod 858: java/io/IO/IO.java test fails intermittently Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/19627
Re: RFR: 8333358: java/io/IO/IO.java test fails intermittently [v5]
On Thu, 20 Jun 2024 09:56:45 GMT, Pavel Rappo wrote: >> Please review this fix for an intermittent test failure. >> >> On some configurations, the default `expect` timeout of 10 seconds is >> insufficient. It is increased to 20; it's hard to imagine a configuration >> for which that new value would still be insufficient, but we'll see. >> >> Aside from that, test-generated diagnostics are improved: the version of the >> `expect` command and the duration of each test method are recorded. >> `output.exp` is modified for robustness and clear indication of the timeout >> condition. >> >> I was reminded, out-of-band, that test timeouts should be scaled with >> `test.timeout.factor`. I propose to integrate this PR first and then >> separately update all the affected tests to scale their `expect` timeouts. > > Pavel Rappo 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 six additional > commits since the last revision: > > - Merge branch 'master' into 858 > - Merge branch 'master' into 858 > - Add a comment for future maintainers > - Disable timeout in expect scripts > - Change url as suggested in review > - Initial commit Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19627#pullrequestreview-2130880264
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update man page header to be consisten with the others
`jnativescan --version` prints the value of Runtime.version where jdeprscan and
some of the other tools print System.getProperty("java.version"). Just
something to check as they might look inconsistent.
-
PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181075350
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee wrote: >> This PR adds a new JDK tool, called `jnativescan`, that can be used to find >> code that accesses native functionality. Currently this includes `native` >> method declarations, and methods marked with `@Restricted`. >> >> The tool accepts a list of class path and module path entries through >> `--class-path` and `--module-path`, and a set of root modules through >> `--add-modules`, as well as an optional target release with `--release`. >> >> The default mode is for the tool to report all uses of `@Restricted` >> methods, and `native` method declaration in a tree-like structure: >> >> >> app.jar (ALL-UNNAMED): >> main.Main: >> main.Main::main(String[])void references restricted methods: >> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment >> main.Main::m()void is a native method declaration >> >> >> The `--print-native-access` option can be used print out all the module >> names of modules doing native access in a comma separated list. For class >> path code, this will print out `ALL-UNNAMED`. >> >> Testing: >> - `langtools_jnativescan` tests. >> - Running the tool over jextract's libclang bindings, which use the FFM API, >> and thus has a lot of references to `@Restricted` methods. >> - tier 1-3 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > update man page header to be consisten with the others One thing to check is usages like `jnativescan foo`, should that fail as the tool doesn't take args. Another thing is that using joptsimple gives up a bit of control, e.g. the help output shows the parameter for --class-path as `` where the java launcher and other tools will show "path" or "class path". Same thing with `--release` where it shows `` where jar, javac, and other tools will say "release". Not important for now, just comments from trying out the tool. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181070909
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Thu, 20 Jun 2024 11:38:26 GMT, Jorn Vernee wrote: >> src/jdk.jdeps/share/man/jnativescan.1 line 121: >> >>> 119: .TP >>> 120: \f[V]--release\f[R] \f[I]version\f[R] >>> 121: Used to specify the Java SE release that specifies the set of >>> restricted >> >> In principle, the release could also affect which methods are native or not? > > Yes, but we don't warn on `native` method references, only on declarations. > So, it would only affect which methods might be `native` in the user code. > But I think that is already implied by the note on multi-release jars? You are right... declarations inside the JDK are... inside the JDK - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647820666
Re: RFR: 8325525: Create jtreg test case for JDK-8325203
On Tue, 4 Jun 2024 07:13:15 GMT, Vanitha B P wrote:
> Created jtreg test case for
> [JDK-8325203](https://bugs.openjdk.org/browse/JDK-8325203) issue.
>
> The JpackageTest created tests that the child process started from the app
> launched by jpackage launcher is not automatically terminated when the the
> launcher is terminated.
Changes requested by asemenyuk (Reviewer).
The test builds an installer that must be installed as a part of the test. This
will not work in an environment where tests are executed under a user account
without restricted permissions. Building app image instead of an installer is
sufficient for this test,
There are helper classes for writing jpackage tests in
https://github.com/openjdk/jdk/tree/master/test/jdk/tools/jpackage/helpers/jdk/jpackage/test
folder.
I'd not use a file to communicate PID of the started process between the
launcher and the test. I'd use stdout instead:
public class ThirdPartyAppLauncher {
public static void main(String[] args) throws IOException {
Process process = new ProcessBuilder("regedit").start();
System.out.println("RegEdit PID=" + process.pid());
}
}
Compiling, jarring, and running jpackage that will create app image from the
jar with these helpers would be as follows:
JPackageCommand.helloAppImage(TKit.TEST_SRC_ROOT.resolve("apps/ThirdPartyAppLauncher.java")
+ "*Hello").executeAndAssertImageCreated();
Then you run "ThirdPartyAppLauncher" class using jpackage launcher and capture
its stdout:
String pidStr = new
Executor().saveOutput().dumpOutput().setExecutable(cmd.appLauncherPath().toAbsolutePath()).execute(0).getFirstLineOfOutput();
// parse regedit PID
long regeditPid = Long.parseLong(pidStr.split("=", 2)[1]);
// Run your checks
...
// Kill only a specific regedit instance
Runtime.getRuntime().exec("taskkill /F /PID " + regeditPid);
You may use one of the existing jpackage tests for the reference
(https://github.com/openjdk/jdk/blob/master/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java
is a good example)
test/jdk/tools/jpackage/windows/JpackageTest.java line 160:
> 158: */
> 159: private void closeThirdPartyApplication() throws Throwable {
> 160: Runtime.getRuntime().exec("taskkill /F /IM regedit.exe");
I guess this will kill all regedit processes including those not started by the
test. This doesn't seem right.
-
PR Review: https://git.openjdk.org/jdk/pull/19536#pullrequestreview-2130771716
PR Comment: https://git.openjdk.org/jdk/pull/19536#issuecomment-2181016935
PR Review Comment: https://git.openjdk.org/jdk/pull/19536#discussion_r1647775863
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v4]
On Thu, 20 Jun 2024 10:54:49 GMT, Emanuel Peter wrote:
>> I'm not opposed to accepting this patch as-is, but I think we should do so
>> with an eye towards reverting if we figure out a way to improve the
>> `putChar` intrinsic so that it doesn't block merge store optimization. What
>> do you think @eme64?
>>
>> As the need for the changes in
>> [b5ad8e7](https://github.com/openjdk/jdk/pull/19626/commits/b5ad8e70928c547d134d0e4a532441cad9a7e4a2)
>> showed added code complexity (like here) can be detrimental to performance,
>> and if the `putChar` can be improved we might see benefits in more places.
>
> @cl4es @wenshao I think we should probably work on `putChar`, or at least
> figure out what blocks `MergeStore` for `putChar`. Can someone produce a
> simple stand-alone `.java` file that uses `putChar`, so that I can
> investigate why `MergeStore` does not work for it?
>
> Otherwise, I agree with @cl4es : the code here may be ok for now, but we
> would have to revert it again later, should `MergeStore` eventually do the
> trick.
@eme64
simple stand-alone java
import jdk.internal.misc.Unsafe;
public class PutCharTest {
static final Unsafe UNSAFE = Unsafe.getUnsafe();
static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int
c4) {
putChar(val, index, (char)(c1));
putChar(val, index + 1, (char)(c2));
putChar(val, index + 2, (char)(c3));
putChar(val, index + 3, (char)(c4));
}
static void putChar(byte[] bytes, int index, int c) {
UNSAFE.putChar(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + (index << 1),
(char)(c));
}
static void putChar0(byte[] bytes, int index, int c) {
UNSAFE.putByte(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + (index << 1),
(byte)(c));
UNSAFE.putByte(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + (index << 1) + 1,
(byte)(c << 8));
}
static void putNull(byte[] bytes, int index) {
putCharsAt(bytes, index, 'n', 'u', 'l', 'l');
}
public static void main(String[] args) {
for (int i = 0; i < 5; i++) {
testNull();
}
System.out.println("done");
}
private static void testNull() {
byte[] bytes = new byte[8192];
for (int i = 0; i < 1000; i++) {
int index = 0;
for (int j = 0; j < 1024; j++) {
putNull(bytes, index);
index += 4;
}
}
}
}
-
PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2180862537
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:51:22 GMT, Evemose wrote: > wouldn't it be better to create one uniform tool See my reply here: https://github.com/openjdk/jdk/pull/19774#issuecomment-2179078565 - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2180653743
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows The changes to the launcher look okay. The move from `ifdef STATIC_BUILD` to `JLI_IsStaticallyLinked` is quite nice. Having libjdwp link to libjvm was a surprise but I think okay. I think it would be useful to provide a brief summary on the when/where the static builds are tested to ensure that the changes don't bit rot. I realise we already have static builds but it isn't obvious where this is tested. src/hotspot/share/runtime/linkType.cpp line 36: > 34: return JNI_TRUE; > 35: #else > 36: return JNI_FALSE; bool != jboolean, I assume you don't want that. The naming is a bit unusual, a function that returns a boolean is usually name is_XXX, but maybe there is reason for this? - PR Comment: https://git.openjdk.org/jdk/pull/19478#issuecomment-2180635747 PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1647480992
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee wrote: >> This PR adds a new JDK tool, called `jnativescan`, that can be used to find >> code that accesses native functionality. Currently this includes `native` >> method declarations, and methods marked with `@Restricted`. >> >> The tool accepts a list of class path and module path entries through >> `--class-path` and `--module-path`, and a set of root modules through >> `--add-modules`, as well as an optional target release with `--release`. >> >> The default mode is for the tool to report all uses of `@Restricted` >> methods, and `native` method declaration in a tree-like structure: >> >> >> app.jar (ALL-UNNAMED): >> main.Main: >> main.Main::main(String[])void references restricted methods: >> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment >> main.Main::m()void is a native method declaration >> >> >> The `--print-native-access` option can be used print out all the module >> names of modules doing native access in a comma separated list. For class >> path code, this will print out `ALL-UNNAMED`. >> >> Testing: >> - `langtools_jnativescan` tests. >> - Running the tool over jextract's libclang bindings, which use the FFM API, >> and thus has a lot of references to `@Restricted` methods. >> - tier 1-3 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > update man page header to be consisten with the others Just as a guest here. Just theoretically, wouldn't it be better to create one uniform tool to detect all "undesired" APIs. This would be more flexible, and potentially could be adapted to use some kind of "meticulous" mode, that will also indicate usage of public APIs, that, while never will be removed, are recommended to remove in favor of new alternatives (like old Date API or old collections". Just a thought btw, not a proposal for now - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2180594098
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: update man page header to be consisten with the others - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/4c6abddf..06f53e31 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v2]
On Tue, 16 Apr 2024 18:25:52 GMT, Thomas Stuefe wrote: >> Severin Gehwolf 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 ten additional >> commits since the last revision: >> >> - Merge branch 'master' into jdk-8261242-is-containerized-fix >> - jcheck fixes >> - Fix tests >> - Implement Metrics.isContainerized() >> - Some clean-up >> - Drop cgroups testing on plain Linux >> - Implement fall-back logic for non-ro controller mounts >> - Make find_ro static and local to compilation unit >> - 8261242: [Linux] OSContainer::is_containerized() returns true > > I am not enough of a container expert to judge if the basic approach is right > - I trust you on this. This is just a technical code review. @tstuefe Do you mind to take another look? Thank you! - PR Comment: https://git.openjdk.org/jdk/pull/18201#issuecomment-2180504024
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v6]
> Please review this enhancement to the container detection code which allows > it to figure out whether the JVM is actually running inside a container > (`podman`, `docker`, `crio`), or with some other means that enforces > memory/cpu limits by means of the cgroup filesystem. If neither of those > conditions hold, the JVM runs in not containerized mode, addressing the issue > described in the JBS tracker. For example, on my Linux system > `is_containerized() == false" is being indicated with the following trace log > line: > > > [0.001s][debug][os,container] OSContainer::init: is_containerized() = false > because no cpu or memory limit is present > > > This state is being exposed by the Java `Metrics` API class using the new > (still JDK internal) `isContainerized()` method. Example: > > > java -XshowSettings:system --version > Operating System Metrics: > Provider: cgroupv1 > System not containerized. > openjdk 23-internal 2024-09-17 > OpenJDK Runtime Environment (fastdebug build > 23-internal-adhoc.sgehwolf.jdk-jdk) > OpenJDK 64-Bit Server VM (fastdebug build 23-internal-adhoc.sgehwolf.jdk-jdk, > mixed mode, sharing) > > > The basic property this is being built on is the observation that the cgroup > controllers typically get mounted read only into containers. Note that the > current container tests assert that `OSContainer::is_containerized() == true` > in various tests. Therefore, using the heuristic of "is any memory or cpu > limit present" isn't sufficient. I had considered that in an earlier > iteration, but many container tests failed. > > Overall, I think, with this patch we improve the current situation of > claiming a containerized system being present when it's actually just a > regular Linux system. > > Testing: > > - [x] GHA (risc-v failure seems infra related) > - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2 (including > gtests) > - [x] Some manual testing using cri-o > > Thoughts? Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: Remove problem listing of PlainRead which is reworked here - Changes: - all: https://git.openjdk.org/jdk/pull/18201/files - new: https://git.openjdk.org/jdk/pull/18201/files/7c163cb2..3d98cbc2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18201&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18201&range=04-05 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18201.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18201/head:pull/18201 PR: https://git.openjdk.org/jdk/pull/18201
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v4]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: man page review comments - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/b5468440..4c6abddf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19774&range=02-03 Stats: 26 lines in 1 file changed: 5 ins; 0 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 21:35:07 GMT, Maurizio Cimadamore wrote: >> Jorn Vernee has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - review comments >> - add man page > > src/jdk.jdeps/share/man/jnativescan.1 line 166: > >> 164: .fi >> 165: .PP >> 166: \f[V]app.jar (ALL-UNNAMED)\f[R] is the path to the jar file, with the > > This is a good explanation, I'm not sure whether it's necessary, as the > output seems quite self-explanatory. But I'm ok either way. I think that people that get to this section of the man page are the people who either don't understand the output, or (probably more likely) have an idea but want to be sure of what things mean. In both those cases, I think walking through the output is useful. The description of the command line options is probably enough for more people? - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647458338
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v5]
> Please review this enhancement to the container detection code which allows > it to figure out whether the JVM is actually running inside a container > (`podman`, `docker`, `crio`), or with some other means that enforces > memory/cpu limits by means of the cgroup filesystem. If neither of those > conditions hold, the JVM runs in not containerized mode, addressing the issue > described in the JBS tracker. For example, on my Linux system > `is_containerized() == false" is being indicated with the following trace log > line: > > > [0.001s][debug][os,container] OSContainer::init: is_containerized() = false > because no cpu or memory limit is present > > > This state is being exposed by the Java `Metrics` API class using the new > (still JDK internal) `isContainerized()` method. Example: > > > java -XshowSettings:system --version > Operating System Metrics: > Provider: cgroupv1 > System not containerized. > openjdk 23-internal 2024-09-17 > OpenJDK Runtime Environment (fastdebug build > 23-internal-adhoc.sgehwolf.jdk-jdk) > OpenJDK 64-Bit Server VM (fastdebug build 23-internal-adhoc.sgehwolf.jdk-jdk, > mixed mode, sharing) > > > The basic property this is being built on is the observation that the cgroup > controllers typically get mounted read only into containers. Note that the > current container tests assert that `OSContainer::is_containerized() == true` > in various tests. Therefore, using the heuristic of "is any memory or cpu > limit present" isn't sufficient. I had considered that in an earlier > iteration, but many container tests failed. > > Overall, I think, with this patch we improve the current situation of > claiming a containerized system being present when it's actually just a > regular Linux system. > > Testing: > > - [x] GHA (risc-v failure seems infra related) > - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2 (including > gtests) > - [x] Some manual testing using cri-o > > Thoughts? Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits: - Merge branch 'master' into jdk-8261242-is-containerized-fix - Merge branch 'master' into jdk-8261242-is-containerized-fix - Add doc for mountinfo scanning. - Unify naming of variables - Merge branch 'master' into jdk-8261242-is-containerized-fix - Merge branch 'master' into jdk-8261242-is-containerized-fix - jcheck fixes - Fix tests - Implement Metrics.isContainerized() - Some clean-up - ... and 4 more: https://git.openjdk.org/jdk/compare/01ee4241...7c163cb2 - Changes: https://git.openjdk.org/jdk/pull/18201/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18201&range=04 Stats: 406 lines in 19 files changed: 301 ins; 78 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/18201.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18201/head:pull/18201 PR: https://git.openjdk.org/jdk/pull/18201
Re: RFR: 8333446: Add tests for hierarchical container support [v2]
On Thu, 20 Jun 2024 08:34:45 GMT, Severin Gehwolf wrote: >> Please review this PR which adds test support for systemd slices so that >> bugs like [JDK-8217338](https://bugs.openjdk.org/browse/JDK-8217338) can be >> verified. The added test, `SystemdMemoryAwarenessTest` currently passes on >> cgroups v1 and fails on cgroups v2 due to the way how >> [JDK-8217338](https://bugs.openjdk.org/browse/JDK-8217338) was implemented >> when JDK 13 was a thing. Therefore immediately problem-listed. It should get >> unlisted once [JDK-8322420](https://bugs.openjdk.org/browse/JDK-8322420) >> merges. >> >> I'm adding those tests in order to not regress another time. >> >> Testing: >> - [x] Container tests on Linux x86_64 cgroups v2 and Linux x86_64 cgroups v1. >> - [x] New systemd test on cg v1 (passes). Fails on cg v2 (due to >> JDK-8322420) >> - [x] GHA > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into jdk-8333446-systemd-slice-tests > - Fix comments > - 8333446: Add tests for hierarchical container support Anyone willing to review this? - PR Comment: https://git.openjdk.org/jdk/pull/19530#issuecomment-2180477503
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Thu, 20 Jun 2024 11:42:04 GMT, Jorn Vernee wrote: >> src/jdk.jdeps/share/man/jnativescan.1 line 127: >> >>> 125: This option should be set to the version of the runtime under which the >>> 126: application is eventually intended to be run. >>> 127: If this flag is omitted, the version of \f[V]jnativescan\f[R] is used >>> as >> >> Maybe we should point to `Runtime.version()` instead? > > Not sure. If a client is just using the tool, how would they know what > `Runtime.version()` returns? I mean, it's the version of the JDK, but to find > that out they'd have to use another JDK tool (e.g. jshell) to print it out, > or look at the release file. The `jnativescan` version on the other hand is > readily accessible through the `--version` flag. I'll add a note like: `, which is the same as the version of the JDK that the tool belongs to.` - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647442344
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v3]
On Wed, 19 Jun 2024 21:30:49 GMT, Maurizio Cimadamore wrote: >> Jorn Vernee has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - review comments >> - add man page > > src/jdk.jdeps/share/man/jnativescan.1 line 121: > >> 119: .TP >> 120: \f[V]--release\f[R] \f[I]version\f[R] >> 121: Used to specify the Java SE release that specifies the set of restricted > > In principle, the release could also affect which methods are native or not? Yes, but we don't warn on `native` method references, only on declarations. So, it would only affect which methods might be `native` in the user code. But I think that is already implied by the note on multi-release jars? > src/jdk.jdeps/share/man/jnativescan.1 line 127: > >> 125: This option should be set to the version of the runtime under which the >> 126: application is eventually intended to be run. >> 127: If this flag is omitted, the version of \f[V]jnativescan\f[R] is used as > > Maybe we should point to `Runtime.version()` instead? Not sure. If a client is just using the tool, how would they know what `Runtime.version()` returns? I mean, it's the version of the JDK, but to find that out they'd have to use another JDK tool (e.g. jshell) to print it out, or look at the release file. The `jnativescan` version on the other hand is readily accessible through the `--version` flag. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647436934 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647441115
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v4]
On Thu, 20 Jun 2024 10:09:46 GMT, Claes Redestad wrote: >> @wenshao I'm glad we figured it out a bit, and I hope you learned a thing or >> two :) >> >>> It would be even better if Unsafe.putChar could be used for MergeStore in >>> the future. >> >> The `_putCharStringU` intrinsic uses >> `LibraryCallKit::inline_string_char_access`. It seems to generate IR nodes, >> I speculate it could be `StoreC`. We'd have to do more digging to see why >> that pattern is not accepted by `MergeStore`. >> >> @wenshao I'm not sure if everything is right with the bounds checking, but I >> leave this to the library folks to review. What you will definately need is >> benchmarking not just on `M1`, but also `x86-64` and other `aarch64` >> architectures. > > I'm not opposed to accepting this patch as-is, but I think we should do so > with an eye towards reverting if we figure out a way to improve the `putChar` > intrinsic so that it doesn't block merge store optimization. What do you > think @eme64? > > As the need for the changes in > [b5ad8e7](https://github.com/openjdk/jdk/pull/19626/commits/b5ad8e70928c547d134d0e4a532441cad9a7e4a2) > showed added code complexity (like here) can be detrimental to performance, > and if the `putChar` can be improved we might see benefits in more places. @cl4es @wenshao I think we should probably work on `putChar`, or at least figure out what blocks `MergeStore` for `putChar`. Can someone produce a simple stand-alone `.java` file that uses `putChar`, so that I can investigate why `MergeStore` does not work for it? Otherwise, I agree with @cl4es : the code here may be ok for now, but we would have to revert it again later, should `MergeStore` eventually do the trick. - PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2180388366
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v4]
On Thu, 13 Jun 2024 07:36:34 GMT, Emanuel Peter wrote:
>> Thanks to @eme64 's patience and help, I found a way to use MergeStore
>> without doing boundary checking.
>>
>> It would be even better if Unsafe.putChar could be used for MergeStore in
>> the future.
>>
>> ## 1. JavaCode
>>
>> class StringUTF16 {
>> public static void putCharsAt(byte[] value, int i, char c1, char c2,
>> char c3, char c4) {
>> // Don't use the putChar method, Its instrinsic will cause C2 unable
>> to combining values into larger stores.
>> putChar1(value, i, c1);
>> putChar1(value, i + 1, c2);
>> putChar1(value, i + 2, c3);
>> putChar1(value, i + 3, c4);
>> }
>>
>> public static void putCharsAt(byte[] value, int i, char c1, char c2,
>> char c3, char c4, char c5) {
>> // Don't use the putChar method, Its instrinsic will cause C2 unable
>> to combining values into larger stores.
>> putChar1(value, i, c1);
>> putChar1(value, i + 1, c2);
>> putChar1(value, i + 2, c3);
>> putChar1(value, i + 3, c4);
>> putChar1(value, i + 4, c5);
>> }
>>
>> static void putChar1(byte[] value, int i, char c) {
>> int address = Unsafe.ARRAY_BYTE_BASE_OFFSET + (i << 1);
>> Unsafe.getUnsafe().putByte(value, address, (byte)(c >>
>> HI_BYTE_SHIFT));
>> Unsafe.getUnsafe().putByte(value, address + 1, (byte)(c >>
>> LO_BYTE_SHIFT));
>> }
>> }
>>
>>
>> ## 2. Benchmark Numbers
>> The performance numbers under MacBookPro M1 Max are as follows:
>>
>> -Benchmark Mode Cnt Score Error Units
>> -StringBuilders.toStringCharWithNull8Latin1 avgt 15 7.073 ? 0.010 ns/op
>> -StringBuilders.toStringCharWithNull8Utf16 avgt 15 9.298 ? 0.015 ns/op
>> -StringBuilders.toStringCharWithBool8Latin1 avgt 15 7.387 ? 0.021 ns/op
>> -StringBuilders.toStringCharWithBool8Utf16 avgt 15 9.615 ? 0.011 ns/op
>>
>> +Benchmark Mode Cnt Score Error Units
>> +StringBuilders.toStringCharWithNull8Latin1 avgt 15 5.628 ? 0.017 ns/op
>> +25.67%
>> +StringBuilders.toStringCharWithNull8Utf16 avgt 15 6.873 ? 0.026 ns/op
>> +35.28%
>> +StringBuilders.toStringCharWithBool8Latin1 avgt 15 6.480 ? 0.077 ns/op
>> +13.99%
>> +StringBuilders.toStringCharWithBool8Utf16 avgt 15 7.456 ? 0.059 ns/op
>> +28.95%
>>
>>
>> ## 3. TraceMergeStores
>>
>> [TraceMergeStores]: Replace
>> 65 StoreB === 54 7 64 12 [[ 87 ]] @byte[int:>=0]
>> (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=4; unsafe
>> Memory: @byte[int:>=0] (java/la...
>
> @wenshao I'm glad we figured it out a bit, and I hope you learned a thing or
> two :)
>
>> It would be even better if Unsafe.putChar could be used for MergeStore in
>> the future.
>
> The `_putCharStringU` intrinsic uses
> `LibraryCallKit::inline_string_char_access`. It seems to generate IR nodes, I
> speculate it could be `StoreC`. We'd have to do more digging to see why that
> pattern is not accepted by `MergeStore`.
>
> @wenshao I'm not sure if everything is right with the bounds checking, but I
> leave this to the library folks to review. What you will definately need is
> benchmarking not just on `M1`, but also `x86-64` and other `aarch64`
> architectures.
I'm not opposed to accepting this patch as-is, but I think we should do so with
an eye towards reverting if we figure out a way to improve the `putChar`
intrinsic so that it doesn't block merge store optimization. What do you think
@eme64?
As the need for the changes in
[b5ad8e7](https://github.com/openjdk/jdk/pull/19626/commits/b5ad8e70928c547d134d0e4a532441cad9a7e4a2)
showed added code complexity (like here) can be detrimental to performance,
and if the `putChar` can be improved we might see benefits in more places.
-
PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2180308686
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v12]
On Mon, 17 Jun 2024 05:53:45 GMT, Shaojin Wen wrote: >> After PR https://github.com/openjdk/jdk/pull/16245, C2 optimizes stores into >> primitive arrays by combining values into larger stores. >> >> This PR rewrites the code of appendNull and append(boolean) methods so that >> these two methods can be optimized by C2. > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > Utf16 case remove `append first utf16 char` src/java.base/share/classes/java/lang/StringLatin1.java line 832: > 830: // Don't use the putChar method, Its instrinsic will cause C2 > unable to combining values into larger stores. > 831: long address = Unsafe.ARRAY_BYTE_BASE_OFFSET + index; > 832: Unsafe UNSAFE = Unsafe.getUnsafe(); Perhaps better to put in a `private static final` field - PR Review Comment: https://git.openjdk.org/jdk/pull/19626#discussion_r1647313459
Re: RFR: 8333358: java/io/IO/IO.java test fails intermittently [v5]
> Please review this fix for an intermittent test failure. > > On some configurations, the default `expect` timeout of 10 seconds is > insufficient. It is increased to 20; it's hard to imagine a configuration for > which that new value would still be insufficient, but we'll see. > > Aside from that, test-generated diagnostics are improved: the version of the > `expect` command and the duration of each test method are recorded. > `output.exp` is modified for robustness and clear indication of the timeout > condition. > > I was reminded, out-of-band, that test timeouts should be scaled with > `test.timeout.factor`. I propose to integrate this PR first and then > separately update all the affected tests to scale their `expect` timeouts. Pavel Rappo 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 six additional commits since the last revision: - Merge branch 'master' into 858 - Merge branch 'master' into 858 - Add a comment for future maintainers - Disable timeout in expect scripts - Change url as suggested in review - Initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/19627/files - new: https://git.openjdk.org/jdk/pull/19627/files/aad8b491..ae111829 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19627&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19627&range=03-04 Stats: 17482 lines in 318 files changed: 13721 ins; 2088 del; 1673 mod Patch: https://git.openjdk.org/jdk/pull/19627.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19627/head:pull/19627 PR: https://git.openjdk.org/jdk/pull/19627
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows Build changes look ok. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19478#pullrequestreview-2129807207
Re: RFR: 8333446: Add tests for hierarchical container support [v2]
> Please review this PR which adds test support for systemd slices so that bugs > like [JDK-8217338](https://bugs.openjdk.org/browse/JDK-8217338) can be > verified. The added test, `SystemdMemoryAwarenessTest` currently passes on > cgroups v1 and fails on cgroups v2 due to the way how > [JDK-8217338](https://bugs.openjdk.org/browse/JDK-8217338) was implemented > when JDK 13 was a thing. Therefore immediately problem-listed. It should get > unlisted once [JDK-8322420](https://bugs.openjdk.org/browse/JDK-8322420) > merges. > > I'm adding those tests in order to not regress another time. > > Testing: > - [x] Container tests on Linux x86_64 cgroups v2 and Linux x86_64 cgroups v1. > - [x] New systemd test on cg v1 (passes). Fails on cg v2 (due to JDK-8322420) > - [x] GHA Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Merge branch 'master' into jdk-8333446-systemd-slice-tests - Fix comments - 8333446: Add tests for hierarchical container support - Changes: - all: https://git.openjdk.org/jdk/pull/19530/files - new: https://git.openjdk.org/jdk/pull/19530/files/98d780ac..00b528ae Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19530&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19530&range=00-01 Stats: 45352 lines in 1129 files changed: 26950 ins; 13694 del; 4708 mod Patch: https://git.openjdk.org/jdk/pull/19530.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19530/head:pull/19530 PR: https://git.openjdk.org/jdk/pull/19530
