Re: RFR: 8284960: Integration of JEP 426: Vector API (Fourth Incubator) [v9]
> Hi All, > > Patch adds the planned support for new vector operations and APIs targeted > for [JEP 426: Vector API (Fourth > Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173) > > Following is the brief summary of changes:- > > 1) Extends the scope of existing lanewise API for following new vector > operations. >- VectorOperations.BIT_COUNT: counts the number of one-bits >- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero > bits >- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing > zero bits >- VectorOperations.REVERSE: reversing the order of bits >- VectorOperations.REVERSE_BYTES: reversing the order of bytes >- compress and expand bits: Semantics are based on Hacker's Delight > section 7-4 Compress, or Generalized Extract. > > 2) Adds following new APIs to perform cross lane vector compress and > expansion operations under the influence of a mask. >- Vector.compress >- Vector.expand >- VectorMask.compress > > 3) Adds predicated and non-predicated versions of following new APIs to load > and store the contents of vector from foreign MemorySegments. > - Vector.fromMemorySegment > - Vector.intoMemorySegment > > 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support > for each newly added operation. > > > Patch has been regressed over AARCH64 and X86 targets different AVX levels. > > Kindly review and share your feedback. > > Best Regards, > Jatin Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision: 8284960: Review comments resolved. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8425/files - new: https://git.openjdk.java.net/jdk/pull/8425/files/17a0e38c..a2c9673d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8425=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8425=07-08 Stats: 110 lines in 7 files changed: 42 ins; 31 del; 37 mod Patch: https://git.openjdk.java.net/jdk/pull/8425.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8425/head:pull/8425 PR: https://git.openjdk.java.net/jdk/pull/8425
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v10]
On Wed, 25 May 2022 03:02:45 GMT, ExE Boss wrote: >> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> add test for newHashSet and newLinkedHashSet > > test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 360: > >> 358: throw new RuntimeException(e); >> 359: } >> 360: }) > > These probably need a `mapField.setAccessible(true)` call, or a `VarHandle` > for the `HashSet.map` field. Yes, this test fails with IllegalAccessException. Probably it's easiest to use a VarHandle to get private fields, similar to other usage already in this test. This test case is a bit odd though in that it's supposed to test HashSet and LinkedHashSet but it mostly actually tests HashMap. It creates the Set instance and immediately extracts the HashMap, which is then passed to the actual test, which operates directly on the HashMap. It would be preferable to create a Set; add an element (so that it's properly allocated); and then make assertions over the Set (which involve extracting the HashMap, etc.) It seems like there should be factoring that allows this sort of arrangement to be retrofitted without adding too much complication. Finally, please add "8284780" to the `@bug` line at the top of this test. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v10]
On Tue, 24 May 2022 21:37:52 GMT, XenoAmess wrote:
>> as title.
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add test for newHashSet and newLinkedHashSet
I looked at all the use sites and they look fine. Some look like they could use
additional cleanup, but that's probably beyond the scope of this change. (Also,
I haven't seen `StringTokenizer` in a long time) It's amazing how many bugs
there are -- the majority look like they allocated the HashSet with the wrong
capacity! Again, this proves the worth of these new APIs.
src/java.base/share/classes/java/util/HashSet.java line 398:
> 396: public static HashSet newHashSet(int numItems) {
> 397: return new HashSet<>(HashMap.calculateHashMapCapacity(numItems));
> 398: }
Please use "elements" instead of "items" throughout the specifications for the
objects that are members of the HashSet. This includes the API notes above as
well as the specs and API notes in LinkedHashSet.
-
PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Tue, 24 May 2022 21:10:29 GMT, David Holmes wrote: > Just FYI. There is a mistake in this changeset. The JDK ProblemList already > contained: > > `java/nio/channels/FileChannel/LargeMapTest.java 8286980 windows-all` > > and this change then added: > > `java/nio/channels/FileChannel/LargeMapTest.java 8286642 generic-i586` > > which appears to have negated the first entry. Whoops, sorry. It seems not to be a problem since [JDK-8287263](https://bugs.openjdk.java.net/browse/JDK-8287263) committed a few hours ago removed the `windows-all` entry and fixed the test. I checked other ProblemLists, and there are no other double entries. - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287064: Modernize ProxyGenerator.PrimitiveTypeInfo [v2]
On Fri, 20 May 2022 22:18:42 GMT, liach wrote: >> Simplify opcode handling, use `final` in `PrimitiveTypeInfo`, and replace >> the hash map with a simple lookup, similar to what's done in >> [JDK-8284880](https://bugs.openjdk.java.net/browse/JDK-8284880) (#8242) > > liach has updated the pull request incrementally with one additional commit > since the last revision: > > Convert PrimitiveTypeInfo to an enum I would prefer to look up in a `Map.ofEntries` than to switch on string class names, which is both faster and clearer. The even faster heuristic of using perfect hash table has already been proven slower than the list of if statements in JDK-8284880, referred to in the pull request description. - PR: https://git.openjdk.java.net/jdk/pull/8801
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v10]
On Tue, 24 May 2022 21:37:52 GMT, XenoAmess wrote:
>> as title.
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add test for newHashSet and newLinkedHashSet
test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 354:
> 352: rsc("rsls", size, cap, () -> {
> 353: try {
> 354: Field mapField =
> HashSet.class.getDeclaredField("map");
For clarity, consider using var handle/method handle and keep the reflection
code in the `WhiteBoxResizeTest` constructor.
-
PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287064: Modernize ProxyGenerator.PrimitiveTypeInfo [v2]
On Fri, 20 May 2022 22:18:42 GMT, liach wrote:
>> Simplify opcode handling, use `final` in `PrimitiveTypeInfo`, and replace
>> the hash map with a simple lookup, similar to what's done in
>> [JDK-8284880](https://bugs.openjdk.java.net/browse/JDK-8284880) (#8242)
>
> liach has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Convert PrimitiveTypeInfo to an enum
> > Did you consider switch (class) {...} in PrimitiveTypeInfo.get?
>
> I don't think we can switch on class instances yet. Even with preview
> enabled, best I can get is (for type incompatibility of `Class` and
> `Class`, etc.)
>
> ```java
> return switch (cl) {
> case Class e && e == int.class -> 1;
> case Class e && e == long.class -> 2;
> case Class e && e == boolean.class -> 3;
> case Class e && e == short.class -> 4;
> case Class e && e == byte.class -> 5;
> case Class e && e == char.class -> 6;
> case Class e && e == float.class -> 7;
> case Class e && e == double.class -> 8;
> default -> 0;
> };
> ```
>
> to avoid type incompatibility; this is in turn implemented by a bootstrap
> method and a loop, which is of course obviously much slower.
Not necessarily recommending this coding idiom, but if you screened on
Class.isPrimitive() being true (taking care to avoid void.class), you could
switch on the string presentations on the class such as getName or
getSimpleName.
-
PR: https://git.openjdk.java.net/jdk/pull/8801
Re: RFR: 8262889: Compiler implementation for Record Patterns [v7]
> 8262889: Compiler implementation for Record Patterns > > A first version of a patch that introduces record patterns into javac as a > preview feature. For the specification, please see: > http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html > > There are two notable tricky parts: > -in the parser, it was necessary to improve the `analyzePattern` method to > handle nested/record patterns, while still keeping error recovery reasonable > -in the `TransPatterns`, the desugaring of the record patterns is very > straightforward - effectivelly the record patterns are desugared into > guards/conditions. This will likely be improved in some future version/preview > > `MatchException` has been extended to cover additional cases related to > record patterns. Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 34 commits: - Merge branch 'master' into patterns-record-deconstruction3 - Post merge fix. - Merge branch 'master' into patterns-record-deconstruction3 - Fixing (non-)exhaustiveness for enum. - Merge branch 'type-pattern-third' into patterns-record-deconstruction3 - Merge branch 'master' into type-patterns-third - Using Type.isRaw instead of checking the AST structure. - Exhaustiveness should accept supertypes of the specified type. - Renaming the features from deconstruction pattern to record pattern. - Fixing guards after record patterns. - ... and 24 more: https://git.openjdk.java.net/jdk/compare/742644e2...f49d3f0a - Changes: https://git.openjdk.java.net/jdk/pull/8516/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8516=06 Stats: 2255 lines in 50 files changed: 2169 ins; 15 del; 71 mod Patch: https://git.openjdk.java.net/jdk/pull/8516.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8516/head:pull/8516 PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v10]
On Tue, 24 May 2022 21:37:52 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > add test for newHashSet and newLinkedHashSet test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 360: > 358: throw new RuntimeException(e); > 359: } > 360: }) These probably need a `mapField.setAccessible(true)` call, or a `VarHandle` for the `HashSet.map` field. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287064: Modernize ProxyGenerator.PrimitiveTypeInfo [v2]
On Tue, 24 May 2022 17:26:52 GMT, Roger Riggs wrote:
> Did you consider switch (class) {...} in PrimitiveTypeInfo.get?
I don't think we can switch on class instances yet. Even with preview enabled,
best I can get is (for type incompatibility of `Class` and `Class`,
etc.)
return switch (cl) {
case Class e && e == int.class -> 1;
case Class e && e == long.class -> 2;
case Class e && e == boolean.class -> 3;
case Class e && e == short.class -> 4;
case Class e && e == byte.class -> 5;
case Class e && e == char.class -> 6;
case Class e && e == float.class -> 7;
case Class e && e == double.class -> 8;
default -> 0;
};
to avoid type incompatibility; this is in turn implemented by a bootstrap
method and a loop, which is of course obviously much slower.
-
PR: https://git.openjdk.java.net/jdk/pull/8801
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]
On Sun, 22 May 2022 16:45:11 GMT, Kim Barrett wrote:
>> It might be GCC bug...
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>>
>> This issue is for integer literal, but [Comment
>> 29](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578#c29) mentions address
>> calculation (e.g. `NULL + offset`) - it is similar the problem in
>> jfrTraceIdBits.inline.hp because `dest` comes from `low_addr()` (`addr +
>> low_offset`).
>
> I don't see the similarity. That gcc bug is about literals used as addresses,
> which get treated (suggested inappropriately) as NULL+offset, with NULL+offset
> being a cause of warnings. I don't see that happening in our case. That is,
> in our `addr + low_offset`, `addr` doesn't seem to be either a literal or NULL
> that I can see.
>
> It's hard for me to investigate this any further just by perusing the code, so
> I'm in the process of getting set up to build with gcc12.x. That will let me
> do some experiments. It may take me a few days to get to that point though.
I spent some time looking into this one. I agree there is a false positive
here, and there doesn't seem to be a better solution than suppressing the
warning. I would prefer the change below, rather than what's proposed. Also
eliminate the changes to utilities/compilerWarnings files. This is a very
gcc-specific issue; there's no reason to generalize the solution. The reason
for relocating the suppression is to be able to describe the issue in more
detail in a context where that description makes sense.
template
inline void JfrTraceIdBits::store(jbyte bits, const T* ptr) {
assert(ptr != NULL, "invariant");
// gcc12 warns "writing 1 byte into a region of size 0" when T == Klass.
// The warning seems to be a false positive. And there is no warning for
// other types that use the same mechanisms. The warning also sometimes
// goes away with minor code perturbations, such as replacing function calls
// with equivalent code directly inlined.
PRAGMA_DIAG_PUSH
PRAGMA_DISABLE_GCC_WARNING("-Wstringop-overflow")
set(bits, traceid_tag_byte(ptr));
PRAGMA_DIAG_POP
}
-
PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v8]
On Sun, 22 May 2022 08:40:28 GMT, Yasumasa Suenaga wrote:
>> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1
>> on Fedora 36.
>> As you can see, the warnings spreads several areas. Let me know if I should
>> separate them by area.
>>
>> * -Wstringop-overflow
>> * src/hotspot/share/oops/array.hpp
>> *
>> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>>
>> In member function 'void Array::at_put(int, const T&) [with T = unsigned
>> char]',
>> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
>> inlined from 'void ConstantPool::method_at_put(int, int, int)' at
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
>> inlined from 'ConstantPool*
>> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:
>
> Yasumasa Suenaga has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 11 commits:
>
> - Merge remote-tracking branch 'upstream/master' into gcc12-warnings
> - Use getter function to access "_data"
> - Revert changes for bytecodeAssembler.cpp, classFileParser.cpp,
> symbolTable.cpp
> - revert changes for memnode.cpp and type.cpp
> - Add assert to check the range of BasicType
> - Merge remote-tracking branch 'upstream/master' into HEAD
> - Revert change for java.c , parse_manifest.c , LinuxPackage.c
> - Calculate char offset before realloc()
> - Use return value from JLI_Snprintf
> - Avoid pragma error in before GCC 12
> - ... and 1 more:
> https://git.openjdk.java.net/jdk/compare/c156bcc5...042c1c70
Changes requested by kbarrett (Reviewer).
src/hotspot/share/oops/array.hpp line 102:
> 100: // standard operations
> 101: int length() const { return _length; }
> 102: T* data() const { return
> reinterpret_cast(reinterpret_cast(this) +
> base_offset_in_bytes()); }
Adding the const-qualifier to the `data()` function and then implicitly
casting it away (by casting through intptr_t) is wrong. Either don't
const-qualify (and leave it to some future use that needs such to address
appropriately), or have two functions. Also, the line length is excessive.
So this:
T* data() {
return reinterpret_cast(
reinterpret_cast(this) + base_offset_in_bytes());
}
and optionally add this:
const T* data() const {
return reinterpret_cast(
reinterpret_cast(this) + base_offset_in_bytes());
}
-
PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v19]
On Tue, 3 May 2022 21:35:48 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Add mask values to constants' javadoc. Will take up work on this issue again for JDK 20. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
> This is an early review of changes to better model JVM access flags, that is > "modifiers" like public, protected, etc. but explicitly at a VM level. > > Language level modifiers and JVM level access flags are closely related, but > distinct. There are concepts that overlap in the two domains (public, > private, etc.), others that only have a language-level modifier (sealed), and > still others that only have an access flag (synthetic). > > The existing java.lang.reflect.Modifier class is inadequate to model these > subtleties. For example, the bit positions used by access flags on different > kinds of elements overlap (such as "volatile" for fields and "bridge" for > methods. Just having a raw integer does not provide sufficient context to > decode the corresponding language-level string. Methods like > Modifier.methodModifiers() were introduced to cope with this situation. > > With additional modifiers and flags on the horizon with projects like > Valhalla, addressing the existent modeling deficiency now ahead of time is > reasonable before further strain is introduced. > > This PR in its current form is meant to give the overall shape of the API. It > is missing implementations to map from, say, method modifiers to access > flags, taking into account overlaps in bit positions. > > The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in > once the API is further along. Joe Darcy 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 32 additional commits since the last revision: - Target JDK 20 rather than 19. - Merge branch 'master' into JDK-8266670 - Add mask values to constants' javadoc. - Implement review feedback from mlchung. - Fix type in @throws tag. - Merge branch 'master' into JDK-8266670 - Respond to review feedback. - Merge branch 'master' into JDK-8266670 - Make workding changes suggested in review feedback. - Merge branch 'master' into JDK-8266670 - ... and 22 more: https://git.openjdk.java.net/jdk/compare/60434fb7...05cf2d8b - Changes: - all: https://git.openjdk.java.net/jdk/pull/7445/files - new: https://git.openjdk.java.net/jdk/pull/7445/files/ead5911f..05cf2d8b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=19 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=18-19 Stats: 255671 lines in 3128 files changed: 171304 ins; 68829 del; 15538 mod Patch: https://git.openjdk.java.net/jdk/pull/7445.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7445/head:pull/7445 PR: https://git.openjdk.java.net/jdk/pull/7445
RFR: 8273346: Expand library mappings to IEEE 754 operations
Generally add apiNote's to map from Java library methods to particular IEEE 754 operations. For now, I only added such notes to java.lang.Math and not java.lang.StrictMath. - Commit messages: - Add floor and ceil. - JDK-8273346: Examine use of "rounding mode" and "rounding policy" in Math and StrictMath Changes: https://git.openjdk.java.net/jdk/pull/8876/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8876=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273346 Stats: 108 lines in 4 files changed: 103 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8876.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8876/head:pull/8876 PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v10]
> as title. XenoAmess has updated the pull request incrementally with one additional commit since the last revision: add test for newHashSet and newLinkedHashSet - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/07e9b8b0..56d029f4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=08-09 Stats: 22 lines in 1 file changed: 21 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8302.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8302/head:pull/8302 PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Mon, 23 May 2022 18:25:23 GMT, Aleksey Shipilev wrote: >> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot >> of x86_32 code. The x86_32 porting is done under >> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, >> we can problemlist the failing tests to get cleaner runs for other patches. >> This should also make GHA runs cleaner. >> >> Additional testing: >> - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier3` (clean now) >> - [x] GHA run > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Another release test There is a mistake in this changeset. The JDK ProblemList already contained: `java/nio/channels/FileChannel/LargeMapTest.java 8286980 windows-all` and this change then added: `java/nio/channels/FileChannel/LargeMapTest.java 8286642 generic-i586` which appears to have negated the first entry. - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287107: CgroupSubsystemFactory.setCgroupV2Path asserts with freezer controller [v2]
On Tue, 24 May 2022 19:34:16 GMT, Severin Gehwolf wrote: >> This is not a fall-through because the previous line ends with a `break`. > > My bad. How about `Intentional incomplete switch. There are ...`? Anyway, why > is the empty `default` case needed other than for the comment? To me, the `default:` switch is a clear indication that "everything else comes here". So you won't be confused whether the comment is related to the last line just above the comment. - PR: https://git.openjdk.java.net/jdk/pull/8858
Re: RFR: 8287107: CgroupSubsystemFactory.setCgroupV2Path asserts with freezer controller [v2]
On Tue, 24 May 2022 16:09:54 GMT, Ioi Lam wrote: >> src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java >> line 155: >> >>> 153: // There are some controllers (such as freezer) that >>> Java doesn't >>> 154: // care about. Just ignore them. These are not >>> considered in the >>> 155: // anyCgroupsV1Controller/anyCgroupsV1Controller >>> checks. >> >> It's not clear why this `default` is necessary. Could we just add the >> comment like so: >> >> >> // Intentional fall-through. There are controllers (such as freezer) that >> // Java doesn't care about. Just ignore them. Only listed controllers are >> // considered in the anyCgroupsV1Controller/anyCgroupsV2Controller checks. > > This is not a fall-through because the previous line ends with a `break`. My bad. How about `Intentional incomplete switch. There are ...`? Anyway, why is the empty `default` case needed other than for the comment? - PR: https://git.openjdk.java.net/jdk/pull/8858
Re: RFR: 8287107: CgroupSubsystemFactory.setCgroupV2Path asserts with freezer controller [v2]
On Tue, 24 May 2022 16:15:03 GMT, Ioi Lam wrote: >> This PR fixes a bug found on an Ubuntu host that's mostly running with >> cgroupv2, but there's a controller (freezer) that is mounted in cgroupv1 >> mode. >> >> The container support code in the VM and JDK checks if we have >> simultaneously mounted v1 and v2 containers. If so, we revert to "hybrid" >> mode (which is essentially v1). >> >> However, the "hybrid checks" only considers the following controllers that >> Java cares about: >> >> - cpu >> - cpuacct >> - cpuset >> - blkio >> - memory >> - pids >> >> If only "freezer" is mounted in v1 mode, Java will start in V2 mode. Later, >> when `setCgroupV2Path()` sees the first line in /proc/self/cgroup, it runs >> into the assert. >> >> >> $ cat /proc/self/cgroup >> 1:freezer:/ >> 0::/user.slice/user-1001.slice/session-85.scope >> >> >> The fix is simple -- when we get to `setCgroupV2Path()`, we have already >> decided that the system has not mounted any v1 controllers that we care >> about, so we should just ignore all the v1 controllers (which has a >> non-empty name). > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > @jerboaa comments Looks good. Thanks for the thorough analysis. - Marked as reviewed by sgehwolf (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8858
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v4]
On Wed, 18 May 2022 23:20:45 GMT, Stuart Marks wrote: >>> Need to add apiNote documentation section to capacity-based constructors >>> like for maps. >> >> @liach done. > > @XenoAmess oops, sorry for the delay. I think it would be good to get these > into 19 as companions to `HashMap.newHashMap` et. al. > > As before, I'd suggest reducing the number of changes to use sites in order > to make review easier. I would suggest keeping the changes under src in > java.base, java.net.http, java.rmi, and jdk.zipfs, and omitting all the other > changes. Also keep the changes under test/jdk. > > There needs to be a test for these new methods. I haven't thought much how to > do that. My first attempt would be to modify our favorite WhiteBoxResizeTest > and add a bit of machinery that asserts the table length of the HashMap > contained within the created HashSet/LinkedHashSet. I haven't looked at it > though, so it might not work out, in which case you should pursue an > alternative approach. > > I'll look at the specs and suggest updates as necessary and then handle > filing of a CSR. @stuart-marks > suggest keeping the changes under src in java.base, java.net.http, java.rmi, > and jdk.zipfs, and omitting all the other changes. Also keep the changes > under test/jdk. Done. > modify our favorite WhiteBoxResizeTest and add a bit of machinery that > asserts the table length of the HashMap contained within the created > HashSet/LinkedHashSet. I'm trying. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v9]
> as title. XenoAmess has updated the pull request incrementally with one additional commit since the last revision: revert much too changes for newHashSet - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/363fcc50..07e9b8b0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=07-08 Stats: 59 lines in 27 files changed: 9 ins; 0 del; 50 mod Patch: https://git.openjdk.java.net/jdk/pull/8302.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8302/head:pull/8302 PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284638: store skip buffers in InputStream Object [v12]
On Fri, 20 May 2022 21:15:23 GMT, Roger Riggs wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add documents
>
> src/java.base/share/classes/java/io/InputStream.java line 78:
>
>> 76: SoftReference ref = this.skipBufferReference;
>> 77: byte[] buffer;
>> 78: if (ref == null || (buffer = ref.get()) == null || buffer.length
>> < size) {
>
> A comment here or in the method javadoc to the effect that a new buffer is
> allocated unless the existing buffer is large enough might head off doubt
> that buffer is always non-null at the return. As would inverting the if:
>
>if (ref != null && (buffer = ref.get()) != null && buffer.length >= size) {
> return buffer;
>}
>// allocate new or larger buffer
>buffer = new byte[size];
>this.skipBufferReference = new SoftReference<>(buffer);
>return buffer;
@RogerRiggs done.
-
PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v15]
> @jmehrens what about this then? > I think it safe now(actually this mechanism is learned from Reader) XenoAmess has updated the pull request incrementally with one additional commit since the last revision: invert if; refine javadoc. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5872/files - new: https://git.openjdk.java.net/jdk/pull/5872/files/d082eae4..cdc4e579 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=13-14 Stats: 8 lines in 1 file changed: 5 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5872/head:pull/5872 PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v14]
> @jmehrens what about this then? > I think it safe now(actually this mechanism is learned from Reader) XenoAmess has updated the pull request incrementally with one additional commit since the last revision: refine javadoc, remove The "notice..." statement - Changes: - all: https://git.openjdk.java.net/jdk/pull/5872/files - new: https://git.openjdk.java.net/jdk/pull/5872/files/580a542c..d082eae4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=12-13 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5872/head:pull/5872 PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v12]
On Fri, 20 May 2022 21:08:51 GMT, Roger Riggs wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add documents
>
> src/java.base/share/classes/java/io/InputStream.java line 72:
>
>> 70: *
>> 71: * @param size minimum length that the skip byte array must have.
>> 72: * notice that this param input MUST be equal or less
>> than {@link #MAX_SKIP_BUFFER_SIZE MAX_SKIP_BUFFER_SIZE}.
>
> The "MUST be equal" reads like something will go wrong if that condition
> isn't satisfied.
> This method does not care about the size, except in potentially resizing if
> the requested size is larger.
>
> The "notice..." statement is making a statement about code in the caller, and
> so doesn't belong here.
The "notice..." statement removed.
-
PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v13]
> @jmehrens what about this then? > I think it safe now(actually this mechanism is learned from Reader) XenoAmess has updated the pull request incrementally with one additional commit since the last revision: rename function skipBufferReference to skipBuffer - Changes: - all: https://git.openjdk.java.net/jdk/pull/5872/files - new: https://git.openjdk.java.net/jdk/pull/5872/files/fbc24743..580a542c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=11-12 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/5872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5872/head:pull/5872 PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v12]
On Fri, 20 May 2022 21:05:07 GMT, Roger Riggs wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add documents
>
> src/java.base/share/classes/java/io/InputStream.java line 75:
>
>> 73: * @return the byte array.
>> 74: */
>> 75: private byte[] skipBufferReference(int size) {
>
> The method name would match the return value better if named
> `skipBuffer(size)`.
done.
-
PR: https://git.openjdk.java.net/jdk/pull/5872
Integrated: 8284209: Replace remaining usages of 'a the' in source code
On Wed, 18 May 2022 14:46:42 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > It's the last issue in the series, and it still touches different areas of > the code. This pull request has now been integrated. Changeset: 9b7e42c0 Author:Alexey Ivanov URL: https://git.openjdk.java.net/jdk/commit/9b7e42c0f078db778dda1011d85cd92e3e4eb979 Stats: 74 lines in 40 files changed: 0 ins; 2 del; 72 mod 8284209: Replace remaining usages of 'a the' in source code Reviewed-by: lancea, wetmore, dfuchs, iris, jjg, ihse - PR: https://git.openjdk.java.net/jdk/pull/8771
RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103
Need to use proper synchronization. The CyclicBarriers might move the thread to WAITING state but not BLOCKED. So it should not confuse existing checks. - Commit messages: - 8287200 Changes: https://git.openjdk.java.net/jdk/pull/8874/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8874=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287200 Stats: 9 lines in 1 file changed: 4 ins; 2 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8874.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8874/head:pull/8874 PR: https://git.openjdk.java.net/jdk/pull/8874
Re: RFR: 8287206: Use WrongThreadException for confinement errors
On Tue, 24 May 2022 09:26:44 GMT, Maurizio Cimadamore wrote: > This patch tweaks the foreign API to use the newly added > `WrongThreadException` instead of `IllegalStateException` to report > confinement errors. Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8865
Integrated: JDK-6725221 Standardize obtaining boolean properties with defaults
On Thu, 5 May 2022 16:49:07 GMT, Mark Powers wrote: > JDK-6725221 Standardize obtaining boolean properties with defaults This pull request has now been integrated. Changeset: 6cc4bb11 Author:Mark Powers Committer: Bradford Wetmore URL: https://git.openjdk.java.net/jdk/commit/6cc4bb1169f34bc091cad3e2deec37cd5585e8d5 Stats: 9 lines in 3 files changed: 1 ins; 1 del; 7 mod 6725221: Standardize obtaining boolean properties with defaults Reviewed-by: prr, rriggs - PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v3]
On Tue, 24 May 2022 18:15:45 GMT, Maurizio Cimadamore wrote: >> Constructing indexed var handles using the `MemoryLayout` API produces >> `VarHandle` which do not check the input indices for out-of-bounds >> conditions. >> While this can never result in a VM crash (after all the memory segment will >> protect against "true" OOB access), it is still possible for an access >> expression to refer to parts of a segment that are logically unrelated. >> >> This patch adds a "logical" bound check to all indexed var handles generated >> using the layout API. >> Benchmarks are not affected by the check. Users are still able to create >> custom "unchecked" var handles, using the combinator API in `MethodHandles`. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v3]
> Constructing indexed var handles using the `MemoryLayout` API produces > `VarHandle` which do not check the input indices for out-of-bounds conditions. > While this can never result in a VM crash (after all the memory segment will > protect against "true" OOB access), it is still possible for an access > expression to refer to parts of a segment that are logically unrelated. > > This patch adds a "logical" bound check to all indexed var handles generated > using the layout API. > Benchmarks are not affected by the check. Users are still able to create > custom "unchecked" var handles, using the combinator API in `MethodHandles`. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8868/files - new: https://git.openjdk.java.net/jdk/pull/8868/files/a682cc03..66cf582a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8868=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8868=01-02 Stats: 7 lines in 2 files changed: 1 ins; 2 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8868.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8868/head:pull/8868 PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 18:02:50 GMT, Maurizio Cimadamore wrote: >> Sorry i misread the text, we are talking about the same thing. I think it >> would be clearer to refer `x_i` being in the range of `0` (inclusive) and >> `b_i` (exclusive), otherwise an is thrown. That way in subsequent doc >> on other methods in matches with `B`, which is exclusive. > > yes, but that seems to affect this statement: > > > `0 <= x_i <= b_i, where 0 <= i <= n`, or IndexOutOfBoundsException is thrown. > > So we can replace with > > > 0 <= x_i < b_i, where 1 <= i <= n`, or IndexOutOfBoundsException is thrown. > Sorry i misread the text, we are talking about the same thing. I think it > would be clearer to refer `x_i` being in the range of `0` (inclusive) and > `b_i` (exclusive), otherwise an is thrown. That way in subsequent doc on > other methods in matches with `B`, which is exclusive. I apologize too - I misread your original question and thought it was about the use of ceilDiv :-) - anyway, at least that prodded me to come up with an example that explains why the logic is the way it is :-) - PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults [v2]
On Tue, 24 May 2022 16:20:10 GMT, Mark Powers wrote: > Mach5 tier1 and tier2 completed without any failures. I don't know what to > make of the pre-submit failures - lang and hotspot? IIUC, these are due to Loom failures in some of the other platforms supported by OpenJDK but not by Oracle. They are being resolved. - PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 18:00:46 GMT, Paul Sandoz wrote: >> The terms `x_1, x_2, ... x__n` are defined, but `x_0` is not. >> >> I think you can refer to the first index out of bounds as the exclusive >> upper bound of the range? > > Sorry i misread the text, we are talking about the same thing. I think it > would be clearer to refer `x_i` being in the range of `0` (inclusive) and > `b_i` (exclusive), otherwise an is thrown. That way in subsequent doc on > other methods in matches with `B`, which is exclusive. yes, but that seems to affect this statement: `0 <= x_i <= b_i, where 0 <= i <= n`, or IndexOutOfBoundsException is thrown. So we can replace with 0 <= x_i < b_i, where 1 <= i <= n`, or IndexOutOfBoundsException is thrown. - PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 17:55:13 GMT, Paul Sandoz wrote: >> Here's a concrete example: >> >> Consider a sequence layout with 6 elements. Then: >> >> element count = 6 >> valid indices 0, 1, 2, 3, 4, 5 >> >> Now consider a var handle that is obtained by calling the path element >> method, passing the following parameters >> >> start = 1 >> step = 2 >> >> This sets up the following mapping between logical an physical indices: >> >> 0 -> 1 >> 1 -> 3 >> 2 -> 5 >> >> Where on the LHS we have the logical index (the one passed to the VH) and on >> the RHS we have the actual index it is translated to. >> >> Note that the index map has three elements. So the upper bound (exclusive) >> of the index map is 3 - that is, we can pass indices 0, 1, 2. >> >> According to the formula shown in the javadoc: >> >> B = ceilDiv((elementCount - start) / step); >> >> so: >> >> B = ceilDiv((6 - 1) / 2) >> = ceilDiv(5 / 2) >> >> Note how, w/o ceilDiv we'd get 2 (the last valid index), and not 3 (the >> first invalid index). > > The terms `x_1, x_2, ... x__n` are defined, but `x_0` is not. > > I think you can refer to the first index out of bounds as the exclusive upper > bound of the range? Sorry i misread the text, we are talking about the same thing. I think it would be clearer to refer `x_i` being in the range of `0` (inclusive) and `b_i` (exclusive), otherwise an is thrown. That way in subsequent doc on other methods in matches with `B`, which is exclusive. - PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287064: Modernize ProxyGenerator.PrimitiveTypeInfo [v2]
On Fri, 20 May 2022 22:18:42 GMT, liach wrote:
>> Simplify opcode handling, use `final` in `PrimitiveTypeInfo`, and replace
>> the hash map with a simple lookup, similar to what's done in
>> [JDK-8284880](https://bugs.openjdk.java.net/browse/JDK-8284880) (#8242)
>
> liach has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Convert PrimitiveTypeInfo to an enum
Looks good.
Did you consider switch (class) {...} in PrimitiveTypeInfo.get?
The `if` cascade may be quicker given the expected distribution of lookups.
-
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8801
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 17:53:38 GMT, Maurizio Cimadamore wrote: >> Indices start at zero. The ceilDiv operation is needed so that the operation >> returns the first index outisde the range (it's a bit subtle, sorry, but I >> don't know how else to express). > > Here's a concrete example: > > Consider a sequence layout with 6 elements. Then: > > element count = 6 > valid indices 0, 1, 2, 3, 4, 5 > > Now consider a var handle that is obtained by calling the path element > method, passing the following parameters > > start = 1 > step = 2 > > This sets up the following mapping between logical an physical indices: > > 0 -> 1 > 1 -> 3 > 2 -> 5 > > Where on the LHS we have the logical index (the one passed to the VH) and on > the RHS we have the actual index it is translated to. > > Note that the index map has three elements. So the upper bound (exclusive) of > the index map is 3 - that is, we can pass indices 0, 1, 2. > > According to the formula shown in the javadoc: > > B = ceilDiv((elementCount - start) / step); > > so: > > B = ceilDiv((6 - 1) / 2) > = ceilDiv(5 / 2) > > Note how, w/o ceilDiv we'd get 2 (the last valid index), and not 3 (the first > invalid index). The terms `x_1, x_2, ... x__n` are defined, but `x_0` is not. I think you can refer to the first index out of bounds as the exclusive upper bound of the range? - PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 17:43:52 GMT, Maurizio Cimadamore
wrote:
>> src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 374:
>>
>>> 372: *
>>> 373: * Additionally, the provided dynamic values must conform to some
>>> bound which is derived from the layout path, that is,
>>> 374: * {@code 0 <= x_i <= b_i}, where {@code 0 <= i <= n}, or {@link
>>> IndexOutOfBoundsException} is thrown.
>>
>> Suggestion:
>>
>> * {@code 0 <= x_i < b_i}, where {@code 1 <= i <= n}, or {@link
>> IndexOutOfBoundsException} is thrown.
>>
>> We refer later to `B` being an exclusive upper bound (computed using
>> `ceilDiv`).
>
> Indices start at zero. The ceilDiv operation is needed so that the operation
> returns the first index outisde the range (it's a bit subtle, sorry, but I
> don't know how else to express).
Here's a concrete example:
Consider a sequence layout with 6 elements. Then:
element count = 6
valid indices 0, 1, 2, 3, 4, 5
Now consider a var handle that is obtained by calling the path element method,
passing the following parameters
start = 1
step = 2
This sets up the following mapping between logical an physical indices:
0 -> 1
1 -> 3
2 -> 5
Where on the LHS we have the logical index (the one passed to the VH) and on
the RHS we have the actual index it is translated to.
Note that the index map has three elements. So the upper bound (exclusive) of
the index map is 3 - that is, we can pass indices 0, 1, 2.
According to the formula shown in the javadoc:
B = ceilDiv((elementCount - start) / step);
so:
B = ceilDiv((6 - 1) / 2)
= ceilDiv(5 / 2)
Note how, w/o ceilDiv we'd get 2 (the last valid index), and not 3 (the first
invalid index).
-
PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 16:23:52 GMT, Paul Sandoz wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Tweak javadoc for ValueLayout::arrayElementVarHandle
>
> src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 374:
>
>> 372: *
>> 373: * Additionally, the provided dynamic values must conform to some
>> bound which is derived from the layout path, that is,
>> 374: * {@code 0 <= x_i <= b_i}, where {@code 0 <= i <= n}, or {@link
>> IndexOutOfBoundsException} is thrown.
>
> Suggestion:
>
> * {@code 0 <= x_i < b_i}, where {@code 1 <= i <= n}, or {@link
> IndexOutOfBoundsException} is thrown.
>
> We refer later to `B` being an exclusive upper bound (computed using
> `ceilDiv`).
Indices start at zero. The ceilDiv operation is needed so that the operation
returns the first index outisde the range (it's a bit subtle, sorry, but I
don't know how else to express).
-
PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 15:28:27 GMT, Maurizio Cimadamore
wrote:
>> Constructing indexed var handles using the `MemoryLayout` API produces
>> `VarHandle` which do not check the input indices for out-of-bounds
>> conditions.
>> While this can never result in a VM crash (after all the memory segment will
>> protect against "true" OOB access), it is still possible for an access
>> expression to refer to parts of a segment that are logically unrelated.
>>
>> This patch adds a "logical" bound check to all indexed var handles generated
>> using the layout API.
>> Benchmarks are not affected by the check. Users are still able to create
>> custom "unchecked" var handles, using the combinator API in `MethodHandles`.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Tweak javadoc for ValueLayout::arrayElementVarHandle
src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java line 256:
> 254: private long[] addBound(long maxIndex) {
> 255: long[] newBounds = new long[bounds.length + 1];
> 256: System.arraycopy(bounds, 0, newBounds, 0, bounds.length);
Suggestion:
long[] newBounds = Arrays.copyOf(bounds, bounds.length + 1);
-
PR: https://git.openjdk.java.net/jdk/pull/8868
Integrated: 8281682: Redundant .ico files in Windows app-image cause unnecessary bloat
On Thu, 19 May 2022 19:13:12 GMT, Alexey Semenyuk wrote: > 8281682: Redundant .ico files in Windows app-image cause unnecessary bloat This pull request has now been integrated. Changeset: 45180633 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/45180633d34b6cbb679bae0753d9f422e76d6297 Stats: 169 lines in 8 files changed: 150 ins; 6 del; 13 mod 8281682: Redundant .ico files in Windows app-image cause unnecessary bloat Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/8794
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
On Tue, 24 May 2022 15:28:27 GMT, Maurizio Cimadamore
wrote:
>> Constructing indexed var handles using the `MemoryLayout` API produces
>> `VarHandle` which do not check the input indices for out-of-bounds
>> conditions.
>> While this can never result in a VM crash (after all the memory segment will
>> protect against "true" OOB access), it is still possible for an access
>> expression to refer to parts of a segment that are logically unrelated.
>>
>> This patch adds a "logical" bound check to all indexed var handles generated
>> using the layout API.
>> Benchmarks are not affected by the check. Users are still able to create
>> custom "unchecked" var handles, using the combinator API in `MethodHandles`.
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Tweak javadoc for ValueLayout::arrayElementVarHandle
src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 374:
> 372: *
> 373: * Additionally, the provided dynamic values must conform to some
> bound which is derived from the layout path, that is,
> 374: * {@code 0 <= x_i <= b_i}, where {@code 0 <= i <= n}, or {@link
> IndexOutOfBoundsException} is thrown.
Suggestion:
* {@code 0 <= x_i < b_i}, where {@code 1 <= i <= n}, or {@link
IndexOutOfBoundsException} is thrown.
We refer later to `B` being an exclusive upper bound (computed using `ceilDiv`).
-
PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults [v2]
On Mon, 23 May 2022 18:58:34 GMT, Mark Powers wrote: >> JDK-6725221 Standardize obtaining boolean properties with defaults > > Mark Powers has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - Alan and Jamil comments > - Merge > - reverse true.equals and false.equals changes > - Merge > - Merge > - first iteration Mach5 tier1 and tier2 completed without any failures. I don't know what to make of the pre-submit failures - lang and hotspot? - PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: 8287107: CgroupSubsystemFactory.setCgroupV2Path asserts with freezer controller [v2]
On Tue, 24 May 2022 10:12:31 GMT, Severin Gehwolf wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> @jerboaa comments
>
> src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
> line 155:
>
>> 153: // There are some controllers (such as freezer) that
>> Java doesn't
>> 154: // care about. Just ignore them. These are not
>> considered in the
>> 155: // anyCgroupsV1Controller/anyCgroupsV1Controller checks.
>
> It's not clear why this `default` is necessary. Could we just add the comment
> like so:
>
>
> // Intentional fall-through. There are controllers (such as freezer) that
> // Java doesn't care about. Just ignore them. Only listed controllers are
> // considered in the anyCgroupsV1Controller/anyCgroupsV2Controller checks.
This is not a fall-through because the previous line ends with a `break`.
> src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
> line 229:
>
>> 227: String name = tokens[1];
>> 228: if (!name.equals("")) {
>> 229: // This is probably a v1 controller that we have ignored
>> (e.g., freezer)
>
> Could we add an assertion that the controller isn't in the `infos` map?
>
>if (!name.equals("")) {
> // This must be a v1 controller that we have ignored (e.g.,
> freezer)
> assert infos.get(name) == null;
> return;
> }
Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/8858
Re: RFR: 8287107: CgroupSubsystemFactory.setCgroupV2Path asserts with freezer controller [v2]
> This PR fixes a bug found on an Ubuntu host that's mostly running with > cgroupv2, but there's a controller (freezer) that is mounted in cgroupv1 mode. > > The container support code in the VM and JDK checks if we have simultaneously > mounted v1 and v2 containers. If so, we revert to "hybrid" mode (which is > essentially v1). > > However, the "hybrid checks" only considers the following controllers that > Java cares about: > > - cpu > - cpuacct > - cpuset > - blkio > - memory > - pids > > If only "freezer" is mounted in v1 mode, Java will start in V2 mode. Later, > when `setCgroupV2Path()` sees the first line in /proc/self/cgroup, it runs > into the assert. > > > $ cat /proc/self/cgroup > 1:freezer:/ > 0::/user.slice/user-1001.slice/session-85.scope > > > The fix is simple -- when we get to `setCgroupV2Path()`, we have already > decided that the system has not mounted any v1 controllers that we care > about, so we should just ignore all the v1 controllers (which has a non-empty > name). Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: @jerboaa comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8858/files - new: https://git.openjdk.java.net/jdk/pull/8858/files/c4d8354d..1f17b6e8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8858=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8858=00-01 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8858.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8858/head:pull/8858 PR: https://git.openjdk.java.net/jdk/pull/8858
Re: RFR: 8287206: Use WrongThreadException for confinement errors
On Tue, 24 May 2022 09:26:44 GMT, Maurizio Cimadamore wrote: > This patch tweaks the foreign API to use the newly added > `WrongThreadException` instead of `IllegalStateException` to report > confinement errors. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8865
Integrated: 8287162: (zipfs) Performance regression related to support for POSIX file permissions
On Mon, 23 May 2022 19:47:33 GMT, Lance Andersen wrote: > Hi all, > > This PR addresses the performance issue that is described in JDK-8287162. > > With this fix, the ZipFileSystem methods: initOwner, initGroup, and > initPermissions will not be invoked unless enablePosixFileAttributes=true. > > Mach5 tiers1-3 are currently running and have not encountered any issues. This pull request has now been integrated. Changeset: a10c5597 Author:Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/a10c5597d93c4402bafdbb570437aac052b10027 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod 8287162: (zipfs) Performance regression related to support for POSIX file permissions Reviewed-by: jpai, alanb, clanger - PR: https://git.openjdk.java.net/jdk/pull/8854
Re: RFR: 8287244: Add bound check in indexed memory access var handle [v2]
> Constructing indexed var handles using the `MemoryLayout` API produces > `VarHandle` which do not check the input indices for out-of-bounds conditions. > While this can never result in a VM crash (after all the memory segment will > protect against "true" OOB access), it is still possible for an access > expression to refer to parts of a segment that are logically unrelated. > > This patch adds a "logical" bound check to all indexed var handles generated > using the layout API. > Benchmarks are not affected by the check. Users are still able to create > custom "unchecked" var handles, using the combinator API in `MethodHandles`. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Tweak javadoc for ValueLayout::arrayElementVarHandle - Changes: - all: https://git.openjdk.java.net/jdk/pull/8868/files - new: https://git.openjdk.java.net/jdk/pull/8868/files/453392ae..a682cc03 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8868=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8868=00-01 Stats: 17 lines in 1 file changed: 16 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8868.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8868/head:pull/8868 PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287181: Avoid redundant HashMap.containsKey calls in InternalLocaleBuilder.setExtension
On Sat, 30 Apr 2022 17:10:55 GMT, Andrey Turbanov wrote: > No need to separately perform `HashMap.containsKey` before `HashMap.remove` > call. If key is present - it will be removed anyway. If it's not present, > nothing will be deleted. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8488
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v3]
> This is the implementation of JEP 428: Structured Concurrency (Incubator). > > This is a non-final API that provides a gentle on-ramp to structure a task as > a family of concurrent subtasks, and to coordinate the subtasks as a unit. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Add statement to close about thread termination - Changes: - all: https://git.openjdk.java.net/jdk/pull/8787/files - new: https://git.openjdk.java.net/jdk/pull/8787/files/c72f0330..4fc454a9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8787=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=01-02 Stats: 12 lines in 3 files changed: 5 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/8787.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787 PR: https://git.openjdk.java.net/jdk/pull/8787
Integrated: 8287158: Explicitly reject unsupported call shapes on macos-aarch64 in mainline
On Mon, 23 May 2022 12:27:40 GMT, Jorn Vernee wrote: > Bring in changes from the panama-foreign repo > > These changes pertain to explicitly rejecting unsupported call shapes on > macos-aarch64. > > Original PRs: > 1. https://github.com/openjdk/panama-foreign/pull/676 > 2. https://github.com/openjdk/panama-foreign/pull/677 > > Testing: `jdk_foreign` on linux-aarch64-debug, macosx-aarch64-debug, > linux-x64-debug, macosx-x64-debug, and windows-x64-debug This pull request has now been integrated. Changeset: 8f0eb5d4 Author:Jorn Vernee URL: https://git.openjdk.java.net/jdk/commit/8f0eb5d40178b49fa69a623d057ca00846526319 Stats: 25411 lines in 18 files changed: 719 ins; 24633 del; 59 mod 8287158: Explicitly reject unsupported call shapes on macos-aarch64 in mainline Reviewed-by: mcimadamore, ngasson - PR: https://git.openjdk.java.net/jdk/pull/8842
Re: RFR: 8287244: Add bound check in indexed memory access var handle
On Tue, 24 May 2022 14:40:56 GMT, Maurizio Cimadamore
wrote:
> Constructing indexed var handles using the `MemoryLayout` API produces
> `VarHandle` which do not check the input indices for out-of-bounds conditions.
> While this can never result in a VM crash (after all the memory segment will
> protect against "true" OOB access), it is still possible for an access
> expression to refer to parts of a segment that are logically unrelated.
>
> This patch adds a "logical" bound check to all indexed var handles generated
> using the layout API.
> Benchmarks are not affected by the check. Users are still able to create
> custom "unchecked" var handles, using the combinator API in `MethodHandles`.
src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 537:
> 535: *
> 536: *
> 537: *if {@code F > 0}, then {@code B = ceilDiv(C - S,
> F)}
These formulas come from the formula for computing the accessed index A:
`A = S + I * F`
And then deriving the value for I, by equating `A = C` (for F > 0) and `A = -1`
(for F < 0) - that is equating the accessed index to the "first" out of bound
index. `ceilDiv` ensures there is "some room" between the max/min index and the
selected one.
src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java line 109:
> 107: SequenceLayout seq = (SequenceLayout)layout;
> 108: checkSequenceBounds(seq, index);
> 109: long elemSize = seq.elementLayout().bitSize();
I've simplified the code here, as it still had traces of attempts to avoid the
call to `bitSize` (this method used to be partial).
-
PR: https://git.openjdk.java.net/jdk/pull/8868
Re: RFR: 8287244: Add bound check in indexed memory access var handle
On Tue, 24 May 2022 14:51:10 GMT, Maurizio Cimadamore
wrote:
>> Constructing indexed var handles using the `MemoryLayout` API produces
>> `VarHandle` which do not check the input indices for out-of-bounds
>> conditions.
>> While this can never result in a VM crash (after all the memory segment will
>> protect against "true" OOB access), it is still possible for an access
>> expression to refer to parts of a segment that are logically unrelated.
>>
>> This patch adds a "logical" bound check to all indexed var handles generated
>> using the layout API.
>> Benchmarks are not affected by the check. Users are still able to create
>> custom "unchecked" var handles, using the combinator API in `MethodHandles`.
>
> src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 537:
>
>> 535: *
>> 536: *
>> 537: *if {@code F > 0}, then {@code B = ceilDiv(C - S,
>> F)}
>
> These formulas come from the formula for computing the accessed index A:
>
> `A = S + I * F`
>
> And then deriving the value for I, by equating `A = C` (for F > 0) and `A =
> -1` (for F < 0) - that is equating the accessed index to the "first" out of
> bound index. `ceilDiv` ensures there is "some room" between the max/min index
> and the selected one.
Note also that these complex bound calculation are performed statically, when
we build the layout path. When we're done, we just have an upper bound, which
we can check against using `Objects.checkIndex`.
-
PR: https://git.openjdk.java.net/jdk/pull/8868
RFR: 8287244: Add bound check in indexed memory access var handle
Constructing indexed var handles using the `MemoryLayout` API produces `VarHandle` which do not check the input indices for out-of-bounds conditions. While this can never result in a VM crash (after all the memory segment will protect against "true" OOB access), it is still possible for an access expression to refer to parts of a segment that are logically unrelated. This patch adds a "logical" bound check to all indexed var handles generated using the layout API. Benchmarks are not affected by the check. Users are still able to create custom "unchecked" var handles, using the combinator API in `MethodHandles`. - Commit messages: - Tweak javadoc - Merge branch 'master' into vh_bound_checks - Initial push Changes: https://git.openjdk.java.net/jdk/pull/8868/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8868=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287244 Stats: 73 lines in 3 files changed: 54 ins; 4 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/8868.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8868/head:pull/8868 PR: https://git.openjdk.java.net/jdk/pull/8868
Integrated: 8287137: Problemlist failing x86_32 tests after Loom integration
On Mon, 23 May 2022 12:28:30 GMT, Aleksey Shipilev wrote: > [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot > of x86_32 code. The x86_32 porting is done under > [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, > we can problemlist the failing tests to get cleaner runs for other patches. > This should also make GHA runs cleaner. > > Additional testing: > - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot) > - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot) > - [x] Linux x86_32 fastdebug `tier3` (clean now) > - [x] GHA run This pull request has now been integrated. Changeset: 0a82c4eb Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/0a82c4ebc3748f6dfbbcd72e4421fbe0ea89e0b0 Stats: 279 lines in 3 files changed: 279 ins; 0 del; 0 mod 8287137: Problemlist failing x86_32 tests after Loom integration Reviewed-by: prr, mcimadamore - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Mon, 23 May 2022 18:25:23 GMT, Aleksey Shipilev wrote: >> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot >> of x86_32 code. The x86_32 porting is done under >> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, >> we can problemlist the failing tests to get cleaner runs for other patches. >> This should also make GHA runs cleaner. >> >> Additional testing: >> - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier3` (clean now) >> - [x] GHA run > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Another release test Thank you! - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8285401: Proxy class initializer should use 3-arg `Class.forName` to avoid unnecessary class initialization [v3]
On Tue, 24 May 2022 05:36:30 GMT, Jaikiran Pai wrote: >> liach has updated the pull request incrementally with one additional commit >> since the last revision: >> >> Move the try catch block as it doesn't throw checked exceptions > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 605: > >> 603: mv.visitLdcInsn(Type.getObjectType(dotToSlash(className))); >> 604: mv.visitMethodInsn(INVOKEVIRTUAL, JL_CLASS, >> 605: "getClassLoader", "()" + LJL_CLASSLOADER, false); > > Hello @liach, should this instead be using the (application supplied) > `loader` returned by the call to `ProxyGenerator.getClassLoader()` or maybe > the `loader` member in the `ProxyGenerator` itself? This is equivalent to `jdk.proxy5.$Proxy5.class.getClassLoader()` in Java source code, so this is exactly the application-supplied loader, which also uses the same loader as the previous behavior of `forName` calls. If you want to pass the loader from `ProxyGenerator` to the proxy, it requires complex tricks. Hidden classes won't work due to serialization incompatibility; accessor methods would be defined in `jdk.internal` and exported specifically to the proxy modules, but writing a class so each proxy gets its loader while what I wrote can already do is overkill. > test/jdk/java/lang/reflect/Proxy/LazyInitializationTest.java line 56: > >> 54: >> 55: value.m(new Parameter()); >> 56: Assert.assertTrue(initialized, "parameter type initialized after >> instantiation"); > >> "parameter type initialized after instantiation" > > Since this is the text that gets displayed/reported when the assertion fails, > should this instead be "parameter type not initialized"? The rendered text for testng assert is `message expected: value actual: value`, so on a mismatch, it would print `parameter type initialized after instantiation expected: true actual: false` - PR: https://git.openjdk.java.net/jdk/pull/8800
Re: RFR: 8287181: Avoid redundant HashMap.containsKey calls in InternalLocaleBuilder.setExtension
On Sat, 30 Apr 2022 17:10:55 GMT, Andrey Turbanov wrote: > No need to separately perform `HashMap.containsKey` before `HashMap.remove` > call. If key is present - it will be removed anyway. If it's not present, > nothing will be deleted. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8488
Re: RFR: 8202449: overflow handling in Random.doubles [v3]
> Extend the range of Random.doubles(double, double) and similar methods. Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 8202449: overflow handling in Random.doubles - Changes: - all: https://git.openjdk.java.net/jdk/pull/8791/files - new: https://git.openjdk.java.net/jdk/pull/8791/files/62322ac1..954d1ea2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8791=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8791=01-02 Stats: 21 lines in 2 files changed: 0 ins; 8 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/8791.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8791/head:pull/8791 PR: https://git.openjdk.java.net/jdk/pull/8791
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:52:07 GMT, Maurizio Cimadamore
wrote:
>> Alan Bateman 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:
>>
>> - Use {@code ...}, replace task->subtask, fix typos, add jls ref
>> - Merge
>> - @ignore StructuredThreadDumpTest until test infra in place
>> - Refresh
>> - Merge
>> - Initial commit
>
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 214:
>
>> 212: * Tree structure
>> 213: *
>> 214: * StructuredTaskScopes form a tree where parent-child relations are
>> established
>
> Should we mention what happens in the owner thread completes its execution
> and the scope's `close` method has not been called? I think that, as
> discussed offline, the fact that the thread will attempt to close any nested
> scopes when terminating is an important aspect of this API.
The close method might be the right place for this. It has to specify that an
attempt to close out of order will close the nested scopes and terminating
without close is much the same. I'll see what I can do, thanks for this
suggestion.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Integrated: 8284213: Replace usages of 'a the' in xml
On Wed, 18 May 2022 13:58:22 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > I tried to avoid changing external libraries, there are quite a few such > typos. > Let me know if I should revert any files. This pull request has now been integrated. Changeset: 5974f5fe Author:Alexey Ivanov URL: https://git.openjdk.java.net/jdk/commit/5974f5fed3ef888e8e64b1bf33793a7bcc4ca77c Stats: 17 lines in 9 files changed: 0 ins; 0 del; 17 mod 8284213: Replace usages of 'a the' in xml Reviewed-by: lancea, dmarkov, iris, prr, joehw - PR: https://git.openjdk.java.net/jdk/pull/8769
Integrated: 8284191: Replace usages of 'a the' in hotspot and java.base
On Wed, 18 May 2022 13:27:24 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > Also, I fixed a couple of spelling mistakes. This pull request has now been integrated. Changeset: e0d361ce Author:Alexey Ivanov URL: https://git.openjdk.java.net/jdk/commit/e0d361cea91d3dd1450aece73f660b4abb7ce5fa Stats: 303 lines in 162 files changed: 0 ins; 11 del; 292 mod 8284191: Replace usages of 'a the' in hotspot and java.base Reviewed-by: lancea, wetmore, naoto, iris, kevinw, xuelei - PR: https://git.openjdk.java.net/jdk/pull/8768
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:48:02 GMT, Maurizio Cimadamore
wrote:
>> More generally, I see that you used `{@code ... }` in a lot of places where
>> `{@link ... }` could also be used. In some of those places (like this one)
>> where there is a clear cross-reference, I think `@link` could be
>> preferrable. The only case where `@code` is fine is when referring to the
>> name of the class itself (e.g. `{@code StructuredTaskScope}`). But of course
>> this is subjective.
>
> Also, note the typo `the join is invoked`. Either `the` is dropped, or
> `method` is added. I've seen more than one occurrence of this.
It should be "before join is invoked". It doesn't use a link here because it
already links to join and joinUntil in the previous sentence.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:47:15 GMT, Maurizio Cimadamore
wrote:
>> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
>> line 1110:
>>
>>> 1108: * invoked {@link #join() join} (or {@link
>>> #joinUntil(Instant) joinUntil}).
>>> 1109: * The behavior of this method is unspecified when invoking
>>> this method before
>>> 1110: * the {@code join} is invoked.
>>
>> Suggestion:
>>
>> * {@link #join} is invoked.
>
> More generally, I see that you used `{@code ... }` in a lot of places where
> `{@link ... }` could also be used. In some of those places (like this one)
> where there is a clear cross-reference, I think `@link` could be preferrable.
> The only case where `@code` is fine is when referring to the name of the
> class itself (e.g. `{@code StructuredTaskScope}`). But of course this is
> subjective.
Also, note the typo `the join is invoked`. Either `the` is dropped, or `method`
is added. I've seen more than one occurrence of this.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:41:59 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 428: Structured Concurrency (Incubator).
>>
>> This is a non-final API that provides a gentle on-ramp to structure a task
>> as a family of concurrent subtasks, and to coordinate the subtasks as a unit.
>
> Alan Bateman 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:
>
> - Use {@code ...}, replace task->subtask, fix typos, add jls ref
> - Merge
> - @ignore StructuredThreadDumpTest until test infra in place
> - Refresh
> - Merge
> - Initial commit
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
line 214:
> 212: * Tree structure
> 213: *
> 214: * StructuredTaskScopes form a tree where parent-child relations are
> established
Should we mention what happens in the owner thread completes its execution and
the scope's `close` method has not been called? I think that, as discussed
offline, the fact that the thread will attempt to close any nested scopes when
terminating is an important aspect of this API.
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
line 1110:
> 1108: * invoked {@link #join() join} (or {@link #joinUntil(Instant)
> joinUntil}).
> 1109: * The behavior of this method is unspecified when invoking
> this method before
> 1110: * the {@code join} is invoked.
Suggestion:
* {@link #join} is invoked.
test/jdk/jdk/incubator/concurrent/StructuredTaskScope/StructuredThreadDumpTest.java
line 200:
> 198: }
> 199:
> 200: }
Watch out for the newline here
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
On Tue, 24 May 2022 10:44:39 GMT, Maurizio Cimadamore
wrote:
>> Alan Bateman 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:
>>
>> - Use {@code ...}, replace task->subtask, fix typos, add jls ref
>> - Merge
>> - @ignore StructuredThreadDumpTest until test infra in place
>> - Refresh
>> - Merge
>> - Initial commit
>
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 1110:
>
>> 1108: * invoked {@link #join() join} (or {@link #joinUntil(Instant)
>> joinUntil}).
>> 1109: * The behavior of this method is unspecified when invoking
>> this method before
>> 1110: * the {@code join} is invoked.
>
> Suggestion:
>
> * {@link #join} is invoked.
More generally, I see that you used `{@code ... }` in a lot of places where
`{@link ... }` could also be used. In some of those places (like this one)
where there is a clear cross-reference, I think `@link` could be preferrable.
The only case where `@code` is fine is when referring to the name of the class
itself (e.g. `{@code StructuredTaskScope}`). But of course this is subjective.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator) [v2]
> This is the implementation of JEP 428: Structured Concurrency (Incubator).
>
> This is a non-final API that provides a gentle on-ramp to structure a task as
> a family of concurrent subtasks, and to coordinate the subtasks as a unit.
Alan Bateman 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:
- Use {@code ...}, replace task->subtask, fix typos, add jls ref
- Merge
- @ignore StructuredThreadDumpTest until test infra in place
- Refresh
- Merge
- Initial commit
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8787/files
- new: https://git.openjdk.java.net/jdk/pull/8787/files/6a9553b9..c72f0330
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=8787=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=8787=00-01
Stats: 6142 lines in 192 files changed: 3679 ins; 1909 del; 554 mod
Patch: https://git.openjdk.java.net/jdk/pull/8787.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8787/head:pull/8787
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8287107: CgroupSubsystemFactory.setCgroupV2Path asserts with freezer controller
On Mon, 23 May 2022 22:11:47 GMT, Ioi Lam wrote:
> This PR fixes a bug found on an Ubuntu host that's mostly running with
> cgroupv2, but there's a controller (freezer) that is mounted in cgroupv1 mode.
>
> The container support code in the VM and JDK checks if we have simultaneously
> mounted v1 and v2 containers. If so, we revert to "hybrid" mode (which is
> essentially v1).
>
> However, the "hybrid checks" only considers the following controllers that
> Java cares about:
>
> - cpu
> - cpuacct
> - cpuset
> - blkio
> - memory
> - pids
>
> If only "freezer" is mounted in v1 mode, Java will start in V2 mode. Later,
> when `setCgroupV2Path()` sees the first line in /proc/self/cgroup, it runs
> into the assert.
>
>
> $ cat /proc/self/cgroup
> 1:freezer:/
> 0::/user.slice/user-1001.slice/session-85.scope
>
>
> The fix is simple -- when we get to `setCgroupV2Path()`, we have already
> decided that the system has not mounted any v1 controllers that we care
> about, so we should just ignore all the v1 controllers (which has a non-empty
> name).
src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
line 155:
> 153: // There are some controllers (such as freezer) that
> Java doesn't
> 154: // care about. Just ignore them. These are not
> considered in the
> 155: // anyCgroupsV1Controller/anyCgroupsV1Controller checks.
It's not clear why this `default` is necessary. Could we just add the comment
like so:
// Intentional fall-through. There are controllers (such as freezer) that
// Java doesn't care about. Just ignore them. Only listed controllers are
// considered in the anyCgroupsV1Controller/anyCgroupsV2Controller checks.
src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
line 229:
> 227: String name = tokens[1];
> 228: if (!name.equals("")) {
> 229: // This is probably a v1 controller that we have ignored
> (e.g., freezer)
Could we add an assertion that the controller isn't in the `infos` map?
if (!name.equals("")) {
// This must be a v1 controller that we have ignored (e.g., freezer)
assert infos.get(name) == null;
return;
}
-
PR: https://git.openjdk.java.net/jdk/pull/8858
Integrated: 8287121: Fix typo in jlink's description resource key lookup
On Sun, 22 May 2022 05:58:25 GMT, Christian Stein wrote: > Commit > https://github.com/openjdk/jdk/commit/655500a4f5e3abcff176599604deceefb6ca6640 > for issue [JDK-8286654](https://bugs.openjdk.java.net/browse/JDK-8286654) > added an optional description accessor on the `ToolProvider` interface. It > included a typo in` jlink`'s description resource key lookup: > `"jlink.desciption"` > > This follow-up commit fixes the typo by adding the missing `r` character: > `"jlink.description"`. > > This commit also also adds an automated check that ensures all current and > future tool provider implementations within the JDK don't throw an exception > when invoking their name and description accessor methods. Specific tool > names and descriptions are not expected by this general test. This pull request has now been integrated. Changeset: a0f6dd32 Author:Christian Stein Committer: Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/a0f6dd329139337a5f48557594fa67fa5b9af3eb Stats: 58 lines in 2 files changed: 57 ins; 0 del; 1 mod 8287121: Fix typo in jlink's description resource key lookup Reviewed-by: alanb, lancea - PR: https://git.openjdk.java.net/jdk/pull/8825
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Tue, 24 May 2022 04:18:44 GMT, Joe Darcy wrote:
>> This is the implementation of JEP 428: Structured Concurrency (Incubator).
>>
>> This is a non-final API that provides a gentle on-ramp to structure a task
>> as a family of concurrent subtasks, and to coordinate the subtasks as a unit.
>
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 237:
>
>> 235: * the task result is retrieved via its {@code Future}, or
>> happen-before any actions
>> 236: * taken in a thread after {@linkplain #join() joining} of the task
>> scope.
>> 237: *
>
> Would a @-jls reference to the appropriate section of the memory model
> chapter help here?
Yes, it can reference JLS 17.4.5.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Mon, 23 May 2022 21:09:24 GMT, Maurizio Cimadamore
wrote:
>> This is the implementation of JEP 428: Structured Concurrency (Incubator).
>>
>> This is a non-final API that provides a gentle on-ramp to structure a task
>> as a family of concurrent subtasks, and to coordinate the subtasks as a unit.
>
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 88:
>
>> 86: * {@code join} method after forking.
>> 87: *
>> 88: * StructuredTaskScope defines the {@link #shutdown() shutdown}
>> method to shut down a
>
> This sentence, because of the place where it appears, is a bit confusing. So
> far we only know about the fact that a scope has an owner thread. So it seems
> odd that shutdown could be called _while_ the owner thread is waiting on a
> `join`. Of course, then you read what's next, and you discover that: (a)
> shutdown might be called by a custom scope subclass and that (b) shutdown is
> confined to the threads contained in this task scope - but this definition is
> only given much later.
I see your point. The intention is to introduce all the public methods before
introducing the subclasses or policies. I think I can adjust this sentence to
make it clear that a subtask may call shutdown while the owner is waiting in
join.
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 353:
>
>> 351: *
>> 352: * The {@code handleComplete} method should be thread safe. It
>> may be
>> 353: * invoked by several threads at around the same.
>
> Something is missing? E.g. "at around the same TIME" ? (I'd suggest just
> using "concurrently")
Thanks, it was supposed to say "around the same time" but saying "concurrently"
would be better.
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 376:
>
>> 374: *
>> 375: * If this task scope is {@linkplain #shutdown() shutdown} (or
>> in the process
>> 376: * of shutting down) then {@code fork} returns a Future
>> representing a {@link
>
> Future in plaintext?
Yes, Daniel also pointed this point that there are a few uses of
"StructuredTaskScope" that should also use {@code ...}.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284199: Implementation of Structured Concurrency (Incubator)
On Mon, 23 May 2022 13:11:29 GMT, Daniel Fuchs wrote:
>> This is the implementation of JEP 428: Structured Concurrency (Incubator).
>>
>> This is a non-final API that provides a gentle on-ramp to structure a task
>> as a family of concurrent subtasks, and to coordinate the subtasks as a unit.
>
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 54:
>
>> 52:
>> 53: /**
>> 54: * A basic API for structured concurrency. StructuredTaskScope
>> supports cases
>
> Should StructuredTaskScope in this class-level API doc comment be surrounded
> by `{@code }` to appear in code font?
Okay, there are quite a few of these so I may have to adjust some of the lines
to avoid too many inconsistent line lengths.
-
PR: https://git.openjdk.java.net/jdk/pull/8787
Re: RFR: 8284209: Replace remaining usages of 'a the' in source code [v3]
On Tue, 24 May 2022 09:52:29 GMT, Alexey Ivanov wrote: >> Replaces usages of articles that follow each other in all combinations: >> a/the, an?/an?, the/the… >> >> It's the last issue in the series, and it still touches different areas of >> the code. > > Alexey Ivanov has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright to 2022 Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8771
Re: RFR: 8284209: Replace remaining usages of 'a the' in source code [v3]
> Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > It's the last issue in the series, and it still touches different areas of > the code. Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision: Update copyright to 2022 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8771/files - new: https://git.openjdk.java.net/jdk/pull/8771/files/fab0a4bb..4d529f79 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8771=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8771=01-02 Stats: 24 lines in 24 files changed: 0 ins; 0 del; 24 mod Patch: https://git.openjdk.java.net/jdk/pull/8771.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8771/head:pull/8771 PR: https://git.openjdk.java.net/jdk/pull/8771
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Tue, 24 May 2022 07:01:52 GMT, Aleksey Shipilev wrote: > Please approve, @AlanBateman, @mcimadamore and others? Okay with me. - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287206: Use WrongThreadException for confinement errors
On Tue, 24 May 2022 09:26:44 GMT, Maurizio Cimadamore wrote: > This patch tweaks the foreign API to use the newly added > `WrongThreadException` instead of `IllegalStateException` to report > confinement errors. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8865
Re: RFR: 8287206: Use WrongThreadException for confinement errors
On Tue, 24 May 2022 09:26:44 GMT, Maurizio Cimadamore wrote: > This patch tweaks the foreign API to use the newly added > `WrongThreadException` instead of `IllegalStateException` to report > confinement errors. javadoc: http://cr.openjdk.java.net/~mcimadamore/8287206/v1/javadoc/java.base/module-summary.html specdiff: http://cr.openjdk.java.net/~mcimadamore/8287206/v1/specdiff_out/overview-summary.html - PR: https://git.openjdk.java.net/jdk/pull/8865
RFR: 8287206: Use WrongThreadException for confinement errors
This patch tweaks the foreign API to use the newly added `WrongThreadException` instead of `IllegalStateException` to report confinement errors. - Commit messages: - Merge branch 'master' into wrong_thread_ex - Initial push Changes: https://git.openjdk.java.net/jdk/pull/8865/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8865=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287206 Stats: 254 lines in 12 files changed: 150 ins; 1 del; 103 mod Patch: https://git.openjdk.java.net/jdk/pull/8865.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8865/head:pull/8865 PR: https://git.openjdk.java.net/jdk/pull/8865
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Mon, 23 May 2022 18:25:23 GMT, Aleksey Shipilev wrote: >> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot >> of x86_32 code. The x86_32 porting is done under >> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, >> we can problemlist the failing tests to get cleaner runs for other patches. >> This should also make GHA runs cleaner. >> >> Additional testing: >> - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier3` (clean now) >> - [x] GHA run > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Another release test Marked as reviewed by mcimadamore (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287158: Explicitly reject unsupported call shapes on macos-aarch64 in mainline
On Mon, 23 May 2022 12:27:40 GMT, Jorn Vernee wrote: > Bring in changes from the panama-foreign repo > > These changes pertain to explicitly rejecting unsupported call shapes on > macos-aarch64. > > Original PRs: > 1. https://github.com/openjdk/panama-foreign/pull/676 > 2. https://github.com/openjdk/panama-foreign/pull/677 > > Testing: `jdk_foreign` on linux-aarch64-debug, macosx-aarch64-debug, > linux-x64-debug, macosx-x64-debug, and windows-x64-debug Marked as reviewed by ngasson (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8842
Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]
On Mon, 23 May 2022 18:25:23 GMT, Aleksey Shipilev wrote: >> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot >> of x86_32 code. The x86_32 porting is done under >> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, >> we can problemlist the failing tests to get cleaner runs for other patches. >> This should also make GHA runs cleaner. >> >> Additional testing: >> - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot) >> - [x] Linux x86_32 fastdebug `tier3` (clean now) >> - [x] GHA run > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Another release test GHA run completes fine on x86_32. There are unrelated Windows x86_64 failures in javac land, to be handled separately. Please approve, @AlanBateman, @mcimadamore and others? - PR: https://git.openjdk.java.net/jdk/pull/8843
Re: RFR: 8287162: (zipfs) Performance regression related to support for POSIX file permissions
On Mon, 23 May 2022 19:47:33 GMT, Lance Andersen wrote: > Hi all, > > This PR addresses the performance issue that is described in JDK-8287162. > > With this fix, the ZipFileSystem methods: initOwner, initGroup, and > initPermissions will not be invoked unless enablePosixFileAttributes=true. > > Mach5 tiers1-3 are currently running and have not encountered any issues. Marked as reviewed by clanger (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8854
Re: RFR: 8279031: Add API note to ToolProvider about being reusable/reentrant [v2]
On Tue, 24 May 2022 04:50:58 GMT, Jaikiran Pai wrote:
>> Christian Stein has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update src/java.base/share/classes/java/util/spi/ToolProvider.java
>>
>> Co-authored-by: Anthony Vanelverdinghe
>
> src/java.base/share/classes/java/util/spi/ToolProvider.java line 59:
>
>> 57: * a tool may be the target of multiple {@code run} method invocations,
>> 58: * and reentrant means that multiple invocations of {@code run} may occur
>> 59: * concurrently.
>
> Hello @sormuras,
>
>> reentrant means that multiple invocations of {@code run} may occur
>> concurrently.
>
> My understanding of re-entrant was that the same method could be re-invoked
> on the same thread while the current method execution is in progress (a
> recursion). Whereas "multiple invocations may occur concurrently" sounds more
> like multiple threads invoking this concurrently (i.e. thread-safety). Which
> of these 2 characteristics are we recommending here?
I agree with @jaikiran that reentrant is much more common in the sense of "on
the same thread". Plus that the JDK itself uses "ReentrantLock" in this
meaning. The JDK uses either "thread-safe" or "synchronized" for the "multiple
threads" meaning.
Actually, being thread-safe implies being reusable, so I'd just phrase it as:
> It is recommended that tools implementing this interface are either
> thread-safe, or clearly document any limitations and restrictions.
Also: should this be an `@implNote`, rather than an `@apiNote`, since it's
about "tools implementing this interface"?
-
PR: https://git.openjdk.java.net/jdk/pull/8833
Re: RFR: 8287121: Fix typo in jlink's description resource key lookup
On Tue, 24 May 2022 04:58:44 GMT, Jaikiran Pai wrote:
>> Commit
>> https://github.com/openjdk/jdk/commit/655500a4f5e3abcff176599604deceefb6ca6640
>> for issue [JDK-8286654](https://bugs.openjdk.java.net/browse/JDK-8286654)
>> added an optional description accessor on the `ToolProvider` interface. It
>> included a typo in` jlink`'s description resource key lookup:
>> `"jlink.desciption"`
>>
>> This follow-up commit fixes the typo by adding the missing `r` character:
>> `"jlink.description"`.
>>
>> This commit also also adds an automated check that ensures all current and
>> future tool provider implementations within the JDK don't throw an exception
>> when invoking their name and description accessor methods. Specific tool
>> names and descriptions are not expected by this general test.
>
> test/jdk/java/util/spi/ToolProviderDescriptionTest.java line 40:
>
>> 38: public static void main(String... args) throws Exception {
>> 39: List descriptions = listToolDescriptions();
>> 40: if (descriptions.isEmpty()) {
>
> Hello @sormuras,
> Won't this condition always be "false", because from what I see in this test
> code, the `descriptions` list will never be empty since in the `describeTool`
> method of this test we always return a `String` instance to be added to the
> `List`, even if there's no `description` for the `ToolProvider` instance.
The list will be empty, if no `ToolProvider` service is observable at all. For
example, when running `java` with `--limit-module java.base` - which doesn't
provide an implemention of `ToolProvider` as of today.
This test checks that all observable tools don't throw on calling their
`name()` and `description()` accessors.
This test does not care for proper results being returned.
-
PR: https://git.openjdk.java.net/jdk/pull/8825
