Re: RFR: 8334755: Asymptotically faster implementation of square root algorithm [v14]
> I have implemented the Zimmermann's square root algorithm, available in works > [here](https://inria.hal.science/inria-00072854/en/) and > [here](https://www.researchgate.net/publication/220532560_A_proof_of_GMP_square_root). > > The algorithm is proved to be asymptotically faster than the Newton's Method, > even for small numbers. To get an idea of how much the Newton's Method is > slow, consult my article [here](https://arxiv.org/abs/2406.07751), in which > I compare Newton's Method with a version of classical square root algorithm > that I implemented. After implementing Zimmermann's algorithm, it turns out > that it is faster than my algorithm even for small numbers. fabioromano1 has updated the pull request incrementally with one additional commit since the last revision: Removed unnecessary variable - Changes: - all: https://git.openjdk.org/jdk/pull/19710/files - new: https://git.openjdk.org/jdk/pull/19710/files/0368a19b..203d3f67 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19710=13 - incr: https://webrevs.openjdk.org/?repo=jdk=19710=12-13 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19710.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19710/head:pull/19710 PR: https://git.openjdk.org/jdk/pull/19710
Re: RFR: 8334755: Asymptotically faster implementation of square root algorithm [v13]
> I have implemented the Zimmermann's square root algorithm, available in works > [here](https://inria.hal.science/inria-00072854/en/) and > [here](https://www.researchgate.net/publication/220532560_A_proof_of_GMP_square_root). > > The algorithm is proved to be asymptotically faster than the Newton's Method, > even for small numbers. To get an idea of how much the Newton's Method is > slow, consult my article [here](https://arxiv.org/abs/2406.07751), in which > I compare Newton's Method with a version of classical square root algorithm > that I implemented. After implementing Zimmermann's algorithm, it turns out > that it is faster than my algorithm even for small numbers. fabioromano1 has updated the pull request incrementally with one additional commit since the last revision: Optimized to compute the remainder only if needed - Changes: - all: https://git.openjdk.org/jdk/pull/19710/files - new: https://git.openjdk.org/jdk/pull/19710/files/923b3475..0368a19b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19710=12 - incr: https://webrevs.openjdk.org/?repo=jdk=19710=11-12 Stats: 47 lines in 2 files changed: 22 ins; 0 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/19710.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19710/head:pull/19710 PR: https://git.openjdk.org/jdk/pull/19710
Re: RFR: 8335110: Fix instruction name and API spec inconsistencies in CodeBuilder [v2]
> This is a collection of fixes and improvements to CodeBuilder, plus 2 renames. > > Fixes include: > 1. `CodeBuilder::receiverSlot` typo > 2. `CodeAttribute::labelToBci` update spec > 3. `CodeBuilder::exceptionCatch` implementation > 4. `CodeBuilder::if_nonnull`/`if_null` -> `ifnonnull`/`ifnull` > 5. Docs for what instructions factories emit, and to explain why some > factories have name mismatch; also a section in summary. Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Patch in csr but forgot to upload - Changes: - all: https://git.openjdk.org/jdk/pull/19889/files - new: https://git.openjdk.org/jdk/pull/19889/files/013a5cf2..8fe07e1c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19889=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19889=00-01 Stats: 15 lines in 1 file changed: 0 ins; 1 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/19889.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19889/head:pull/19889 PR: https://git.openjdk.org/jdk/pull/19889
Withdrawn: 8310994: Add JFR event for selection operations
On Fri, 17 Nov 2023 16:22:55 GMT, Tim Prinzing wrote: > Added mirror event with static methods: jdk.internal.event.SelectionEvent > that provides the duration of select calls and the count of how many keys are > available. > > Emit the event from SelectorImpl::lockAndDoSelect > > Test at jdk.jfr.event.io.TestSelectionEvents This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/16710
Re: RFR: 8333308: javap --system handling doesn't work on internal class names
On Tue, 25 Jun 2024 13:59:35 GMT, Sonia Zaldana Calles wrote: > Hi all, > > This PR addresses [JDK-808](https://bugs.openjdk.org/browse/JDK-808) > enabling `javap -system` to handle internal class names. > > Thanks, > Sonia Technically javap accepts both notations of `a.b.C` and `a/b/C.class` and accepts both `.` and `$` as inner class separators. So this is fine. However it's hard to verify that the jdk in `--system` is really used, so I put a noreg-hard label on the original JBS issue; it's hard to create a suitable argument for the `--system` flag as you need a whole JDK. - PR Comment: https://git.openjdk.org/jdk/pull/19883#issuecomment-2189848518
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Thu, 20 Jun 2024 18:58:27 GMT, Naoto Sato wrote: >> lingjun-cg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 896: Performance regression of DecimalFormat.format > > I did not mean to introduce a public API for this change (if we do, the fix > cannot be backported). I thought we could define a package private one, but > based on your observation, it may get messier... So I agree that falling back > to `StringBuf` is the way to go, IMO. > So, considering all the information given, is it enough to start our new > review process? @naotoj @liach @justin-curtis-lu Well, I was suggesting the same buffer proxying for other Format classes than NumberFormat subclasses, such as DateFormat so that they would have the same performance benefit. Would you be willing to do that too? - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2189821715
Integrated: 8334653: ISO 4217 Amendment 177 Update
On Thu, 20 Jun 2024 18:01:39 GMT, Justin Lu wrote: > Please review this PR which incorporates the ISO 4217 Amendment 177 Update. > > Specifically, the introduction of the new currency, Zimbabwe Gold. This pull request has now been integrated. Changeset: 86b0cf25 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/86b0cf259fb3cbe3a1973151148e5d36c6a99d91 Stats: 23 lines in 6 files changed: 3 ins; 0 del; 20 mod 8334653: ISO 4217 Amendment 177 Update Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/19813
Integrated: 8334418: Update IANA Language Subtag Registry to Version 2024-06-14
On Mon, 17 Jun 2024 21:05:13 GMT, Justin Lu wrote: > Please review this PR, which is a trivial update of the IANA subtag registry > to version 2024-06-14. > Announcement: > https://mm.icann.org/pipermail/ietf-languages-announcements/2024-June/92.html This pull request has now been integrated. Changeset: 861aefca Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/861aefcafacdc21459ef966307f52568e327fd49 Stats: 7 lines in 2 files changed: 4 ins; 0 del; 3 mod 8334418: Update IANA Language Subtag Registry to Version 2024-06-14 Reviewed-by: lancea, iris, srl, naoto - PR: https://git.openjdk.org/jdk/pull/19757
Re: RFR: 8334418: Update IANA Language Subtag Registry to Version 2024-06-14
On Mon, 17 Jun 2024 21:05:13 GMT, Justin Lu wrote: > Please review this PR, which is a trivial update of the IANA subtag registry > to version 2024-06-14. > Announcement: > https://mm.icann.org/pipermail/ietf-languages-announcements/2024-June/92.html Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/19757#issuecomment-2189763247
Integrated: 8334810: Redo: Un-ProblemList LocaleProvidersRun and CalendarDataRegression
On Mon, 24 Jun 2024 04:25:45 GMT, Yude Lin wrote: > [JDK-8318107](https://bugs.openjdk.org/browse/JDK-8318107) Un-ProblemListed > LocaleProvidersRun and CalendarDataRegression, and > [JDK-8288899](https://bugs.openjdk.org/browse/JDK-8288899) added them back. > I'm guessing it's a mistake in resolving merge conflict. This pull request has now been integrated. Changeset: f8bf470b Author:Yude Lin Committer: Naoto Sato URL: https://git.openjdk.org/jdk/commit/f8bf470b773884911290fa6ce059f7cc13686186 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod 8334810: Redo: Un-ProblemList LocaleProvidersRun and CalendarDataRegression 8268379: java/util/Locale/LocaleProvidersRun.java and sun/util/locale/provider/CalendarDataRegression.java timed out Reviewed-by: naoto, jlu - PR: https://git.openjdk.org/jdk/pull/19849
Re: RFR: 8334441: Mark tests in jdk_security_infra group as manual [v2]
On Fri, 21 Jun 2024 16:11:34 GMT, Rajan Halade wrote: >> Updated all the tests that depend on external infrastructure services as >> manual. These tests may fail with external reasons, for instance - change in >> CA test portal, certificate status updates, or network issues. > > Rajan Halade has updated the pull request incrementally with one additional > commit since the last revision: > > fix typos Is there still going to be some monitoring of these tests? Making it harder to run these tests potentially means that genuine failures can go unnoticed and JDK certificates are not updated when needed. - PR Comment: https://git.openjdk.org/jdk/pull/19814#issuecomment-2189601735
RFR: 8335110: Fix instruction name and API spec inconsistencies in CodeBuilder
This is a collection of fixes and improvements to CodeBuilder, plus 2 renames. Fixes include: 1. `CodeBuilder::receiverSlot` typo 2. `CodeAttribute::labelToBci` update spec 3. `CodeBuilder::exceptionCatch` implementation 4. `CodeBuilder::if_nonnull`/`if_null` -> `ifnonnull`/`ifnull` 5. Docs for what instructions factories emit, and to explain why some factories have name mismatch; also a section in summary. - Commit messages: - Describe extra types of instructions that can be generated - Minor polishing to CodeBuilder Changes: https://git.openjdk.org/jdk/pull/19889/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19889=00 Issue: https://bugs.openjdk.org/browse/JDK-8335110 Stats: 123 lines in 6 files changed: 106 ins; 1 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/19889.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19889/head:pull/19889 PR: https://git.openjdk.org/jdk/pull/19889
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v18]
> The current versions of FloatToDecimal and DoubleToDecimal allocate
> additional objects. Reducing these allocations can improve the performance of
> Float/Double.toString and AbstractStringBuilder's append(float/double).
>
> This patch is just a code refactoring to reduce object allocation, but does
> not change the Float/Double to decimal algorithm.
>
> The following code comments the allocated objects to be removed.
>
>
> class FloatToDecimal {
> public static String toString(float v) {
> // allocate object FloatToDecimal
> return new FloatToDecimal().toDecimalString(v);
> }
>
> public static Appendable appendTo(float v, Appendable app)
> throws IOException {
> // allocate object FloatToDecimal
> return new FloatToDecimal().appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(float v, Appendable app)
> throws IOException {
> switch (toDecimal(v)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> if (app instanceof StringBuffer buffer) {
> return buffer.append(chars);
> }
> for (char c : chars) {
> app.append(c);
> }
> return app;
> // ...
> }
> }
> }
>
> class DoubleToDecimal {
> public static String toString(double v) {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).toDecimalString(v);
> }
>
> public static Appendable appendTo(double v, Appendable app)
> throws IOException {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(double v, Appendable app)
> throws IOException {
> switch (toDecimal(v, null)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> ...
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
from @liach: use s.getBytes for performance
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19730/files
- new: https://git.openjdk.org/jdk/pull/19730/files/a18c1a1b..5d3405f9
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19730=17
- incr: https://webrevs.openjdk.org/?repo=jdk=19730=16-17
Stats: 4 lines in 1 file changed: 1 ins; 2 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19730.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19730/head:pull/19730
PR: https://git.openjdk.org/jdk/pull/19730
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v4]
On Thu, 20 Jun 2024 14:32:25 GMT, Shaojin Wen wrote:
>> @cl4es @wenshao I think we should probably work on `putChar`, or at least
>> figure out what blocks `MergeStore` for `putChar`. Can someone produce a
>> simple stand-alone `.java` file that uses `putChar`, so that I can
>> investigate why `MergeStore` does not work for it?
>>
>> Otherwise, I agree with @cl4es : the code here may be ok for now, but we
>> would have to revert it again later, should `MergeStore` eventually do the
>> trick.
>
> @eme64
>
> simple stand-alone java
>
> import jdk.internal.misc.Unsafe;
>
> public class PutCharTest {
> static final Unsafe UNSAFE = Unsafe.getUnsafe();
>
> static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int
> c4) {
> putChar(val, index, (char)(c1));
> putChar(val, index + 1, (char)(c2));
> putChar(val, index + 2, (char)(c3));
> putChar(val, index + 3, (char)(c4));
> }
>
> static void putChar(byte[] bytes, int index, int c) {
> UNSAFE.putChar(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + (index << 1),
> (char)(c));
> }
>
> static void putChar0(byte[] bytes, int index, int c) {
> UNSAFE.putByte(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + (index << 1),
> (byte)(c));
> UNSAFE.putByte(bytes, Unsafe.ARRAY_BYTE_BASE_OFFSET + (index << 1) +
> 1, (byte)(c << 8));
> }
>
> static void putNull(byte[] bytes, int index) {
> putCharsAt(bytes, index, 'n', 'u', 'l', 'l');
> }
>
> public static void main(String[] args) {
> for (int i = 0; i < 5; i++) {
> testNull();
> }
>
> System.out.println("done");
> }
>
> private static void testNull() {
> byte[] bytes = new byte[8192];
>
> for (int i = 0; i < 1000; i++) {
> int index = 0;
> for (int j = 0; j < 1024; j++) {
> putNull(bytes, index);
> index += 4;
> }
> }
> }
> }
@wenshao @cl4es I think this use-case is quite a valid one, and deserves an
extension. I filed:
[JDK-8335113](https://bugs.openjdk.org/browse/JDK-8335113): C2 MergeStores:
allow merging of putChar and other larger stores on smaller array elements
-
PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2189523984
Re: RFR: 8333893: Optimization for StringBuilder append boolean & null [v13]
On Sun, 23 Jun 2024 08:47:25 GMT, Shaojin Wen wrote:
>> After PR https://github.com/openjdk/jdk/pull/16245, C2 optimizes stores into
>> primitive arrays by combining values into larger stores.
>>
>> This PR rewrites the code of appendNull and append(boolean) methods so that
>> these two methods can be optimized by C2.
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> private static final field `UNSAFE`
I'm running your benchmark
[PutCharTest.java](https://github.com/user-attachments/files/15974672/PutCharTest.txt)
with:
/oracle-work/jdk-fork2/build/linux-x64-slowdebug/jdk/bin/java --add-modules
java.base --add-exports java.base/jdk.internal.misc=ALL-UNNAMED --add-exports
java.base/jdk.internal.util=ALL-UNNAMED -XX:+TraceMergeStores -Xbatch
-XX:CompileCommand=printcompilation,PutCharTest::*
-XX:CompileCommand=compileonly,PutCharTest::put* -XX:+PrintIdeal
PutCharTest.java
Aha, I found the limitation in `MergeStores`:
https://github.com/openjdk/jdk/blob/f7862bd6b9994814c6dfd43d471122408601f288/src/hotspot/share/opto/memnode.cpp#L2982-L2986
Basically, I require the `store` to have the same data-size as the element-size
of the array.
But the `putChar` ends up as a 2-byte `StoreC`, and the array is a `byte[]`
with 1-byte elements.
I quickly removed this check:
diff --git a/src/hotspot/share/opto/memnode.cpp
b/src/hotspot/share/opto/memnode.cpp
index d0b6c59637f..78efda2db13 100644
--- a/src/hotspot/share/opto/memnode.cpp
+++ b/src/hotspot/share/opto/memnode.cpp
@@ -2980,10 +2980,10 @@ StoreNode* MergePrimitiveArrayStores::run() {
return nullptr;
}
BasicType bt = aryptr_t->elem()->array_element_basic_type();
- if (!is_java_primitive(bt) ||
- type2aelembytes(bt) != _store->memory_size()) {
-return nullptr;
- }
+ //if (!is_java_primitive(bt) ||
+ //type2aelembytes(bt) != _store->memory_size()) {
+ // return nullptr;
+ //}
// The _store must be the "last" store in a chain. If we find a use we could
merge with
// then that use or a store further down is the "last" store.
@@ -3033,13 +3033,13 @@ bool
MergePrimitiveArrayStores::is_compatible_store(const StoreNode* other_store
if (!is_java_primitive(aryptr_bt1) || !is_java_primitive(aryptr_bt2)) {
return false;
}
- int size1 = type2aelembytes(aryptr_bt1);
- int size2 = type2aelembytes(aryptr_bt2);
- if (size1 != size2 ||
- size1 != _store->memory_size() ||
- _store->memory_size() != other_store->memory_size()) {
-return false;
- }
+ //int size1 = type2aelembytes(aryptr_bt1);
+ //int size2 = type2aelembytes(aryptr_bt2);
+ //if (size1 != size2 ||
+ //size1 != _store->memory_size() ||
+ //_store->memory_size() != other_store->memory_size()) {
+ // return false;
+ //}
return true;
}
And now it optimizes:
15523 103b 4 PutCharTest::putNull (14 bytes)
[TraceMergeStores]: Replace
74 StoreC === 62 7 73 22 [[ 99 ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):exact+any *, idx=4; mismatched
unsafe Memory: @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=4; !jvms:
PutCharTest::putChar @ bci:14 (line 14) PutCharTest::putCharsAt @ bci:4 (line
7) PutCharTest::putNull @ bci:10 (line 23)
99 StoreC === 62 74 98 23 [[ 124 ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):exact+any *, idx=4; mismatched
unsafe Memory: @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=4; !jvms:
PutCharTest::putChar @ bci:14 (line 14) PutCharTest::putCharsAt @ bci:13 (line
8) PutCharTest::putNull @ bci:10 (line 23)
124 StoreC === 62 99 123 24 [[ 150 ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):exact+any *, idx=4; mismatched
unsafe Memory: @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=4; !jvms:
PutCharTest::putChar @ bci:14 (line 14) PutCharTest::putCharsAt @ bci:23 (line
9) PutCharTest::putNull @ bci:10 (line 23)
150 StoreC === 62 124 149 24 [[ 17 ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):exact+any *, idx=4; mismatched
unsafe Memory: @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=4; !jvms:
PutCharTest::putChar @ bci:14 (line 14) PutCharTest::putCharsAt @ bci:33 (line
10) PutCharTest::putNull @ bci:10 (line 23)
[TraceMergeStores]: with
155 ConL === 0 [[ 156 ]] #long:30399761348886638
156 StoreL === 62 7 73 155 [[ ]] @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=4;
mismatched Memory: @byte[int:>=0]
(java/lang/Cloneable,java/io/Serializable):NotNull:exact+any *, idx=4;
I will file an RFE to enable this use-case as well. @wenshao thanks for the
standalone test, that was really helpful!
-
PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2189509983
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v17]
> The current versions of FloatToDecimal and DoubleToDecimal allocate
> additional objects. Reducing these allocations can improve the performance of
> Float/Double.toString and AbstractStringBuilder's append(float/double).
>
> This patch is just a code refactoring to reduce object allocation, but does
> not change the Float/Double to decimal algorithm.
>
> The following code comments the allocated objects to be removed.
>
>
> class FloatToDecimal {
> public static String toString(float v) {
> // allocate object FloatToDecimal
> return new FloatToDecimal().toDecimalString(v);
> }
>
> public static Appendable appendTo(float v, Appendable app)
> throws IOException {
> // allocate object FloatToDecimal
> return new FloatToDecimal().appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(float v, Appendable app)
> throws IOException {
> switch (toDecimal(v)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> if (app instanceof StringBuffer buffer) {
> return buffer.append(chars);
> }
> for (char c : chars) {
> app.append(c);
> }
> return app;
> // ...
> }
> }
> }
>
> class DoubleToDecimal {
> public static String toString(double v) {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).toDecimalString(v);
> }
>
> public static Appendable appendTo(double v, Appendable app)
> throws IOException {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(double v, Appendable app)
> throws IOException {
> switch (toDecimal(v, null)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> ...
Shaojin Wen has updated the pull request incrementally with two additional
commits since the last revision:
- fix assert from @rgiulietti
- fix comment from @rgiulietti
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19730/files
- new: https://git.openjdk.org/jdk/pull/19730/files/47ae33d0..a18c1a1b
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19730=16
- incr: https://webrevs.openjdk.org/?repo=jdk=19730=15-16
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/19730.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19730/head:pull/19730
PR: https://git.openjdk.org/jdk/pull/19730
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v13]
On Tue, 25 Jun 2024 14:47:45 GMT, Raffaello Giulietti wrote: >> Shaojin Wen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Utf16 case remove `append first utf16 char` > > src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 236: > >> 234: dk = -1; >> 235: } >> 236: return toDecimal(str, index, Q_MIN, t, dk, fd) - start; > > I suggest restoring the original logic like so: > > /* subnormal value */ > return (t < C_TINY > ? toDecimal(str, index, Q_MIN, 10 * t, -1, fd) > : toDecimal(str, index, Q_MIN, t, 0, fd)) - start; I like the new implementation, the code is cleaner, is your suggestion to revert to the original version due to smaller changes? - PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1653210427
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v16]
> The current versions of FloatToDecimal and DoubleToDecimal allocate
> additional objects. Reducing these allocations can improve the performance of
> Float/Double.toString and AbstractStringBuilder's append(float/double).
>
> This patch is just a code refactoring to reduce object allocation, but does
> not change the Float/Double to decimal algorithm.
>
> The following code comments the allocated objects to be removed.
>
>
> class FloatToDecimal {
> public static String toString(float v) {
> // allocate object FloatToDecimal
> return new FloatToDecimal().toDecimalString(v);
> }
>
> public static Appendable appendTo(float v, Appendable app)
> throws IOException {
> // allocate object FloatToDecimal
> return new FloatToDecimal().appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(float v, Appendable app)
> throws IOException {
> switch (toDecimal(v)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> if (app instanceof StringBuffer buffer) {
> return buffer.append(chars);
> }
> for (char c : chars) {
> app.append(c);
> }
> return app;
> // ...
> }
> }
> }
>
> class DoubleToDecimal {
> public static String toString(double v) {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).toDecimalString(v);
> }
>
> public static Appendable appendTo(double v, Appendable app)
> throws IOException {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(double v, Appendable app)
> throws IOException {
> switch (toDecimal(v, null)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> ...
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
refactor from @rgiulietti
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19730/files
- new: https://git.openjdk.org/jdk/pull/19730/files/884757fa..47ae33d0
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19730=15
- incr: https://webrevs.openjdk.org/?repo=jdk=19730=14-15
Stats: 62 lines in 3 files changed: 4 ins; 13 del; 45 mod
Patch: https://git.openjdk.org/jdk/pull/19730.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19730/head:pull/19730
PR: https://git.openjdk.org/jdk/pull/19730
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v15]
> The current versions of FloatToDecimal and DoubleToDecimal allocate
> additional objects. Reducing these allocations can improve the performance of
> Float/Double.toString and AbstractStringBuilder's append(float/double).
>
> This patch is just a code refactoring to reduce object allocation, but does
> not change the Float/Double to decimal algorithm.
>
> The following code comments the allocated objects to be removed.
>
>
> class FloatToDecimal {
> public static String toString(float v) {
> // allocate object FloatToDecimal
> return new FloatToDecimal().toDecimalString(v);
> }
>
> public static Appendable appendTo(float v, Appendable app)
> throws IOException {
> // allocate object FloatToDecimal
> return new FloatToDecimal().appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(float v, Appendable app)
> throws IOException {
> switch (toDecimal(v)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> if (app instanceof StringBuffer buffer) {
> return buffer.append(chars);
> }
> for (char c : chars) {
> app.append(c);
> }
> return app;
> // ...
> }
> }
> }
>
> class DoubleToDecimal {
> public static String toString(double v) {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).toDecimalString(v);
> }
>
> public static Appendable appendTo(double v, Appendable app)
> throws IOException {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(double v, Appendable app)
> throws IOException {
> switch (toDecimal(v, null)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> ...
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
Update src/java.base/share/classes/jdk/internal/math/ToDecimal.java
Co-authored-by: Raffaello Giulietti
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19730/files
- new: https://git.openjdk.org/jdk/pull/19730/files/e3830386..884757fa
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19730=14
- incr: https://webrevs.openjdk.org/?repo=jdk=19730=13-14
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19730.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19730/head:pull/19730
PR: https://git.openjdk.org/jdk/pull/19730
RFR: 8333308: javap --system handling doesn't work on internal class names
Hi all, This PR addresses [JDK-808](https://bugs.openjdk.org/browse/JDK-808) enabling `javap -system` to handle internal class names. Thanks, Sonia - Commit messages: - 808: javap --system handling doesn't work on internal class names Changes: https://git.openjdk.org/jdk/pull/19883/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19883=00 Issue: https://bugs.openjdk.org/browse/JDK-808 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19883.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19883/head:pull/19883 PR: https://git.openjdk.org/jdk/pull/19883
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v14]
> The current versions of FloatToDecimal and DoubleToDecimal allocate
> additional objects. Reducing these allocations can improve the performance of
> Float/Double.toString and AbstractStringBuilder's append(float/double).
>
> This patch is just a code refactoring to reduce object allocation, but does
> not change the Float/Double to decimal algorithm.
>
> The following code comments the allocated objects to be removed.
>
>
> class FloatToDecimal {
> public static String toString(float v) {
> // allocate object FloatToDecimal
> return new FloatToDecimal().toDecimalString(v);
> }
>
> public static Appendable appendTo(float v, Appendable app)
> throws IOException {
> // allocate object FloatToDecimal
> return new FloatToDecimal().appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(float v, Appendable app)
> throws IOException {
> switch (toDecimal(v)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> if (app instanceof StringBuffer buffer) {
> return buffer.append(chars);
> }
> for (char c : chars) {
> app.append(c);
> }
> return app;
> // ...
> }
> }
> }
>
> class DoubleToDecimal {
> public static String toString(double v) {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).toDecimalString(v);
> }
>
> public static Appendable appendTo(double v, Appendable app)
> throws IOException {
> // allocate object DoubleToDecimal
> return new DoubleToDecimal(false).appendDecimalTo(v, app);
> }
>
> private Appendable appendDecimalTo(double v, Appendable app)
> throws IOException {
> switch (toDecimal(v, null)) {
> case NON_SPECIAL:
> // allocate object char[]
> char[] chars = new char[index + 1];
> for (int i = 0; i < chars.length; ++i) {
> chars[i] = (char) bytes[i];
> }
> if (app instanceof StringBuilder builder) {
> return builder.append(chars);
> }
> ...
Shaojin Wen has updated the pull request incrementally with four additional
commits since the last revision:
- Update src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java
Co-authored-by: Raffaello Giulietti
- Update src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java
Co-authored-by: Raffaello Giulietti
- Update src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java
Co-authored-by: Raffaello Giulietti
- Update src/java.base/share/classes/jdk/internal/math/ToDecimal.java
Co-authored-by: Raffaello Giulietti
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19730/files
- new: https://git.openjdk.org/jdk/pull/19730/files/b8e80428..e3830386
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19730=13
- incr: https://webrevs.openjdk.org/?repo=jdk=19730=12-13
Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/19730.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19730/head:pull/19730
PR: https://git.openjdk.org/jdk/pull/19730
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v13]
On Mon, 17 Jun 2024 04:58:41 GMT, Shaojin Wen wrote:
>> The current versions of FloatToDecimal and DoubleToDecimal allocate
>> additional objects. Reducing these allocations can improve the performance
>> of Float/Double.toString and AbstractStringBuilder's append(float/double).
>>
>> This patch is just a code refactoring to reduce object allocation, but does
>> not change the Float/Double to decimal algorithm.
>>
>> The following code comments the allocated objects to be removed.
>>
>>
>> class FloatToDecimal {
>> public static String toString(float v) {
>> // allocate object FloatToDecimal
>> return new FloatToDecimal().toDecimalString(v);
>> }
>>
>> public static Appendable appendTo(float v, Appendable app)
>> throws IOException {
>> // allocate object FloatToDecimal
>> return new FloatToDecimal().appendDecimalTo(v, app);
>> }
>>
>> private Appendable appendDecimalTo(float v, Appendable app)
>> throws IOException {
>> switch (toDecimal(v)) {
>> case NON_SPECIAL:
>> // allocate object char[]
>> char[] chars = new char[index + 1];
>> for (int i = 0; i < chars.length; ++i) {
>> chars[i] = (char) bytes[i];
>> }
>> if (app instanceof StringBuilder builder) {
>> return builder.append(chars);
>> }
>> if (app instanceof StringBuffer buffer) {
>> return buffer.append(chars);
>> }
>> for (char c : chars) {
>> app.append(c);
>> }
>> return app;
>> // ...
>> }
>> }
>> }
>>
>> class DoubleToDecimal {
>> public static String toString(double v) {
>> // allocate object DoubleToDecimal
>> return new DoubleToDecimal(false).toDecimalString(v);
>> }
>>
>> public static Appendable appendTo(double v, Appendable app)
>> throws IOException {
>> // allocate object DoubleToDecimal
>> return new DoubleToDecimal(false).appendDecimalTo(v, app);
>> }
>>
>> private Appendable appendDecimalTo(double v, Appendable app)
>> throws IOException {
>> switch (toDecimal(v, null)) {
>> case NON_SPECIAL:
>> // allocate object char[]
>> char[] chars = new char[index + 1];
>> for (int i = 0; i < chars.length; ++i) {
>> chars[i] = (char) bytes[i];
>> }
>> ...
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Utf16 case remove `append first utf16 char`
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 43:
> 41: public final class DoubleToDecimal extends ToDecimal {
> 42: /**
> 43: * Use LATIN1 encoding to process the input byte[] str
Suggestion:
* Use LATIN1 encoding to process the in-out byte[] str
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 49:
> 47:
> 48: /**
> 49: * Use UTF16 encoding to process the input byte[] str
Suggestion:
* Use UTF16 encoding to process the in-out byte[] str
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 175:
> 173: */
> 174: public int putDecimal(byte[] str, int index, double v) {
> 175: assert index >= 0 && index + MAX_CHARS <= length(str) : "Trusted
> caller missed bounds check";
Suggestion:
assert 0 <= index && index <= length(str) - MAX_CHARS : "Trusted caller
missed bounds check";
This avoids a potential overflow.
But if you want to enforce the check, you should probably avoid using `assert`
since assertions might be disabled.
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 236:
> 234: dk = -1;
> 235: }
> 236: return toDecimal(str, index, Q_MIN, t, dk, fd) - start;
I suggest restoring the original logic like so:
/* subnormal value */
return (t < C_TINY
? toDecimal(str, index, Q_MIN, 10 * t, -1, fd)
: toDecimal(str, index, Q_MIN, t, 0, fd)) - start;
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 452:
> 450: if (l != 0) {
> 451: put8Digits(str, index, l);
> 452: index += 8;
As discussed in `ToDecimal`, all `put...` methods should return the updated
`index`. Then this code can be rewritten as
Suggestion:
index = put8Digits(str, index, l);
so that the caller doesn't need to know how many characters were added.
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 460:
> 458: putChar(str, index++, 'E');
> 459: if (e < 0) {
> 460: putChar(str, index++, '-');
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v3]
On Tue, 25 Jun 2024 13:47:39 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more de-lambda work on ClassSpecializer Thanks! This looks better. `jtreg:test/jdk/java/lang/invoke/condy` have passed on linux ppc64 Big Endian. I'll put it in our nightly tests to run it on AIX, too. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2189112288
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v33]
On Mon, 24 Jun 2024 14:33:51 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 137 commits: > > - JLinkDedupTestBatchSizeOne.java test fix > >Run only the packaged modules version if we have a linkable JDK runtime >with packaged modules available as well in the JDK install. > - Enable IncludeLocalesPluginTest > - Enable GenerateJLIClassesPluginTest.java test > - Enable JLinkReproducibleTest.java for linkable JDK images > - Remove restriction on directory based modules > >Enable the following tests: >- JLink100Modules.java >- JLinkDedupTestBatchSizeOne.java > - Comment fix in JRTArchive. Long line in JlinkTask > - Comment fixes in ImageFileCreator > - Comments and clean-up in JlinkTask > - Helper support for linkable JDK runtimes > - Test clean-up. class-file API module. > - ... and 127 more: https://git.openjdk.org/jdk/compare/5cad0b4d...04cd98f8 I've pushed some clean-up fixes to this PR so as to fix the overly long lines and added comments to relevant methods. The latest GHA failure on MacOSX x86_64 is infra related. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2189072464
Integrated: 8334397: RISC-V: verify perf of ReverseBytesS/US
On Fri, 21 Jun 2024 14:24:26 GMT, Hamlin Li wrote: > Hi, > Can you help to review this patch? > Thanks! > > This is similar with previous JDK-8334396. > Added some tests. > > ### Test > > | Tests | Scores | Errors | Unit > -- | -- | -- | -- | -- > Intrinsic, +zbb, +rvv | Characters.reverseBytes | 1654.535 | 69.36 | ns/op > | Shorts.reverseBytes | 1795.403 | 44.015 | ns/op > Intrinsic, +zbb, -rvv | Characters.reverseBytes | 1649.752 | 74.965 | ns/op > | Shorts.reverseBytes | 1798.637 | 49.52 | ns/op > Intrinsic, -zbb, +rvv | Characters.reverseBytes | 2279.588 | 44.222 | ns/op > | Shorts.reverseBytes | 2441.674 | 63.895 | ns/op > Intrinsic, -zbb, -rvv | Characters.reverseBytes | 2288.876 | 49.099 | ns/op > | Shorts.reverseBytes | 2454.454 | 94.004 | ns/op > No intrinsic | Characters.reverseBytes | 1629.722 | 23.656 | ns/op > | Shorts.reverseBytes | 2108.81 | 43.378 | ns/op > > This pull request has now been integrated. Changeset: cae94b26 Author:Hamlin Li URL: https://git.openjdk.org/jdk/commit/cae94b268d633b0557a54e3b21eff60d7f0edc2d Stats: 129 lines in 4 files changed: 98 ins; 31 del; 0 mod 8334397: RISC-V: verify perf of ReverseBytesS/US Reviewed-by: fyang, luhenry - PR: https://git.openjdk.org/jdk/pull/19830
Re: RFR: 8334397: RISC-V: verify perf of ReverseBytesS/US
On Tue, 25 Jun 2024 12:51:46 GMT, Ludovic Henry wrote: >> Hi, >> Can you help to review this patch? >> Thanks! >> >> This is similar with previous JDK-8334396. >> Added some tests. >> >> ### Test >> >> | Tests | Scores | Errors | Unit >> -- | -- | -- | -- | -- >> Intrinsic, +zbb, +rvv | Characters.reverseBytes | 1654.535 | 69.36 | ns/op >> | Shorts.reverseBytes | 1795.403 | 44.015 | ns/op >> Intrinsic, +zbb, -rvv | Characters.reverseBytes | 1649.752 | 74.965 | ns/op >> | Shorts.reverseBytes | 1798.637 | 49.52 | ns/op >> Intrinsic, -zbb, +rvv | Characters.reverseBytes | 2279.588 | 44.222 | ns/op >> | Shorts.reverseBytes | 2441.674 | 63.895 | ns/op >> Intrinsic, -zbb, -rvv | Characters.reverseBytes | 2288.876 | 49.099 | ns/op >> | Shorts.reverseBytes | 2454.454 | 94.004 | ns/op >> No intrinsic | Characters.reverseBytes | 1629.722 | 23.656 | ns/op >> | Shorts.reverseBytes | 2108.81 | 43.378 | ns/op >> >> > > Marked as reviewed by luhenry (Committer). Thanks @luhenry @RealFYang for your reviewing. - PR Comment: https://git.openjdk.org/jdk/pull/19830#issuecomment-2189053398
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Tue, 25 Jun 2024 13:49:31 GMT, Adam Sotona wrote: >> Transforming the classModel back to the bytes array in order to write the >> transformed classfile > > And what is the purpose of parsing a model from bytes and transforming it > back to bytes without a change? This transformation is called only after the `classModel` is transformed: - at line 472 and 380 when the `transformToBytes()` method is called is after effectively transforming the `classModel` inside the `Logic` class. - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652868906
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v6]
On Thu, 20 Jun 2024 17:37:05 GMT, Thomas Stuefe wrote:
>> Severin Gehwolf has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove problem listing of PlainRead which is reworked here
>
> src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 422:
>
>> 420: * (12) super options: matched with '%s' and captured in
>> 'tmpcgroups'
>> 421: */
>> 422: if (sscanf(p, "%*d %*d %*d:%*d %s %s %s%*[^-]- %s %*s %s", tmproot,
>> tmpmount, mount_opts, tmp_fs_type, tmpcgroups) == 5) {
>
> The only difference to v1 is that we parse the super options (12), right?
> Could we factor out the parsing into a helper function? Or, alternatively, at
> least `#define` the scanf format somewhere up top, including the nice
> comment, and reuse that format string?
That's correct. I've moved it into a local helper function. Thanks!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18201#discussion_r1652863523
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v7]
> Please review this enhancement to the container detection code which allows > it to figure out whether the JVM is actually running inside a container > (`podman`, `docker`, `crio`), or with some other means that enforces > memory/cpu limits by means of the cgroup filesystem. If neither of those > conditions hold, the JVM runs in not containerized mode, addressing the issue > described in the JBS tracker. For example, on my Linux system > `is_containerized() == false" is being indicated with the following trace log > line: > > > [0.001s][debug][os,container] OSContainer::init: is_containerized() = false > because no cpu or memory limit is present > > > This state is being exposed by the Java `Metrics` API class using the new > (still JDK internal) `isContainerized()` method. Example: > > > java -XshowSettings:system --version > Operating System Metrics: > Provider: cgroupv1 > System not containerized. > openjdk 23-internal 2024-09-17 > OpenJDK Runtime Environment (fastdebug build > 23-internal-adhoc.sgehwolf.jdk-jdk) > OpenJDK 64-Bit Server VM (fastdebug build 23-internal-adhoc.sgehwolf.jdk-jdk, > mixed mode, sharing) > > > The basic property this is being built on is the observation that the cgroup > controllers typically get mounted read only into containers. Note that the > current container tests assert that `OSContainer::is_containerized() == true` > in various tests. Therefore, using the heuristic of "is any memory or cpu > limit present" isn't sufficient. I had considered that in an earlier > iteration, but many container tests failed. > > Overall, I think, with this patch we improve the current situation of > claiming a containerized system being present when it's actually just a > regular Linux system. > > Testing: > > - [x] GHA (risc-v failure seems infra related) > - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2 (including > gtests) > - [x] Some manual testing using cri-o > > Thoughts? Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits: - Refactor mount info matching to helper function - Merge branch 'master' into jdk-8261242-is-containerized-fix - Remove problem listing of PlainRead which is reworked here - Merge branch 'master' into jdk-8261242-is-containerized-fix - Merge branch 'master' into jdk-8261242-is-containerized-fix - Add doc for mountinfo scanning. - Unify naming of variables - Merge branch 'master' into jdk-8261242-is-containerized-fix - Merge branch 'master' into jdk-8261242-is-containerized-fix - jcheck fixes - ... and 7 more: https://git.openjdk.org/jdk/compare/baafa662...532ea33b - Changes: https://git.openjdk.org/jdk/pull/18201/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18201=06 Stats: 411 lines in 20 files changed: 305 ins; 79 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/18201.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18201/head:pull/18201 PR: https://git.openjdk.org/jdk/pull/18201
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Tue, 25 Jun 2024 13:45:17 GMT, Oussama Louati wrote:
>> test/jdk/java/lang/invoke/indify/Indify.java line 386:
>>
>>> 384:
>>> 385: byte[] transformToBytes(ClassModel classModel) {
>>> 386: return of().transform(classModel, ClassTransform.ACCEPT_ALL);
>>
>> What is the purpose of this transformation?
>
> Transforming the classModel back to the bytes array in order to write the
> transformed classfile
And what is the purpose of parsing a model from bytes and transforming it back
to bytes without a change?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652861409
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Tue, 25 Jun 2024 13:17:13 GMT, Adam Sotona wrote:
>> Oussama Louati has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> remove: removed unnecessary logging
>
> test/jdk/java/lang/invoke/indify/Indify.java line 386:
>
>> 384:
>> 385: byte[] transformToBytes(ClassModel classModel) {
>> 386: return of().transform(classModel, ClassTransform.ACCEPT_ALL);
>
> What is the purpose of this transformation?
Transforming the classModel back to the bytes array in order to write the
transformed classfile
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652854265
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v2]
On Tue, 25 Jun 2024 09:36:37 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more lambdas conversions to fix bootstrap Thanks for the report and patience. Yes, CodeBuilder::withMethodBody is also internally using lambda. Another version of the patch is ready for test. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2189005614
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v3]
> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: more de-lambda work on ClassSpecializer - Changes: - all: https://git.openjdk.org/jdk/pull/19863/files - new: https://git.openjdk.org/jdk/pull/19863/files/357d36a0..1416d4d2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19863=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19863=01-02 Stats: 53 lines in 1 file changed: 4 ins; 0 del; 49 mod Patch: https://git.openjdk.org/jdk/pull/19863.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19863/head:pull/19863 PR: https://git.openjdk.org/jdk/pull/19863
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v6]
On Tue, 25 Jun 2024 13:39:07 GMT, Jan Kratochvil wrote: > Currently this patch conflicts a lot with #19085 > (jerboaa:jdk-8331560-cgroup-controller-delegation). Yes, I'll resolve it one way or another depending on which one goes in first. - PR Comment: https://git.openjdk.org/jdk/pull/18201#issuecomment-2189001364
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v6]
On Thu, 20 Jun 2024 12:06:43 GMT, Severin Gehwolf wrote: >> Please review this enhancement to the container detection code which allows >> it to figure out whether the JVM is actually running inside a container >> (`podman`, `docker`, `crio`), or with some other means that enforces >> memory/cpu limits by means of the cgroup filesystem. If neither of those >> conditions hold, the JVM runs in not containerized mode, addressing the >> issue described in the JBS tracker. For example, on my Linux system >> `is_containerized() == false" is being indicated with the following trace >> log line: >> >> >> [0.001s][debug][os,container] OSContainer::init: is_containerized() = false >> because no cpu or memory limit is present >> >> >> This state is being exposed by the Java `Metrics` API class using the new >> (still JDK internal) `isContainerized()` method. Example: >> >> >> java -XshowSettings:system --version >> Operating System Metrics: >> Provider: cgroupv1 >> System not containerized. >> openjdk 23-internal 2024-09-17 >> OpenJDK Runtime Environment (fastdebug build >> 23-internal-adhoc.sgehwolf.jdk-jdk) >> OpenJDK 64-Bit Server VM (fastdebug build >> 23-internal-adhoc.sgehwolf.jdk-jdk, mixed mode, sharing) >> >> >> The basic property this is being built on is the observation that the cgroup >> controllers typically get mounted read only into containers. Note that the >> current container tests assert that `OSContainer::is_containerized() == >> true` in various tests. Therefore, using the heuristic of "is any memory or >> cpu limit present" isn't sufficient. I had considered that in an earlier >> iteration, but many container tests failed. >> >> Overall, I think, with this patch we improve the current situation of >> claiming a containerized system being present when it's actually just a >> regular Linux system. >> >> Testing: >> >> - [x] GHA (risc-v failure seems infra related) >> - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2 >> (including gtests) >> - [x] Some manual testing using cri-o >> >> Thoughts? > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Remove problem listing of PlainRead which is reworked here Currently this patch conflicts a lot with #19085 (jerboaa:jdk-8331560-cgroup-controller-delegation). - PR Comment: https://git.openjdk.org/jdk/pull/18201#issuecomment-2188994651
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Sat, 22 Jun 2024 15:55:28 GMT, Oussama Louati wrote:
>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle,
>> MethodType, and CallSite constants.
>> ### Purpose of Indify
>>
>> - **Transformation Tool**: `Indify` transforms existing class files to
>> leverage `invokedynamic` and other JSR 292 features. This transformation
>> allows for dynamic method calls.
>>
>> ### Key Functions
>>
>> - **Method Handle and Method Type Constants**:
>> - **MH_x and MT_x Methods**: These methods are used to generate
>> `MethodHandle` and `MethodType` constants. The tool transforms calls to
>> these methods into `CONSTANT_MethodHandle` and `CONSTANT_MethodType` "ldc"
>> (load constant) instructions.
>> - **Invokedynamic Call Sites**:
>> - **INDY_x Methods**: These methods generate `invokedynamic` call sites.
>> Each `INDY_x` method starts with a call to an `MH_x` method, which acts as
>> the bootstrap method. This is followed by an `invokeExact` call.
>> - **Transformation**: The tool transforms pairs of calls (to `INDY_x`
>> followed by `invokeExact`) into `invokedynamic` instructions. This mimics
>> the JVM's handling of `invokedynamic` instructions, creating dynamic call
>> sites that are linked at runtime.
>>
>> It currently uses ad-hoc code to process class files and intends to migrate
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional
> commit since the last revision:
>
> remove: removed unnecessary logging
test/jdk/java/lang/invoke/indify/Indify.java line 386:
> 384:
> 385: byte[] transformToBytes(ClassModel classModel) {
> 386: return of().transform(classModel, ClassTransform.ACCEPT_ALL);
What is the purpose of this transformation?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652808158
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Sat, 22 Jun 2024 15:55:28 GMT, Oussama Louati wrote: >> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code >> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, >> MethodType, and CallSite constants. >> ### Purpose of Indify >> >> - **Transformation Tool**: `Indify` transforms existing class files to >> leverage `invokedynamic` and other JSR 292 features. This transformation >> allows for dynamic method calls. >> >> ### Key Functions >> >> - **Method Handle and Method Type Constants**: >> - **MH_x and MT_x Methods**: These methods are used to generate >> `MethodHandle` and `MethodType` constants. The tool transforms calls to >> these methods into `CONSTANT_MethodHandle` and `CONSTANT_MethodType` "ldc" >> (load constant) instructions. >> - **Invokedynamic Call Sites**: >> - **INDY_x Methods**: These methods generate `invokedynamic` call sites. >> Each `INDY_x` method starts with a call to an `MH_x` method, which acts as >> the bootstrap method. This is followed by an `invokeExact` call. >> - **Transformation**: The tool transforms pairs of calls (to `INDY_x` >> followed by `invokeExact`) into `invokedynamic` instructions. This mimics >> the JVM's handling of `invokedynamic` instructions, creating dynamic call >> sites that are linked at runtime. >> >> It currently uses ad-hoc code to process class files and intends to migrate >> to ASM; but since we have the Classfile API, we can migrate to Classfile API >> instead. > > Oussama Louati has updated the pull request incrementally with one additional > commit since the last revision: > > remove: removed unnecessary logging Unfortunately this pull request is not converging to an acceptable state. Core transformation should be designed from top to down, with understanding of the functionality and with respect to Class-File API architecture. Currently proposed implementation is confusing mix of the original code, glued together with fragments of Class-File API. The patch also contains many cosmetic changes and unrelated semantic changes making the patch very hard to read. The PR could not be finished just by requesting reviewers attention and partly reflecting their suggestions, it should be significantly re-designed. - Changes requested by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18841#pullrequestreview-2138609501
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v2]
On Tue, 25 Jun 2024 09:36:37 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more lambdas conversions to fix bootstrap Unfortunately it is still failing. This is the exception stack trace: [StackOverflow2.log](https://github.com/user-attachments/files/15971514/StackOverflow2.log) Thanks, Richard. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188858307
Re: RFR: 8334397: RISC-V: verify perf of ReverseBytesS/US
On Tue, 25 Jun 2024 08:12:53 GMT, Hamlin Li wrote:
>> test/micro/org/openjdk/bench/java/lang/Characters.java line 69:
>>
>>> 67:
>>> 68: @Benchmark
>>> 69: public void reverseBytes() {
>>
>> I'm not sure we want to be adding that benchmark to this class. Could you
>> move to a different class that will test exclusively `reverseBytes` on
>> `char[]`? You can then move the code from
>> `test/micro/org/openjdk/bench/java/lang/Shorts.java` into that same class or
>> a similarly named class for `short[]`.
>
> Not sure if I understand you correctly.
> The test is for reverseBytes of a char, not char[]. And it's quite similar as
> existing test in for integer at:
> https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/lang/Integers.java#L171
I meant put this `reverseBytes` benchmark in a different class, but given it's
how it's done in
https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/lang/Integers.java#L171,
please ignore this comment.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19830#discussion_r1652763769
Re: RFR: 8334397: RISC-V: verify perf of ReverseBytesS/US
On Fri, 21 Jun 2024 14:24:26 GMT, Hamlin Li wrote: > Hi, > Can you help to review this patch? > Thanks! > > This is similar with previous JDK-8334396. > Added some tests. > > ### Test > > | Tests | Scores | Errors | Unit > -- | -- | -- | -- | -- > Intrinsic, +zbb, +rvv | Characters.reverseBytes | 1654.535 | 69.36 | ns/op > | Shorts.reverseBytes | 1795.403 | 44.015 | ns/op > Intrinsic, +zbb, -rvv | Characters.reverseBytes | 1649.752 | 74.965 | ns/op > | Shorts.reverseBytes | 1798.637 | 49.52 | ns/op > Intrinsic, -zbb, +rvv | Characters.reverseBytes | 2279.588 | 44.222 | ns/op > | Shorts.reverseBytes | 2441.674 | 63.895 | ns/op > Intrinsic, -zbb, -rvv | Characters.reverseBytes | 2288.876 | 49.099 | ns/op > | Shorts.reverseBytes | 2454.454 | 94.004 | ns/op > No intrinsic | Characters.reverseBytes | 1629.722 | 23.656 | ns/op > | Shorts.reverseBytes | 2108.81 | 43.378 | ns/op > > Marked as reviewed by luhenry (Committer). - PR Review: https://git.openjdk.org/jdk/pull/19830#pullrequestreview-2138583857
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 16:01:41 GMT, Adam Sotona wrote: > After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam More lambdas converted, hopefully it is the last round. @reinrich Please re-test. Thank you. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188431487
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v2]
> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: more lambdas conversions to fix bootstrap - Changes: - all: https://git.openjdk.org/jdk/pull/19863/files - new: https://git.openjdk.org/jdk/pull/19863/files/ac8fb2d5..357d36a0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19863=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19863=00-01 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19863.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19863/head:pull/19863 PR: https://git.openjdk.org/jdk/pull/19863
Re: RFR: 8334755: Asymptotically faster implementation of square root algorithm [v12]
> I have implemented the Zimmermann's square root algorithm, available in works > [here](https://inria.hal.science/inria-00072854/en/) and > [here](https://www.researchgate.net/publication/220532560_A_proof_of_GMP_square_root). > > The algorithm is proved to be asymptotically faster than the Newton's Method, > even for small numbers. To get an idea of how much the Newton's Method is > slow, consult my article [here](https://arxiv.org/abs/2406.07751), in which > I compare Newton's Method with a version of classical square root algorithm > that I implemented. After implementing Zimmermann's algorithm, it turns out > that it is faster than my algorithm even for small numbers. fabioromano1 has updated the pull request incrementally with one additional commit since the last revision: Optimized multiplication - Changes: - all: https://git.openjdk.org/jdk/pull/19710/files - new: https://git.openjdk.org/jdk/pull/19710/files/f895b63b..923b3475 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19710=11 - incr: https://webrevs.openjdk.org/?repo=jdk=19710=10-11 Stats: 28 lines in 1 file changed: 10 ins; 0 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/19710.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19710/head:pull/19710 PR: https://git.openjdk.org/jdk/pull/19710
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 16:01:41 GMT, Adam Sotona wrote: > After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Yes, I'm working on it. Class-File API avoids lambdas on critical JDK paths only. Unfortunately we did not have the full map of the critical paths on all platforms. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188311520
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 16:01:41 GMT, Adam Sotona wrote: > After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Looks like there is a bootstrap cycle initializing this lambda in ClassBuilder: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/classfile/ClassBuilder.java#L202 I'm not sure, but it looks like the PPC VM might be passing args to the bootstrap method differently, which triggers adapting code in BootstrapMethodInvoker::pushMePullYou - this might be enough to start this cycle. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188298757
Re: RFR: 8331051: Add an `@since` checker test for `java.base` module [v10]
> This checker checks the values of the `@since` tag found in the documentation > comment for an element against the release in which the element first > appeared. > > Real since value of an API element is computed as the oldest release in which > the given API element was introduced. That is: > - for modules, classes and interfaces, the release in which the element with > the given qualified name was introduced > - for constructors, the release in which the constructor with the given VM > descriptor was introduced > - for methods and fields, the release in which the given method or field with > the given VM descriptor became a member of its enclosing class or interface, > whether direct or inherited > > Effective since value of an API element is computed as follows: > - if the given element has a `@since` tag in its javadoc, it is used > - in all other cases, return the effective since value of the enclosing > element > > The since checker verifies that for every API element, the real since value > and the effective since value are the same, and reports an error if they are > not. > > Preview method are handled as per JEP 12, if `@PreviewFeature` is used > consistently going forward then the checker doesn't need to be updated with > every release. The checker has explicit knowledge of preview elements that > came before `JDK 14` because they weren't marked in a machine understandable > way and preview elements that came before `JDK 17` that didn't have > `@PreviewFeature`. > > Important note : We only check code written since `JDK 9` as the releases > used to determine the expected value of `@since` tags are taken from the > historical data built into `javac` which only goes back that far > > The intial comment at the beginning of `SinceChecker.java` holds more > information into the program. > > I already have filed issues and fixed some wrong tags like in #18640, #18032, > #18030, #18055, #18373, #18954, #18972. Nizar Benalla has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits: - Move the tests to module/module_name directory for clearer naming - (C) - Merge remote-tracking branch 'upstream/master' into source-based-since-checker # Conflicts: #test/jdk/TEST.groups - Improve checker report messages - Merge branch 'master' into source-based-since-checker - - removed unused parameter `i` - make rest of methods private - ".toString" instead of "toString" Signed-off-by: Nizar Benalla - Add a few more legacy methods, needed a few more after changes to the checker - checking for null values directly rather than catching NPE - Merge remote-tracking branch 'upstream/master' into source-based-since-checker - Now only skipping the common methods that every record class will have, and will never need a new `@since` - ... and 10 more: https://git.openjdk.org/jdk/compare/7baddc20...62aebb0b - Changes: https://git.openjdk.org/jdk/pull/18934/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18934=09 Stats: 959 lines in 3 files changed: 957 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18934.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18934/head:pull/18934 PR: https://git.openjdk.org/jdk/pull/18934
Re: RFR: 8334287: Man page update for jstatd deprecation
On Fri, 21 Jun 2024 14:05:51 GMT, Kevin Walls wrote: > Man page update for JDK-8327793 which marked jstatd as deprecated for removal > in JDK 24. Thanks Alan and David, moving to a single line: JSTATD(1) JDK CommandsJSTATD(1) NAME jstatd - monitor the creation and termination of instrumented Java HotSpot VMs SYNOPSIS WARNING: This command is experimental, unsupported, and deprecated. It will be removed in a future release. jstatd [options] - PR Comment: https://git.openjdk.org/jdk/pull/19829#issuecomment-2188272502
Re: RFR: 8334287: Man page update for jstatd deprecation [v2]
On Tue, 25 Jun 2024 08:25:00 GMT, Kevin Walls wrote: >> Man page update for JDK-8327793 which marked jstatd as deprecated for >> removal in JDK 24. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > text update Marked as reviewed by alanb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19829#pullrequestreview-2137853292
Re: RFR: 8334287: Man page update for jstatd deprecation [v2]
> Man page update for JDK-8327793 which marked jstatd as deprecated for removal > in JDK 24. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: text update - Changes: - all: https://git.openjdk.org/jdk/pull/19829/files - new: https://git.openjdk.org/jdk/pull/19829/files/55f6dbea..6c41666a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19829=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19829=00-01 Stats: 4 lines in 1 file changed: 0 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19829.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19829/head:pull/19829 PR: https://git.openjdk.org/jdk/pull/19829
Re: RFR: 8334397: RISC-V: verify perf of ReverseBytesS/US
On Mon, 24 Jun 2024 21:01:14 GMT, Ludovic Henry wrote:
>> Hi,
>> Can you help to review this patch?
>> Thanks!
>>
>> This is similar with previous JDK-8334396.
>> Added some tests.
>>
>> ### Test
>>
>> | Tests | Scores | Errors | Unit
>> -- | -- | -- | -- | --
>> Intrinsic, +zbb, +rvv | Characters.reverseBytes | 1654.535 | 69.36 | ns/op
>> | Shorts.reverseBytes | 1795.403 | 44.015 | ns/op
>> Intrinsic, +zbb, -rvv | Characters.reverseBytes | 1649.752 | 74.965 | ns/op
>> | Shorts.reverseBytes | 1798.637 | 49.52 | ns/op
>> Intrinsic, -zbb, +rvv | Characters.reverseBytes | 2279.588 | 44.222 | ns/op
>> | Shorts.reverseBytes | 2441.674 | 63.895 | ns/op
>> Intrinsic, -zbb, -rvv | Characters.reverseBytes | 2288.876 | 49.099 | ns/op
>> | Shorts.reverseBytes | 2454.454 | 94.004 | ns/op
>> No intrinsic | Characters.reverseBytes | 1629.722 | 23.656 | ns/op
>> | Shorts.reverseBytes | 2108.81 | 43.378 | ns/op
>>
>>
>
> test/micro/org/openjdk/bench/java/lang/Characters.java line 69:
>
>> 67:
>> 68: @Benchmark
>> 69: public void reverseBytes() {
>
> I'm not sure we want to be adding that benchmark to this class. Could you
> move to a different class that will test exclusively `reverseBytes` on
> `char[]`? You can then move the code from
> `test/micro/org/openjdk/bench/java/lang/Shorts.java` into that same class or
> a similarly named class for `short[]`.
Not sure if I understand you correctly.
The test is for reverseBytes of a char, not char[]. And it's quite similar as
existing test in for integer at:
https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/lang/Integers.java#L171
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19830#discussion_r1652215989
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 22:11:55 GMT, Richard Reingruber wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Hi Adam, > > I've tested this change on a Linux PPC64 big endian system. Sadly I still get > StackOverflowErrors. > I've run only test/jdk/java/lang/invoke/condy/ConstantBootstrapsTest.java. > I've experimented with `ThreadStackSize` and `VMThreadStackSize` without > success. > > When running the test standalone I get the following message: > > > ./jdk/bin/java > -cp > testclasses:jtreg_latest/lib/testng-7.3.0.jar:jtreg_latest/lib/jcommander-1.82.jar > -XX:+UnlockDiagnosticVMOptions > -XX:UseBootstrapCallInfo=3 > -Xint > org.testng.TestNG > -testclass > ConstantBootstrapsTest > Error occurred during initialization of boot layer > java.lang.StackOverflowError > > > It's likely not a byte ordering problem since @offamitkumar cannot reproduce > the failures on s390x. > @reinrich Could you try running with `-XX:+UnlockDiagnosticVMOptions > -XX:+ShowHiddenFrames` and share the full stacktrace again? I've done that now. Please find it in the description/attachment of the [JBS-item](https://bugs.openjdk.org/browse/JDK-8334872). - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188155291
Re: RFR: 8334810: Redo: Un-ProblemList LocaleProvidersRun and CalendarDataRegression
On Mon, 24 Jun 2024 04:25:45 GMT, Yude Lin wrote: > [JDK-8318107](https://bugs.openjdk.org/browse/JDK-8318107) Un-ProblemListed > LocaleProvidersRun and CalendarDataRegression, and > [JDK-8288899](https://bugs.openjdk.org/browse/JDK-8288899) added them back. > I'm guessing it's a mistake in resolving merge conflict. Hello Yude, can you do `/issue add 8268379` so that even 8268379 gets resolved when this PR gets integrated? Otherwise, 8268379 will still remain open after removing the tests from the problem listing. - PR Comment: https://git.openjdk.org/jdk/pull/19849#issuecomment-2188111502
Re: RFR: 8334287: Man page update for jstatd deprecation
On Fri, 21 Jun 2024 14:05:51 GMT, Kevin Walls wrote: > Man page update for JDK-8327793 which marked jstatd as deprecated for removal > in JDK 24. Sorry this got left "pending" yesterday src/jdk.jstatd/share/man/jstatd.1 line 49: > 47: future release. > 48: .PP > 49: \f[B]Note:\f[R] This command is experimental and unsupported. Can we combine the new warning with the old note e.g. > This command is deprecated and will be removed in a future release. It was > designated as experimental and unsupported. ? - PR Review: https://git.openjdk.org/jdk/pull/19829#pullrequestreview-2134703147 PR Review Comment: https://git.openjdk.org/jdk/pull/19829#discussion_r1650378180
