Re: Integrated: 8286562: GCC 12 reports some compiler warnings
The new assertion in src/hotspot/share/utilities/globalDefinitions.hpp
inline const char* type2name(BasicType t) {
assert((uint)t < T_CONFLICT + 1, "invalid type");
return type2name_tab[t];
}
is failing with test
compiler/jvmci/errors/TestInvalidDebugInfo.java
I have filed:
https://bugs.openjdk.java.net/browse/JDK-8287491
The test will probably need ProblemListing as it is causing major noise
in our CI.
David
On 28/05/2022 12:12 pm, Yasumasa Suenaga wrote:
On Wed, 11 May 2022 05:58:31 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:
This pull request has now been integrated.
Changeset: 410a25d5
Author:Yasumasa Suenaga
URL:
https://git.openjdk.java.net/jdk/commit/410a25d59a11b6a627bbb0a2c405c2c2be19f464
Stats: 41 lines in 5 files changed: 26 ins; 1 del; 14 mod
8286562: GCC 12 reports some compiler warnings
Reviewed-by: ihse, kbarrett, prr
-
PR: https://git.openjdk.java.net/jdk/pull/8646
Integrated: 8286562: GCC 12 reports some compiler warnings
On Wed, 11 May 2022 05:58:31 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: This pull request has now been integrated. Changeset: 410a25d5 Author:Yasumasa Suenaga URL: https://git.openjdk.java.net/jdk/commit/410a25d59a11b6a627bbb0a2c405c2c2be19f464 Stats: 41 lines in 5 files changed: 26 ins; 1 del; 14 mod 8286562: GCC 12 reports some compiler warnings Reviewed-by: ihse, kbarrett, prr - PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Fri, 27 May 2022 21:53:38 GMT, Roger Riggs wrote: > As for retaining SUPER, both SUPER and IDENTITY have location class and > according to the BasicAccessFlagTest, the bit patterns should be disjoint If the defined set of class locations, considered over time, have non-disjoint definitions, then the test would need to be modified accordingly of course. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str) [v2]
On Fri, 27 May 2022 20:16:12 GMT, Joe Darcy wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8233760: Result of BigDecimal.toString throws overflow exception on new
>> BigDecimal(str)
>
> test/jdk/java/math/BigDecimal/StringConstructor.java line 69:
>
>> 67: constructWithError("0.01e"+Integer.MIN_VALUE);
>> 68: constructWithError("1e"+((long)Integer.MIN_VALUE-1));
>> 69:
>
> Please add some test cases to demonstrate that the round-tripping that
> previous did not work is functional after the fix.
Tests were added in the new commit.
-
PR: https://git.openjdk.java.net/jdk/pull/8905
Re: RFR: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str) [v2]
> BigDecimal(String) currently fails to accept some strings produced by > BigDecimal.toString(). This PR removes this limitation. Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str) - Changes: - all: https://git.openjdk.java.net/jdk/pull/8905/files - new: https://git.openjdk.java.net/jdk/pull/8905/files/e82cece0..2a25c359 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8905=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8905=00-01 Stats: 22 lines in 1 file changed: 21 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8905.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8905/head:pull/8905 PR: https://git.openjdk.java.net/jdk/pull/8905
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v8]
> Please review this change to replace the finalizer in > `AbstractLdapNamingEnumeration` with a Cleaner. > > The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult > res`, and `LdapClient enumClnt`) are moved to a static inner class . From > there, the change is fairly mechanical. > > Details of note: > 1. Some operations need to change the state values (the update() method is > probably the most interesting). > 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read > `homeCtx` from the superclass's `state`. > > The test case is based on a copy of > `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case > might be possible, but this was done for expediency. > > The test only confirms that the new Cleaner use does not keep the object > reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` > or `LdapBindingEnumeration`, though all are subclasses of > `AbstractLdapNamingEnumeration`). > > Thanks. Brent Christian has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits: - Merge branch 'master' into remove-finalizers - Threading-related fixes - NOW it builds - Fix the merge fix - Fix merge - Merge - Rename inner class to EnumCtx ; use homeCtx() in AbstractLdapNamingEnumeration for consistencty ; new instance vars can be final - fix whitespace - Merge branch 'master' into remove-finalizers - Test changes to test new cleaner code - ... and 4 more: https://git.openjdk.java.net/jdk/compare/ed8e8ac2...4af66bff - Changes: https://git.openjdk.java.net/jdk/pull/8311/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8311=07 Stats: 579 lines in 6 files changed: 309 ins; 101 del; 169 mod Patch: https://git.openjdk.java.net/jdk/pull/8311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8311/head:pull/8311 PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v7]
On Fri, 27 May 2022 22:00:24 GMT, Brent Christian wrote: >> Please review this change to replace the finalizer in >> `AbstractLdapNamingEnumeration` with a Cleaner. >> >> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult >> res`, and `LdapClient enumClnt`) are moved to a static inner class . From >> there, the change is fairly mechanical. >> >> Details of note: >> 1. Some operations need to change the state values (the update() method is >> probably the most interesting). >> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read >> `homeCtx` from the superclass's `state`. >> >> The test case is based on a copy of >> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test >> case might be possible, but this was done for expediency. >> >> The test only confirms that the new Cleaner use does not keep the object >> reachable. It only tests `LdapSearchEnumeration` (not >> `LdapNamingEnumeration` or `LdapBindingEnumeration`, though all are >> subclasses of `AbstractLdapNamingEnumeration`). >> >> Thanks. > > Brent Christian has updated the pull request incrementally with one > additional commit since the last revision: > > Threading-related fixes `AbstractLdapEnumeration`'s mutable state brings the possibility of threading issues between the program and cleaner threads. I've added some threading-related changes to the fix. See my [comment in the bug report](https://bugs.openjdk.java.net/browse/JDK-8283660?focusedCommentId=14498566=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14498566) for additional background. Since synchronization may now happen on the cleaner thread, I've changed `AbstractLdapEnumeration` to use its own `Cleaner` instance instead of the shared cleaner, for added safety. There have been deadlocks in ldap cleanup in the past. The added `finally` blocks led to a lot of indentation changes. The "hide whitespace" option might help with viewing the changes. - PR: https://git.openjdk.java.net/jdk/pull/8311
Withdrawn: JDK-8242888: Convert dynamic proxy to hidden classes
On Sun, 17 Apr 2022 16:17:30 GMT, liach wrote:
> Convert dynamic proxies to hidden classes. Modifies the serialization of
> proxies (requires change in "Java Object Serialization Specification"). Makes
> the proxies hidden in stack traces. Removes duplicate logic in proxy building.
>
> The main compatibility changes and their rationales are:
> 1. Modification to the serialization specification: In the "An instance of
> the class is allocated... The contents restored appropriately" section, I
> propose explicitly state that handling of proxies are unspecified as to allow
> implementation freedom, though I've seen deliberate attempts for proxies to
> implement interfaces with `readResolve` in order to control their
> serialization behavior.
>- This is for the existing generated constructor accessor is
> bytecode-based, which cannot refer to hidden classes.
>- An alternative is to preserve the behavior, where the serialization
> constructor calls `invokespecial` on the closest serializable superclass'
> no-arg constructor, like in #1830 by @DasBrain.
>- My rationale against preservation is such super calls are unsafe and
> should be discouraged in the long term. Calling the existing constructor with
> a dummy argument, as in my implementation, would be more safe.
> 2. The disappearance of proxies in stack traces.
>- Same behavior exists in lambda expressions: For instance, in
> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method,
> implemented by the lambda, will not appear in the stack trace, and isn't too
> problematic.
>
> A summary of the rest of the changes:
> 1. Merged the two passes of determining module and package of the proxy into
> one. This reduced a lot of code and allowed anchor class (for hidden class
> creation) selection be done together as well.
> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of
> `java.base`. This API is intended for implementation of legacy (pre
> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more
> complex tricks to obtain proper permissions as lookups.
> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959):
> passes methods computed by proxy generator as class data to the hidden proxy
> class to reduce generated proxy class size and improve performance.
>
> In addition, since this change is somewhat large, should we keep the old
> proxy generator as well and have it toggled through a command-line flag (like
> the old v49 proxy generator or the old reflection implementation)?
>
> Please feel free to comment or review. This change definitely requires a CSR,
> but I have yet to determine what specifications should be changed.
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.java.net/jdk/pull/8278
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v7]
> Please review this change to replace the finalizer in > `AbstractLdapNamingEnumeration` with a Cleaner. > > The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult > res`, and `LdapClient enumClnt`) are moved to a static inner class . From > there, the change is fairly mechanical. > > Details of note: > 1. Some operations need to change the state values (the update() method is > probably the most interesting). > 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read > `homeCtx` from the superclass's `state`. > > The test case is based on a copy of > `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case > might be possible, but this was done for expediency. > > The test only confirms that the new Cleaner use does not keep the object > reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` > or `LdapBindingEnumeration`, though all are subclasses of > `AbstractLdapNamingEnumeration`). > > Thanks. Brent Christian has updated the pull request incrementally with one additional commit since the last revision: Threading-related fixes - Changes: - all: https://git.openjdk.java.net/jdk/pull/8311/files - new: https://git.openjdk.java.net/jdk/pull/8311/files/a48a56cc..ef8c1410 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8311=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8311=05-06 Stats: 371 lines in 5 files changed: 122 ins; 87 del; 162 mod Patch: https://git.openjdk.java.net/jdk/pull/8311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8311/head:pull/8311 PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 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 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/f2d5c3d7...05cf2d8b As for retaining SUPER, both SUPER and IDENTITY have location class and according to the BasicAccessFlagTest, the bit patterns should be disjoint - PR: https://git.openjdk.java.net/jdk/pull/7445
Integrated: 8284400: Improve XPath exception handling
On Fri, 27 May 2022 01:12:18 GMT, Joe Wang wrote: > Addresses an insufficiency of error handling in the XPath implementation. > Some cleanup where appropriate, without attempting to do too much as this is > a component that hasn't had much changes for 15 years, a fairly stable > application. > > Test: tier2 passed > JCK: JCK XML tests passed This pull request has now been integrated. Changeset: ed8e8ac2 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/ed8e8ac2892af3a0a70b95330e01ec976d3fea3c Stats: 863 lines in 11 files changed: 617 ins; 179 del; 67 mod 8284400: Improve XPath exception handling Reviewed-by: lancea, naoto - PR: https://git.openjdk.java.net/jdk/pull/8910
Re: RFR: 8287440: Typo in package-info.java of java.util.random
On Wed, 18 May 2022 11:45:46 GMT, Anthony Vanelverdinghe wrote: > The change is trivial, but I'll need help from someone to create a JBS issue > & sponsor this PR. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8766
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 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 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/fb4068e0...05cf2d8b Please comment here: https://github.com/openjdk/valhalla/pull/698 The AccessFlag API identifies Modifier as *the* primary definition of source elements. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 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 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/43363358...05cf2d8b With the `AccessFlag` API, what is the role of the `Modifier` API going forward? [Value Objects JEP](https://openjdk.java.net/jeps/8277163) defines the new `identity` and `value` modifiers. [PR #698](https://github.com/openjdk/valhalla/pull/698) proposes to add `Modifier.IDENTITY` and `Modifier.VALUE` constants as the `identity` and `value` modifiers are encoded in a class file using the `ACC_IDENTITY` and `ACC_VALUE` flags. However, with the new improved `AccessFlag` API, the new flags will be defined in the `AccessFlag` API. I think we should avoid adding the new flags in `Modifier` and leave it for the existing usage. Use `AccessFlag` for new features. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str)
On Thu, 26 May 2022 18:02:14 GMT, Raffaello Giulietti
wrote:
> BigDecimal(String) currently fails to accept some strings produced by
> BigDecimal.toString(). This PR removes this limitation.
test/jdk/java/math/BigDecimal/StringConstructor.java line 69:
> 67: constructWithError("0.01e"+Integer.MIN_VALUE);
> 68: constructWithError("1e"+((long)Integer.MIN_VALUE-1));
> 69:
Please add some test cases to demonstrate that the round-tripping that previous
did not work is functional after the fix.
-
PR: https://git.openjdk.java.net/jdk/pull/8905
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 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 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/2ff12e53...05cf2d8b I have the same view as Joe. `ACC_SUPER` has been specified in JVMS and set in class files of an older version. If `ACC_SUPER` would be removed from JVMS before this API becomes final, the `AccessFlag` API would be revised and not to define it at that time. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles [v2]
> In preparation of #8855 this PR refactors the conversions from `List` to > array and array to `List`, reducing the number of conversions when calling > `MethodHandles.dropArguments` in particular. This remove about ~5% of > allocations on the `StringConcatFactoryBootstraps` microbenchmark. Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Review comments, eagerly convert sooner in tryFinally - Changes: - all: https://git.openjdk.java.net/jdk/pull/8923/files - new: https://git.openjdk.java.net/jdk/pull/8923/files/019e2ef8..15ff2125 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8923=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8923=00-01 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8923.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8923/head:pull/8923 PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287440: Typo in package-info.java of java.util.random
On Wed, 18 May 2022 11:45:46 GMT, Anthony Vanelverdinghe wrote: > The change is trivial, but I'll need help from someone to create a JBS issue > & sponsor this PR. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8766
Re: RFR: 8287440: Typo in package-info.java of java.util.random
On Wed, 18 May 2022 11:45:46 GMT, Anthony Vanelverdinghe wrote: > The change is trivial, but I'll need help from someone to create a JBS issue > & sponsor this PR. Hello @anthonyvdotbe, I've created https://bugs.openjdk.java.net/browse/JDK-8287440 for this change. Please edit the title of this PR to "8287440: Typo in package-info.java of java.util.random" so that it triggers the review process. - PR: https://git.openjdk.java.net/jdk/pull/8766
RFR: 8287440: Typo in package-info.java of java.util.random
The change is trivial, but I'll need help from someone to create a JBS issue & sponsor this PR. - Commit messages: - Copy edit: remove pleonasm Changes: https://git.openjdk.java.net/jdk/pull/8766/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8766=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287440 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8766.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8766/head:pull/8766 PR: https://git.openjdk.java.net/jdk/pull/8766
Re: RFR: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException [v2]
> 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Fix pull request comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/8400/files - new: https://git.openjdk.java.net/jdk/pull/8400/files/6d091b81..f83dde2f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8400=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8400=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8400.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8400/head:pull/8400 PR: https://git.openjdk.java.net/jdk/pull/8400
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 19:27:44 GMT, ExE Boss wrote: > If `parameterList` is too slow for `List.of` copies the backing parameter > types array, wouldn't calling > `JavaUtilCollectionAccess::listFromTrustedArray` a better alternative, as it > only allocates one wrapper and has better performance on `subList` than > `Arrays.copyOfRange`? Good observation that `MethodType::parameterList()` is a plausible candidate for `listFromTrustedArray`. That could be a good standalone and orthogonal RFE. I recall @rose00 musing about how he'd prefer it if `MethodType` and friends would have been implemented with `List.of`-style immutable lists rather than arrays. While I agree with him in principle we now have a mix of both worlds where we're converting between either representation haphazardly, making the implementation more complex in the process. It seems better for now to streamline the implementation with an early conversion to what we use internally (arrays) and stick with that. - PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 16:08:03 GMT, liach wrote: > If `parameterList` is too slow for `List.of` copies the backing parameter > types array, wouldn't calling > `JavaUtilCollectionAccess::listFromTrustedArray` a better alternative, as it > only allocates one wrapper and has better performance on `subList` than > `Arrays.copyOfRange`? No, because `listFromTrustedArray` doesn’t allow invocation with array classes other than `Object[].class`: https://github.com/openjdk/jdk/blob/6520843f86f638fe4d1e5b3358fab5799daca654/src/java.base/share/classes/java/util/ImmutableCollections.java#L195-L222 - PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
On Tue, 26 Apr 2022 15:04:15 GMT, Thiago Henrique Hüpner
wrote:
> 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
src/java.base/share/classes/java/lang/InterruptedException.java line 35:
> 33: * this exception. The following code can be used to achieve
> 34: * this effect:
> 35: * {@snippet:
I recommend using `{@snippet lang=java :` instead, like in
https://github.com/openjdk/jdk/blob/6520843f86f638fe4d1e5b3358fab5799daca654/src/java.compiler/share/classes/javax/tools/JavaFileManager.java#L84
-
PR: https://git.openjdk.java.net/jdk/pull/8400
Re: RFR: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
On Tue, 26 Apr 2022 15:04:15 GMT, Thiago Henrique Hüpner wrote: > 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException (not a maintainer) You can make one yourself in https://bugreport.java.com. - PR: https://git.openjdk.java.net/jdk/pull/8400
Re: RFR: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
On Tue, 26 Apr 2022 15:04:15 GMT, Thiago Henrique Hüpner wrote: > 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException If this is a valid change, could someone please create a Jira issue to link to it? - PR: https://git.openjdk.java.net/jdk/pull/8400
RFR: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException - Commit messages: - Use snippet tag instead of pre tag Changes: https://git.openjdk.java.net/jdk/pull/8400/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8400=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287353 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8400.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8400/head:pull/8400 PR: https://git.openjdk.java.net/jdk/pull/8400
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v16]
On Fri, 27 May 2022 16:19:56 GMT, Naoto Sato wrote: >> XenoAmess has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'stuart-marks/pull/8302' into fix_8284780 >> >># Conflicts: >># src/java.base/share/classes/java/util/HashSet.java >># src/java.base/share/classes/java/util/LinkedHashSet.java >> - Minor terminology fixes: change "item" and "key" to element; remove >>"insertion-ordered" from LinkedHashSet static factory method because >>LHS is implicit insertion-ordered. > > src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java line > 227: > >> 225: } >> 226: StringTokenizer tokens = new >> StringTokenizer(supportedLocaleString); >> 227: Set tagset = HashSet.newHashSet(tokens.countTokens()); > > Hi @XenoAmess, during the refactoring of the above `StringTokenizer`, it > turned out that this `HashSet` can be replaced with `Set.of()`. So you can > leave this piece as it is, as I will take care of it with JDK-8287340. @naotoj done. > src/java.base/share/classes/sun/util/locale/provider/JRELocaleProviderAdapter.java > line 461: > >> 459: } >> 460: StringTokenizer tokens = new >> StringTokenizer(supportedLocaleString); >> 461: Set tagset = HashSet.newHashSet(tokens.countTokens()); > > Same as above. @naotoj done. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
> as title. XenoAmess has updated the pull request incrementally with one additional commit since the last revision: do it as naotoj said - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/230a767e..98bfb0e1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=16 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=15-16 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 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: 8287430 MemorySessionImpl::addOrCleanupIfFail does not rethrow exceptions
On Fri, 27 May 2022 10:29:14 GMT, Maurizio Cimadamore wrote: > This patch fix a missing rethrow in `MemorySessionImpl::addOrCleanupIfFail`. > As noted in the JBS issue, this bug does not affect correctness, but it > delays error reporting. > > Writing a test for this is nearly impossible, given that (a) a memory > resource created against a closed session would be inaccessible by clients > (because the session is closed!), and (b) because of the narrow window in > which the problem might manifest (for this problem to occur, a session state > change would have to occur between the first state check and when the cleanup > action list is updated). Marked as reviewed by jvernee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8917
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v2]
On Thu, 26 May 2022 23:05:32 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8287384: Speed up jdk.test.lib.util.ForceGC
On Thu, 26 May 2022 18:50:07 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have this test update reviewed? > > The ForceGC could be enhanced by using smaller wait/sleep time, and shared > cleaner. > > Thanks, > Xuelei Marked as reviewed by bchristi (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8907
Re: RFR: 8284400: Improve XPath exception handling [v2]
On Fri, 27 May 2022 17:17:28 GMT, Joe Wang wrote: >> Addresses an insufficiency of error handling in the XPath implementation. >> Some cleanup where appropriate, without attempting to do too much as this is >> a component that hasn't had much changes for 15 years, a fairly stable >> application. >> >> Test: tier2 passed >> JCK: JCK XML tests passed > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > update copyright year and LastModified tag Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8910
Re: RFR: 8284400: Improve XPath exception handling
On Fri, 27 May 2022 01:12:18 GMT, Joe Wang wrote: > Addresses an insufficiency of error handling in the XPath implementation. > Some cleanup where appropriate, without attempting to do too much as this is > a component that hasn't had much changes for 15 years, a fairly stable > application. > > Test: tier2 passed > JCK: JCK XML tests passed Thanks Lance, Naoto! Updated copyright year and LastModified tag. - PR: https://git.openjdk.java.net/jdk/pull/8910
Re: RFR: 8284400: Improve XPath exception handling [v2]
> Addresses an insufficiency of error handling in the XPath implementation. > Some cleanup where appropriate, without attempting to do too much as this is > a component that hasn't had much changes for 15 years, a fairly stable > application. > > Test: tier2 passed > JCK: JCK XML tests passed Joe Wang has updated the pull request incrementally with one additional commit since the last revision: update copyright year and LastModified tag - Changes: - all: https://git.openjdk.java.net/jdk/pull/8910/files - new: https://git.openjdk.java.net/jdk/pull/8910/files/a03cb0d4..12575c95 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8910=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8910=00-01 Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8910.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8910/head:pull/8910 PR: https://git.openjdk.java.net/jdk/pull/8910
Re: RFR: 8284400: Improve XPath exception handling
On Fri, 27 May 2022 01:12:18 GMT, Joe Wang wrote: > Addresses an insufficiency of error handling in the XPath implementation. > Some cleanup where appropriate, without attempting to do too much as this is > a component that hasn't had much changes for 15 years, a fairly stable > application. > > Test: tier2 passed > JCK: JCK XML tests passed LGTM. Nit: some files have old copyright years and/or `@LastModified` fields - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8910
Re: RFR: 8284400: Improve XPath exception handling
On Fri, 27 May 2022 01:12:18 GMT, Joe Wang wrote: > Addresses an insufficiency of error handling in the XPath implementation. > Some cleanup where appropriate, without attempting to do too much as this is > a component that hasn't had much changes for 15 years, a fairly stable > application. > > Test: tier2 passed > JCK: JCK XML tests passed Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8910
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v16]
On Fri, 27 May 2022 06:14:13 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge remote-tracking branch 'stuart-marks/pull/8302' into fix_8284780 > ># Conflicts: ># src/java.base/share/classes/java/util/HashSet.java ># src/java.base/share/classes/java/util/LinkedHashSet.java > - Minor terminology fixes: change "item" and "key" to element; remove >"insertion-ordered" from LinkedHashSet static factory method because >LHS is implicit insertion-ordered. src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java line 227: > 225: } > 226: StringTokenizer tokens = new > StringTokenizer(supportedLocaleString); > 227: Set tagset = HashSet.newHashSet(tokens.countTokens()); Hi @XenoAmess, during the refactoring of the above `StringTokenizer`, it turned out that this `HashSet` can be replaced with `Set.of()`. So you can leave this piece as it is, as I will take care of it with JDK-8287340. src/java.base/share/classes/sun/util/locale/provider/JRELocaleProviderAdapter.java line 461: > 459: } > 460: StringTokenizer tokens = new > StringTokenizer(supportedLocaleString); > 461: Set tagset = HashSet.newHashSet(tokens.countTokens()); Same as above. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 14:38:27 GMT, Claes Redestad wrote:
>> src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 5266:
>>
>>> 5264: */
>>> 5265: public static MethodHandle dropArguments(MethodHandle target, int
>>> pos, List> valueTypes) {
>>> 5266: return dropArguments(target, pos, valueTypes.toArray(new
>>> Class[0]).clone(), true);
>>
>> Isn't this call to `clone()` unnecessary, as `valueTypes.toArray` should
>> either return the passed empty array, or a newly created array?
>
> It might be a bit too paranoid in this instance (since we don't keep the
> array around for long), but not cloning the result of calling `toArray` on an
> arbitrary and possibly adversary `List` could open up for TOCTOU race bugs /
> attacks. The existing code was being paranoid and copying and I don't want to
> weaken something that could have security implications without double- and
> triple-checking that it's safe to do so.
You can probably call the `dropArguments` with `false` for `trusted` instead.
-
PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 14:18:19 GMT, Claes Redestad wrote: > In preparation of #8855 this PR refactors the conversions from `List` to > array and array to `List`, reducing the number of conversions when calling > `MethodHandles.dropArguments` in particular. This remove about ~5% of > allocations on the `StringConcatFactoryBootstraps` microbenchmark. If `parameterList` is too slow for `List.of` copies the backing parameter types array, wouldn't calling `JavaUtilCollectionAccess::listFromTrustedArray` a better alternative, as it only allocates one wrapper and has better performance on `subList` than `Arrays.copyOfRange`? - PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 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 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/75552cfd...05cf2d8b To reinforce the argument not to propagate ACC_SUPER to the new API. It is NOT currently defined in j.l.reflect.Modifier, so there are no existing dependencies on a symbol. Class.getModifiers() *never* returns ACC_SUPER; the VM explicitly removes it (since 2007, for all class file versions). The note in the JVMS spec indicates it is retained only for source compatibility with earlier releases. The JVMS spec considers the flag to always be set. and It is always unset in the modifiers returned from getModifiers(). Defining the value where it has not been defined before has no useful purpose. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 14:18:19 GMT, Claes Redestad wrote:
> In preparation of #8855 this PR refactors the conversions from `List` to
> array and array to `List`, reducing the number of conversions when calling
> `MethodHandles.dropArguments` in particular. This remove about ~5% of
> allocations on the `StringConcatFactoryBootstraps` microbenchmark.
src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 5266:
> 5264: */
> 5265: public static MethodHandle dropArguments(MethodHandle target, int
> pos, List> valueTypes) {
> 5266: return dropArguments(target, pos, valueTypes.toArray(new
> Class[0]).clone(), true);
Isn't this call to `clone()` unnecessary, as `valueTypes.toArray` should either
return the passed empty array, or a newly created array?
-
PR: https://git.openjdk.java.net/jdk/pull/8923
RFR: 8281695: jtreg test com/sun/jndi/ldap/LdapPoolTimeoutTest.java fails intermittently in nightly run
Test change to silently pass if the test environment encounters a NoRouteToHostException. These are intermittent at the moment but I will attempt to find an alternative to the use of example.com in some follow up work. - Commit messages: - 8281695: jtreg test com/sun/jndi/ldap/LdapPoolTimeoutTest.java fails intermittently in nightly run Changes: https://git.openjdk.java.net/jdk/pull/8925/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8925=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281695 Stats: 9 lines in 1 file changed: 7 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8925.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8925/head:pull/8925 PR: https://git.openjdk.java.net/jdk/pull/8925
Integrated: 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer
On Wed, 25 May 2022 23:08:38 GMT, Brian Burkhalter wrote: > If only a leftover `char` remains in the stream, do not add `-1` to the > return value in `lockedRead()`. This pull request has now been integrated. Changeset: 6520843f Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/6520843f86f638fe4d1e5b3358fab5799daca654 Stats: 33 lines in 2 files changed: 24 ins; 1 del; 8 mod 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer Reviewed-by: jpai, rriggs - PR: https://git.openjdk.java.net/jdk/pull/8895
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 14:18:19 GMT, Claes Redestad wrote: > In preparation of #8855 this PR refactors the conversions from `List` to > array and array to `List`, reducing the number of conversions when calling > `MethodHandles.dropArguments` in particular. This remove about ~5% of > allocations on the `StringConcatFactoryBootstraps` microbenchmark. It might be a bit too paranoid in this instance (since we don't keep the array around for long), but not cloning the result of calling `toArray` on an arbitrary and possibly adversary `List` could open up for TOCTOU race bugs / attacks. The existing code was being paranoid and copying and I don't want to weaken something that could have security implications without double- and triple-checking that it's safe to do so. - PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 14:18:19 GMT, Claes Redestad wrote: > In preparation of #8855 this PR refactors the conversions from `List` to > array and array to `List`, reducing the number of conversions when calling > `MethodHandles.dropArguments` in particular. This remove about ~5% of > allocations on the `StringConcatFactoryBootstraps` microbenchmark. Another datapoint is the `MethodHandlesDropArguments.interCreate` microbenchmark which explicitly tests `MethodHandles.dropArguments` (with a minimal argument list): Before: BenchmarkMode Cnt ScoreError Units MethodHandlesDropArguments.interCreate avgt 5 136.778 ? 1.265 ns/op MethodHandlesDropArguments.interCreate:?gc.alloc.rate.norm avgt 5 264.020 ? 0.021B/op Patch: BenchmarkMode Cnt Score Error Units MethodHandlesDropArguments.interCreate avgt 5 120.536 ? 3.133 ns/op MethodHandlesDropArguments.interCreate:?gc.alloc.rate.norm avgt 5 220.818 ? 41.309B/op - PR: https://git.openjdk.java.net/jdk/pull/8923
RFR: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
In preparation of #8855 this PR refactors the conversions from `List` to array and array to `List`, reducing the number of conversions when calling `MethodHandles.dropArguments` in particular. This remove about ~5% of allocations on the `StringConcatFactoryBootstraps` microbenchmark. - Commit messages: - Move all conversions closer to the edge - Merge master - Streamline MHs.dropArguments parameter handling - Missed correctly taking b1 into account in of(byte, int, int...) (java/lang/String/concat/ImplicitStringConcatShapes.java test failure) - Address review comments, fix unsafe shift, rework and remove ofBothArrays - Revert unrelated and unverified hashCode change - Improve TransformKey to pack more kinds of transforms efficiently Changes: https://git.openjdk.java.net/jdk/pull/8923/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8923=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287442 Stats: 46 lines in 3 files changed: 9 ins; 3 del; 34 mod Patch: https://git.openjdk.java.net/jdk/pull/8923.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8923/head:pull/8923 PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer
On Wed, 25 May 2022 23:08:38 GMT, Brian Burkhalter wrote: > If only a leftover `char` remains in the stream, do not add `-1` to the > return value in `lockedRead()`. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8895
Re: RFR: 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer
On Wed, 25 May 2022 23:08:38 GMT, Brian Burkhalter wrote: > If only a leftover `char` remains in the stream, do not add `-1` to the > return value in `lockedRead()`. Looks fine to me. - Marked as reviewed by jpai (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8895
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v2]
On Thu, 26 May 2022 23:05:32 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Integrated: 8287292: Improve TransformKey to pack more kinds of transforms efficiently
On Wed, 25 May 2022 09:38:08 GMT, Claes Redestad wrote: > The bespoke caching scheme in `jl.invoke.LambdaFormEditor.TransformKey` > allows keys to be compacted when all byte values of the key fit in 4 bits, > otherwise a byte array is allocated and used. This means that all transforms > with a kind value above 15 will be forced to allocate and use array > comparisons. > > Removing unused and folding some transforms to ensure all existing kinds can > fit snugly within the 0-15 value range realize a minor improvement to > footprint, speed and allocation pressure of affected transforms, e.g. > ~300bytes/op reduction in the `StringConcatFactoryBootstraps` microbenchmark: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt 15 > 2048.475 ? 69.887 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15 > 3487.311 ? 80.385B/op > > > Patched: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt 15 > 1961.985 ? 101.519 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15 > 3156.478 ? 183.600B/op This pull request has now been integrated. Changeset: be933185 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/be93318576896e8f5f9733ae1f7e3e74d63f5594 Stats: 124 lines in 2 files changed: 70 ins; 21 del; 33 mod 8287292: Improve TransformKey to pack more kinds of transforms efficiently Reviewed-by: jlaskey, jvernee, mchung - PR: https://git.openjdk.java.net/jdk/pull/8881
Re: RFR: 8287292: Improve TransformKey to pack more kinds of transforms efficiently [v3]
On Wed, 25 May 2022 14:13:52 GMT, Claes Redestad wrote: >> The bespoke caching scheme in `jl.invoke.LambdaFormEditor.TransformKey` >> allows keys to be compacted when all byte values of the key fit in 4 bits, >> otherwise a byte array is allocated and used. This means that all transforms >> with a kind value above 15 will be forced to allocate and use array >> comparisons. >> >> Removing unused and folding some transforms to ensure all existing kinds can >> fit snugly within the 0-15 value range realize a minor improvement to >> footprint, speed and allocation pressure of affected transforms, e.g. >> ~300bytes/op reduction in the `StringConcatFactoryBootstraps` microbenchmark: >> >> Baseline: >> >> Benchmark Mode Cnt >> Score Error Units >> SCFB.makeConcatWithConstants avgt 15 >> 2048.475 ? 69.887 ns/op >> SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15 >> 3487.311 ? 80.385B/op >> >> >> Patched: >> >> Benchmark Mode Cnt >> Score Error Units >> SCFB.makeConcatWithConstants avgt 15 >> 1961.985 ? 101.519 ns/op >> SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15 >> 3156.478 ? 183.600B/op > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Missed correctly taking b1 into account in of(byte, int, int...) > (java/lang/String/concat/ImplicitStringConcatShapes.java test failure) Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8881
Re: RFR: 8287292: Improve TransformKey to pack more kinds of transforms efficiently [v4]
> The bespoke caching scheme in `jl.invoke.LambdaFormEditor.TransformKey` > allows keys to be compacted when all byte values of the key fit in 4 bits, > otherwise a byte array is allocated and used. This means that all transforms > with a kind value above 15 will be forced to allocate and use array > comparisons. > > Removing unused and folding some transforms to ensure all existing kinds can > fit snugly within the 0-15 value range realize a minor improvement to > footprint, speed and allocation pressure of affected transforms, e.g. > ~300bytes/op reduction in the `StringConcatFactoryBootstraps` microbenchmark: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt 15 > 2048.475 ? 69.887 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15 > 3487.311 ? 80.385B/op > > > Patched: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt 15 > 1961.985 ? 101.519 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt 15 > 3156.478 ? 183.600B/op Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Address Mandy review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8881/files - new: https://git.openjdk.java.net/jdk/pull/8881/files/612b4ece..bcd79aad Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8881=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8881=02-03 Stats: 10 lines in 1 file changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/8881.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8881/head:pull/8881 PR: https://git.openjdk.java.net/jdk/pull/8881
Re: RFR: 8287430 MemorySessionImpl::addOrCleanupIfFail does not rethrow exceptions
On Fri, 27 May 2022 10:29:14 GMT, Maurizio Cimadamore wrote: > This patch fix a missing rethrow in `MemorySessionImpl::addOrCleanupIfFail`. > As noted in the JBS issue, this bug does not affect correctness, but it > delays error reporting. > > Writing a test for this is nearly impossible, given that (a) a memory > resource created against a closed session would be inaccessible by clients > (because the session is closed!), and (b) because of the narrow window in > which the problem might manifest (for this problem to occur, a session state > change would have to occur between the first state check and when the cleanup > action list is updated). src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java line 112: > 110: // to the list (and, in case of an add vs. close race, it might > happen that the cleanup action will be > 111: // called immediately after). > 112: resourceList.add(resource); Note that I've removed the try/catch, as resourceList::add cannot throw the ScopedAccessError singleton (that is only issued in the raw/internal `checkValidState`) - PR: https://git.openjdk.java.net/jdk/pull/8917
RFR: 8287430 MemorySessionImpl::addOrCleanupIfFail does not rethrow exceptions
This patch fix a missing rethrow in `MemorySessionImpl::addOrCleanupIfFail`. As noted in the JBS issue, this bug does not affect correctness, but it delays error reporting. Writing a test for this is nearly impossible, given that (a) a memory resource created against a closed session would be inaccessible by clients (because the session is closed!), and (b) because of the narrow window in which the problem might manifest (for this problem to occur, a session state change would have to occur between the first state check and when the cleanup action list is updated). - Commit messages: - Add more code comments in `MemorySession::addInternal` - Initial push Changes: https://git.openjdk.java.net/jdk/pull/8917/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8917=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287430 Stats: 9 lines in 1 file changed: 3 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8917.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8917/head:pull/8917 PR: https://git.openjdk.java.net/jdk/pull/8917
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v2]
On Thu, 26 May 2022 23:05:32 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v14]
On Fri, 27 May 2022 05:31:28 GMT, Stuart Marks wrote: >> XenoAmess has updated the pull request with a new target base due to a merge >> or a rebase. The pull request now contains 16 commits: >> >> - Merge branch 'master' of https://git.openjdk.java.net/jdk into fix_8284780 >> - Merge remote-tracking branch 'openjdk/master' into fix_8284780 >> >># Conflicts: >># test/jdk/java/util/HashMap/WhiteBoxResizeTest.java >> - add 8284780 to test >> - redo the tests >> - rename items to elements >> - add test for newHashSet and newLinkedHashSet >> - revert much too changes for newHashSet >> - add more replaces >> - add more replaces >> - add more replaces >> - ... and 6 more: >> https://git.openjdk.java.net/jdk/compare/7cb368b3...117918f4 > > Thanks for the updates. I've made a couple minor changes to the specs; please > merge the latest commit from this branch: > > https://github.com/stuart-marks/jdk/tree/pull/8302 > > I've created a CSR and have included these changes in it. Please review: > [JDK-8287419](https://bugs.openjdk.java.net/browse/JDK-8287419) > > I'll look at the test changes later. I wanted to get the CSR moving first, > since it will take a few days (and a long weekend in the US is coming up). @stuart-marks @liach done. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v16]
> as title. XenoAmess has updated the pull request incrementally with two additional commits since the last revision: - Merge remote-tracking branch 'stuart-marks/pull/8302' into fix_8284780 # Conflicts: #src/java.base/share/classes/java/util/HashSet.java #src/java.base/share/classes/java/util/LinkedHashSet.java - Minor terminology fixes: change "item" and "key" to element; remove "insertion-ordered" from LinkedHashSet static factory method because LHS is implicit insertion-ordered. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/42fba932..230a767e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=15 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=14-15 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 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: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v15]
> as title. XenoAmess has updated the pull request incrementally with six additional commits since the last revision: - Update src/java.base/share/classes/java/util/LinkedHashSet.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update src/java.base/share/classes/java/util/LinkedHashSet.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update src/java.base/share/classes/java/util/LinkedHashSet.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update src/java.base/share/classes/java/util/HashSet.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update src/java.base/share/classes/java/util/HashSet.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update src/java.base/share/classes/java/util/HashSet.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/117918f4..42fba932 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=14 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=13-14 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 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
