Re: RFR: JDK-8292595: jdwp utf_util getWideString might leak memory [v2]
On Fri, 26 Aug 2022 12:04:57 GMT, Matthias Baesken wrote: >> There seems to be a case where utf_util.c getWideString might leak memory in >> an early return. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce UTF_WARNING and use the fallback Hi, if it is still considered better to always abort in the MultiByteToWideChar failure cases and not use the fallback code, that's fine with me too. - PR: https://git.openjdk.org/jdk/pull/9918
Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic [v9]
> The proposal is to encapsulate the nmethod mark for deoptimization logic in
> one place and only allow access to the `mark_for_deoptimization` from a
> closure object:
> ```C++
> class DeoptimizationMarkerClosure : StackObj {
> public:
> virtual void marker_do(Deoptimization::MarkFn mark_fn) = 0;
> };
>
> This closure takes a `MarkFn` which it uses to mark which nmethods should be
> deoptimized. This marking can only be done through the `MarkFn` and a
> `MarkFn` can only be created in the following code which runs the closure.
> ```C++
> {
> NoSafepointVerifier nsv;
> assert_locked_or_safepoint(Compile_lock);
> marker_closure.marker_do(MarkFn());
> anything_deoptimized = deoptimize_all_marked();
> }
> if (anything_deoptimized) {
> run_deoptimize_closure();
> }
>
> This ensures that this logic is encapsulated and the `NoSafepointVerifier`
> and `assert_locked_or_safepoint(Compile_lock)` makes `deoptimize_all_marked`
> not having to scan the whole code cache sound.
>
> The exception to this pattern, from `InstanceKlass::unload_class`, is
> discussed in the JBS issue, and gives reasons why not marking for
> deoptimization there is ok.
>
> An effect of this encapsulation is that the deoptimization logic was moved
> from the `CodeCache` class to the `Deoptimization` class and the class
> redefinition logic was moved from the `CodeCache` class to the
> `VM_RedefineClasses` class/operation.
>
> Testing: Tier 1-5
>
> _Update_
> ---
> Switched too using a RAII object to track the context instead of putting code
> in a closure. But all the encapsulation is still the same.
>
> Testing: Tier 1-7
>
> _Update_
> ---
>> @stefank suggested splitting out unloading klass logic change into a
>> separate issue [JDK-8291718](https://bugs.openjdk.org/browse/JDK-8291718).
>>
>> Will probably also limit this PR to only encapsulation. (Skipping the linked
>> list optimisation) And create a separate issue for that as well.
>>
>> But this creates a chain of three dependent issues.
>> [JDK-8291237](https://bugs.openjdk.org/browse/JDK-8291237) depends on
>> [JDK-8291718](https://bugs.openjdk.org/browse/JDK-8291718). And the link
>> list optimisation depend will depend on
>> [JDK-8291237](https://bugs.openjdk.org/browse/JDK-8291237).
>>
>> Will mark this as a draft for now and create a PR for
>> [JDK-8291718](https://bugs.openjdk.org/browse/JDK-8291718) first.
>
> _Update_
> ---
> Testing after 11d9dd2: Oracle platforms tier 1-5
Axel Boldt-Christmas has updated the pull request with a new target base due to
a merge or a rebase. The pull request now contains 29 commits:
- Merge remote-tracking branch 'upstream/master' into JDK-8291237
- Add asserts and comments
- Merge remote-tracking branch 'upstream/master' into JDK-8291237
- Add context active assert
- Cleanup comment
- Add list optimization
- Merge remote-tracking branch 'upstream/master' into JDK-8291237
- Rename deoptimize_done enum value
- Add missing code from list revert
- Initial draft new terminology
- ... and 19 more: https://git.openjdk.org/jdk/compare/512fee1d...c35f5ed6
-
Changes: https://git.openjdk.org/jdk/pull/9655/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9655&range=08
Stats: 753 lines in 27 files changed: 351 ins; 282 del; 120 mod
Patch: https://git.openjdk.org/jdk/pull/9655.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/9655/head:pull/9655
PR: https://git.openjdk.org/jdk/pull/9655
Re: RFR: 8292657: Calling GetLocalXXX from virtual thread with thread parameter set to NULL returns carrier locals [v2]
On Fri, 26 Aug 2022 21:33:13 GMT, Serguei Spitsyn wrote: >> If JVM TI GetLocalXXX/SetLocalXXX is called from a virtual thread with the >> thread parameter set to NULL (meaning current thread) then it should get/set >> the value of the locals in the virtual thread frames. Instead, it reads the >> carrier thread locals and/or crashes. >> >> The fix is that the relevant checking of the jthread parameter for NULL and >> adjusting it to current thread is added. >> It is done in new utility `function >> current_thread_obj_or_resolve_external_guard(jthread thread)`. For more >> convenient testing the same adjustment is done in the JVM TI extension >> function `GetCarrierThread()`. >> >> The test serviceability/jvmti/vthread/GetSetLocalTest is updated to add >> previously missed test coverage. >> >> The test serviceability/jvmti/vthread/VThreadTest has been updated to adopt >> to update behavior of the `GetCarrierThread`. >> >> The fix was verified with the >> test/hotspot/jtreg/serviceability/jvmti/vthread/ tests. >> >> The fix was also tested with the existing JVM TI and JDI tests to make sure >> no regressions are introduced. > > Serguei Spitsyn has updated the pull request incrementally with one > additional commit since the last revision: > > remove unneeded commented lines in test Thank you for review, Chris! - PR: https://git.openjdk.org/jdk/pull/10051
Re: RFR: JDK-8292595: jdwp utf_util getWideString might leak memory [v2]
On Sun, 28 Aug 2022 22:42:14 GMT, Chris Plummer wrote: > If you are going to do that, then you should track down all uses of this code > (direct and indirect) to see if this error is ever acceptable and is handled > properly. It seems that if the unicode string is bad, we should be exiting. I > see this code being used indirectly from setAgentPropertyValue(), which only > seems to be used for sun.jdwp.listenerAddress. Do we want the raw unicode > bytes to be used in this case if they cannot be converted? It's also used > from printLastError(), which gets the unicode bytes from GetLastError(). > Should't they always be valid? I agree it complicates things and it would require looking at all usages. My comment was just to say that if it is changed to emit a warning then it would only make sense to do so in limited cases (the NO_UNICODE_TRANSLATION mostly). It may be simpler to not change the code of course. - PR: https://git.openjdk.org/jdk/pull/9918
Re: RFR: 8292375: Convert ProtectionDomainCacheTable to ResourceHashtable [v3]
On Fri, 26 Aug 2022 17:07:54 GMT, Coleen Phillimore wrote: >> Please review this simple conversion for the ProtectionDomainCacheTable from >> Old Hashtable to ResourceHashtable. There are specific tests for this table >> in test/hotspot/jtreg/runtime/Dictionary and >> serviceability/dcmd/vm/DictionaryStatsTest.java. >> Also tested with tier1-7. > > Coleen Phillimore has updated the pull request incrementally with one > additional commit since the last revision: > > include mutexLocker.hpp for minimal build. Thanks for reading through this. I was going to make a larger change, then changed my mind. This conversion is limited. It is able to take advantage of the ability to copy WeakHandle without side effects, so that we don't have to store ProtectionDomainCacheEntry into the pd_set linked list, which makes it nicer. - PR: https://git.openjdk.org/jdk/pull/10043
Re: RFR: 8292375: Convert ProtectionDomainCacheTable to ResourceHashtable [v3]
On Mon, 29 Aug 2022 00:12:36 GMT, David Holmes wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> include mutexLocker.hpp for minimal build.
>
> src/hotspot/share/classfile/protectionDomainCache.cpp line 42:
>
>> 40: #include "utilities/resourceHash.hpp"
>> 41:
>> 42: unsigned int ProtectionDomainCacheTable::compute_hash(const WeakHandle&
>> protection_domain) {
>
> Why are we now using `WeakHandle` everywhere?
WeakHandle is the object we're storing as in the hashtable. It also turns out
to be the key.
> src/hotspot/share/classfile/protectionDomainCache.cpp line 43:
>
>> 41:
>> 42: unsigned int ProtectionDomainCacheTable::compute_hash(const WeakHandle&
>> protection_domain) {
>> 43: return (unsigned int)(protection_domain.resolve()->identity_hash());
>
> And if it is a `WeakHandle` can't `resolve` now return NULL?
compute_hash() is always called on a live WeakHandle so will never return null.
> src/hotspot/share/classfile/protectionDomainCache.cpp line 50:
>
>> 48: }
>> 49:
>> 50: ResourceHashtable> mtClass,
>
> I am struggling to understand what the key and values types are in this
> hashtable ???
WeakHandle is the key and the value in this hashtable. We need to match the
WeakHandle in the input by value (see equals function), but we also want to
return the WeakHandle to store in to the pd_set list in the DictionaryEntry.
> src/hotspot/share/classfile/protectionDomainCache.cpp line 162:
>
>> 160: // Purge any deleted entries outside of the SystemDictionary_lock.
>> 161: purge_deleted_entries();
>> 162: int oops_removed = purge_entries_from_table();
>
> Maybe add comment
>
> `int oops_removed = purge_entries_from_table(); // reacquires SD lock`
>
> otherwise it gives the false impression this is done lock-free.
Ok.
> src/hotspot/share/classfile/protectionDomainCache.cpp line 168:
>
>> 166: }
>> 167:
>> 168: void ProtectionDomainCacheTable::print_on(outputStream* st) {
>
> It is a little disconcerting that `print_on` can no longer be a `const`
> function!
It's static, so it can't.
> src/hotspot/share/classfile/protectionDomainCache.cpp line 186:
>
>> 184:
>> 185: // The object_no_keepalive() call peeks at the phantomly reachable oop
>> without
>> 186: // keeping it alive. This is okay to do in the VM thread state if it is
>> not
>
> You don't call `object_no_keepalive()` any more
This one (not the one I removed), is called by dictionary.cpp - the pd_set is a
linked list of ProtectionDomainEntry, where we don't keep the WeakHandle alive
when looking at the value.
> src/hotspot/share/classfile/protectionDomainCache.cpp line 202:
>
>> 200: // Keep entry alive
>> 201: (void)wk->resolve();
>> 202: return *wk;
>
> Can't this be factored out of the if-else as `wk` is always the right entry
> to resolve and return?
Yes, that works.
-
PR: https://git.openjdk.org/jdk/pull/10043
Re: RFR: 8292375: Convert ProtectionDomainCacheTable to ResourceHashtable [v4]
> Please review this simple conversion for the ProtectionDomainCacheTable from > Old Hashtable to ResourceHashtable. There are specific tests for this table > in test/hotspot/jtreg/runtime/Dictionary and > serviceability/dcmd/vm/DictionaryStatsTest.java. > Also tested with tier1-7. Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision: David's comments - Changes: - all: https://git.openjdk.org/jdk/pull/10043/files - new: https://git.openjdk.org/jdk/pull/10043/files/842b30ab..ac67b187 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=10043&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10043&range=02-03 Stats: 9 lines in 1 file changed: 3 ins; 5 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/10043.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10043/head:pull/10043 PR: https://git.openjdk.org/jdk/pull/10043
RFR: 8293037: Remove DebuggerBase.writeBytes() and related code from SA
DebuggerBase.writeBytes() is not needed. It is only called by a number of other "write" methods, such as writeJBoolean(), but these methods are never called, so they can be removed along with writeBytes(). Lastly, writeBytes() calls writeBytesToProcess() which has no other callers, so it too can be removed. All it currently does is throw an exception. I imagine there may have been some use for this "write" capability 20+ years ago on Solaris/x86, or maybe it was part of future plans that never panned out. In any case, it's all just baggage now that can be removed. - Commit messages: - Get rid of writeBytes() and related code. Changes: https://git.openjdk.org/jdk/pull/10068/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10068&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8293037 Stats: 118 lines in 7 files changed: 0 ins; 116 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/10068.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10068/head:pull/10068 PR: https://git.openjdk.org/jdk/pull/10068
RFR: 8292995: improve the SA page cache
The page caching support in SA is woefully dated. I think it has stayed the same for over 20 years when it was originally done for solarix-x86. This code has been replicated for every port. Currently all ports only have a 16mb cache. They use 4k pages and there are 4k of them. I think the 4k page size is fine. The following comment appears in all the ports: // ... This is a cache of 4096 4K pages, or 16 MB. The page // size must be adjusted to be the hardware's page size. // (FIXME: should pick this up from the debugger.) I disagree with this. Matching the possibly very large hardware page size (I think maybe they meant OS page size) would require the SA page cache to be very very large, using a lot of java heap space. It would also require a lot of unnecessary copying from the debuggee process's memory. There's no reason for the SA cache's page size to match the OS page size. However, 16mb seems very small. I tried 256mb and this gave about a 10% performance improvement in a heap dump, and is still fairly small, so I think it is a reasonable adjustment. Another comment you see in all the ports (copied from solaris-x86) is: // FIXME: re-test necessity of cache on Linux, where data // fetching is faster // Cache portion of the remote process's address space. // Fetching data over the socket connection to dbx is slow. // Might be faster if we were using a binary protocol to talk to // dbx, but would have to test. For now, this cache works best // if it covers the entire heap of the remote process. FIXME: at At least on linux the cache is definitely needed. I turned it off and a heap dump took 9x longer. Also I think covering the entire heap is overkill, and I doubt was ever being done given how small the cache is. So I think this comment can just be removed. - Commit messages: - Do some tuning and cleanup of SA page cache. Changes: https://git.openjdk.org/jdk/pull/10069/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10069&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8292995 Stats: 48 lines in 4 files changed: 0 ins; 40 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/10069.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10069/head:pull/10069 PR: https://git.openjdk.org/jdk/pull/10069
Re: RFR: JDK-8292066 Convert TestInputArgument.sh and TestSystemLoadAvg.sh to java version
On Thu, 11 Aug 2022 21:12:08 GMT, Bill Huang wrote:
> This task converts 2 shell tests to java version.
> test/java/lang/management/OperatingSystemMXBean/TestSystemLoadAvg.sh
> test/java/lang/management/RuntimeMXBean/TestInputArgument.sh
test/jdk/java/lang/management/OperatingSystemMXBean/GetSystemLoadAverage.java
line 59:
> 57: // The system load average may be changing due to other jobs running.
> 58: // Allow some delta.
> 59: private static double DELTA = 0.05;
Not a part of your fix but could you make it final?
test/jdk/java/lang/management/OperatingSystemMXBean/GetSystemLoadAverage.java
line 69:
> 67: }
> 68:
> 69: @Test(invocationCount = 5, timeOut = 300)
I am not sure it is the correct replacement. Accordingly to TestNG doc the
invocationCount = 5 means that TestNG calls the test 5 times. And test fails
if any of the invocations fail while the bash script makes 5 attempts and
passes if testcase passed in any of them.
test/jdk/java/lang/management/OperatingSystemMXBean/GetSystemLoadAverage.java
line 71:
> 69: @Test(invocationCount = 5, timeOut = 300)
> 70: void testSystemLoadAvg() throws Exception {
> 71: if (!OS.contains("Win")) {
Check /test/lib/jdk/test/lib/Platform.java, it contains "Platform.isWindows()"
which could be used for this.
-
PR: https://git.openjdk.org/jdk/pull/9848
Re: RFR: 8293037: Remove DebuggerBase.writeBytes() and related code from SA
On Mon, 29 Aug 2022 18:54:58 GMT, Chris Plummer wrote: > DebuggerBase.writeBytes() is not needed. It is only called by a number of > other "write" methods, such as writeJBoolean(), but these methods are never > called, so they can be removed along with writeBytes(). Lastly, writeBytes() > calls writeBytesToProcess() which has no other callers, so it too can be > removed. All it currently does is throw an exception. I imagine there may > have been some use for this "write" capability 20+ years ago on Solaris/x86, > or maybe it was part of future plans that never panned out. In any case, it's > all just baggage now that can be removed. Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.org/jdk/pull/10068
Re: RFR: JDK-8292066 Convert TestInputArgument.sh and TestSystemLoadAvg.sh to java version
On Mon, 29 Aug 2022 21:50:14 GMT, Leonid Mesnik wrote: >> This task converts 2 shell tests to java version. >> test/java/lang/management/OperatingSystemMXBean/TestSystemLoadAvg.sh >> test/java/lang/management/RuntimeMXBean/TestInputArgument.sh > > test/jdk/java/lang/management/OperatingSystemMXBean/GetSystemLoadAverage.java > line 69: > >> 67: } >> 68: >> 69: @Test(invocationCount = 5, timeOut = 300) > > I am not sure it is the correct replacement. Accordingly to TestNG doc the > invocationCount = 5 means that TestNG calls the test 5 times. And test fails > if any of the invocations fail while the bash script makes 5 attempts and > passes if testcase passed in any of them. Good catch! I've missed the exit statement in the original shell script. - PR: https://git.openjdk.org/jdk/pull/9848
Re: RFR: JDK-8292067 Convert test/sun/management/jmxremote/bootstrap shell tests to java version
On Mon, 22 Aug 2022 22:51:50 GMT, Bill Huang wrote:
> This task convert 3 shell tests below to java version.
> test/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh
> test/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh
> test/sun/management/jmxremote/bootstrap/RmiSslNoKeyStoreTest.sh
test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 33:
> 31: import java.rmi.server.ExportException;
> 32:
> 33: import java.util.*;
wildcards in import are not welcome, please avoid them
not needed to replace existing, just don't introduce them
test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 115:
> 113: throws IOException {
> 114:
> 115: final Set names = server.queryNames(pattern, query);
Since you change these lines, might add template parameters here?
Set names = ...
and simplify if it is possible.
Not a request, just a proposal.
test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 166:
> 164: if (args.length == 0) {
> 165: throw new IllegalArgumentException("Argument is required for
> this" +
> 166: " test");
not needed to split lines
test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 700:
> 698: }
> 699: }
> 700: System.err.println(
I think that the previous indentation was good enough. No need to change it.
test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 716:
> 714: throws InterruptedException, IOException {
> 715: String errStr = null;
> 716: errStr = testConfiguration(conf);
Merge with errStr = null ?
test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 47:
> 45: static final String TEST_SRC = "@TEST-SRC@";
> 46:
> 47: static final boolean isWindows =
> System.getProperty("os.name").contains(
Check Platform.java.
test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 112:
> 110:
> 111: static String getDefaultFileName(String basename) {
> 112: final StringBuffer defaultFileName =
Why don't just create
final String = System.getProperty(.._) + SEP + + basename;
or even make static final variable like defaultFileNamePrefix = ... and
return defaultFileNamePrefix + basename;
test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 130:
> 128: **/
> 129: static String getDefaultStoreName(String basename) {
> 130: final StringBuffer defaultFileName =
the same as previous
test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 162:
> 160: Utils.replaceFilesString(propertyFiles,
> 161: (s) -> s.replace(TEST_SRC, DEST)
> 162: .replaceAll("[/]", ""));
Could you please comment this replacement.
-
PR: https://git.openjdk.org/jdk/pull/9973
Re: RFR: JDK-8292066 Convert TestInputArgument.sh and TestSystemLoadAvg.sh to java version [v2]
> This task converts 2 shell tests to java version. > test/java/lang/management/OperatingSystemMXBean/TestSystemLoadAvg.sh > test/java/lang/management/RuntimeMXBean/TestInputArgument.sh Bill Huang has updated the pull request incrementally with two additional commits since the last revision: - Added a blank line between fields and methods. - Implemented review comments. - Changes: - all: https://git.openjdk.org/jdk/pull/9848/files - new: https://git.openjdk.org/jdk/pull/9848/files/41a45483..21e7ce84 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9848&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9848&range=00-01 Stats: 47 lines in 1 file changed: 26 ins; 7 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/9848.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9848/head:pull/9848 PR: https://git.openjdk.org/jdk/pull/9848
Re: RFR: JDK-8292067 Convert test/sun/management/jmxremote/bootstrap shell tests to java version
On Mon, 29 Aug 2022 22:06:18 GMT, Leonid Mesnik wrote: >> This task convert 3 shell tests below to java version. >> test/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh >> test/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh >> test/sun/management/jmxremote/bootstrap/RmiSslNoKeyStoreTest.sh > > test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 33: > >> 31: import java.rmi.server.ExportException; >> 32: >> 33: import java.util.*; > > wildcards in import are not welcome, please avoid them > not needed to replace existing, just don't introduce them Good catch! I didn't introduce this intentionally. It was automatically rearranged by IntelliJ. I will find out a way to disable that feature. - PR: https://git.openjdk.org/jdk/pull/9973
Re: RFR: JDK-8292066 Convert TestInputArgument.sh and TestSystemLoadAvg.sh to java version [v2]
On Mon, 29 Aug 2022 23:25:11 GMT, Bill Huang wrote:
>> This task converts 2 shell tests to java version.
>> test/java/lang/management/OperatingSystemMXBean/TestSystemLoadAvg.sh
>> test/java/lang/management/RuntimeMXBean/TestInputArgument.sh
>
> Bill Huang has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Added a blank line between fields and methods.
> - Implemented review comments.
test/jdk/java/lang/management/OperatingSystemMXBean/GetSystemLoadAverage.java
line 73:
> 71: String.format("Run %d: TestSystemLoadAvg", i));
> 72: if (!Platform.isWindows()) {
> 73: // On Linux or Solaris
Sorry, if missed the first time. It is Linux or Mac, or better something like
*mix.
test/jdk/java/lang/management/OperatingSystemMXBean/GetSystemLoadAverage.java
line 95:
> 93: i));
> 94: if (i == MAX_RETRIES)
> 95: {
please move move brace to the prev line
-
PR: https://git.openjdk.org/jdk/pull/9848
Re: RFR: JDK-8292067 Convert test/sun/management/jmxremote/bootstrap shell tests to java version
On Mon, 29 Aug 2022 22:11:38 GMT, Leonid Mesnik wrote:
>> This task convert 3 shell tests below to java version.
>> test/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh
>> test/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh
>> test/sun/management/jmxremote/bootstrap/RmiSslNoKeyStoreTest.sh
>
> test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 115:
>
>> 113: throws IOException {
>> 114:
>> 115: final Set names = server.queryNames(pattern, query);
>
> Since you change these lines, might add template parameters here?
> Set names = ...
> and simplify if it is possible.
> Not a request, just a proposal.
Actually, I didn't make changes to these lines. They were done by the IDEA auto
formatter. But I can simplify it and add template parameters.
> test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 166:
>
>> 164: if (args.length == 0) {
>> 165: throw new IllegalArgumentException("Argument is required
>> for this" +
>> 166: " test");
>
> not needed to split lines
The IDEA splits the lines for a line limit of 80 characters.
-
PR: https://git.openjdk.org/jdk/pull/9973
