Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache src/java.base/share/classes/java/lang/Long.java line 1233: > 1231: HEX256[i] = (char) (((hi < 10 ? '0' + hi : 'a' + hi - > 10) << 8) > 1232: + (lo < 10 ? '0' + lo : 'a' + lo - 10)); > 1233: } Did you mean to put this in LongCache and is the intention it be archived or are you putting this into its own holder class? Right now it's confusing as HEX256 is not read from the archive. - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1238023213
Integrated: 8309853: StructuredTaskScope.join description improvements
On Mon, 12 Jun 2023 14:32:07 GMT, Alan Bateman wrote: > StructuredTaskScope's class description introduces the join method as waiting > for all subtasks to finish but the API docs for join/joinUntil are phrased in > terms of waiting for all threads to finish. ShutdownOnXXX join/joinUntil > inherit this description but would be clearer if their description said they > wait until all subtasks finished or a subtask to succeed or fail. This pull request has now been integrated. Changeset: 3661cdee Author:Alan Bateman URL: https://git.openjdk.org/jdk/commit/3661cdee1b20ab2868025637871d22bb30add6bd Stats: 82 lines in 1 file changed: 54 ins; 3 del; 25 mod 8309853: StructuredTaskScope.join description improvements Reviewed-by: rpressler, darcy - PR: https://git.openjdk.org/jdk/pull/14419
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote:
>> By optimizing the implementation of java.lang.Long#fastUUID, the performance
>> of the java.util.UUID#toString method can be significantly improved.
>>
>> The following are the test results of JMH:
>>
>> Benchmark Mode Cnt Score Error Units
>> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms
>> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit
> since the last revision:
>
> move HEX256 to LongCache
I've tested the benchmarks and the patch and baseline (with extra stable
annotation) with a slightly varied version suitable for gradle run:
package com.alibaba.openjdk;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
@BenchmarkMode(Mode.Throughput)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Warmup(iterations = 3, time = 10)
@Measurement(iterations = 6, time = 5)
@Fork(1)
public class UUIDUtilsBenchmark {
public static UUID uuid = UUID.randomUUID();
@Benchmark
public void jdk(Blackhole bh) {
bh.consume(uuid.toString());
}
@Benchmark
public void fast(Blackhole bh) {
bh.consume(UUIDUtils.fastUUID(uuid));
}
}
The throughput varies a lot between iterations somehow; the patch and baseline
with stable has no significant difference (i.e. within the error range, about
10%)
-
PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601953110
Re: RFR: 8308780: Fix the Java Integer types on Windows [v6]
> On Windows, the basic Java Integer types are defined as long and __int64 > respectively. In particular, the former is rather problematic since it breaks > compilation as the Visual C++ becomes stricter and more compliant with every > release, which means the way Windows code treats long as a typedef for int is > no longer correct, especially with -permissive- enabled. Instead of changing > every piece of broken code to match the jint = long typedef, which is far too > time consuming, we can instead change jint to an int (which is still the same > 32 bit number type as long), as there are far fewer problems caused by this > definition. It's better to get this over and done with sooner than later when > a future version of Visual C++ finally starts to break on existing code Julian Waters has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: Revert NULL to nullptr changes in jaccesswalker - Changes: - all: https://git.openjdk.org/jdk/pull/14125/files - new: https://git.openjdk.org/jdk/pull/14125/files/9a8a9158..a31e52e9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14125=05 - incr: https://webrevs.openjdk.org/?repo=jdk=14125=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14125.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14125/head:pull/14125 PR: https://git.openjdk.org/jdk/pull/14125
Re: RFR: 8308780: Fix the Java Integer types on Windows [v5]
> On Windows, the basic Java Integer types are defined as long and __int64 > respectively. In particular, the former is rather problematic since it breaks > compilation as the Visual C++ becomes stricter and more compliant with every > release, which means the way Windows code treats long as a typedef for int is > no longer correct, especially with -permissive- enabled. Instead of changing > every piece of broken code to match the jint = long typedef, which is far too > time consuming, we can instead change jint to an int (which is still the same > 32 bit number type as long), as there are far fewer problems caused by this > definition. It's better to get this over and done with sooner than later when > a future version of Visual C++ finally starts to break on existing code Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Revert NULL to nullptr changes in jaccesswalker - Changes: - all: https://git.openjdk.org/jdk/pull/14125/files - new: https://git.openjdk.org/jdk/pull/14125/files/5fa2d3eb..9a8a9158 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14125=04 - incr: https://webrevs.openjdk.org/?repo=jdk=14125=03-04 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/14125.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14125/head:pull/14125 PR: https://git.openjdk.org/jdk/pull/14125
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v7]
On Thu, 22 Jun 2023 00:33:33 GMT, Mandy Chung wrote:
>> This PR clarifies the spec of the 3-arg Class::forName regarding the format
>> of the name for an array type which is of the form: one or more of "[" +
>> binary name of the element type + ";'.
>
> Mandy Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review comments
src/java.base/share/classes/java/lang/Class.java line 442:
> 440: * whose name is {@code I} instead.
> 441: *
> 442: * To obtain {@code Class} object associated with an array class,
To obtain _the_ Class object ...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237907917
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v6]
On Wed, 21 Jun 2023 23:06:23 GMT, Chen Liang wrote:
>> Mandy Chung 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 eight additional
>> commits since the last revision:
>>
>> - improve the specification of forName
>> - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242
>> - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242
>> - Review comment. Add a test
>> - missing 'L' for the array class name
>> - review comment
>> - Clarify for an array class of primitive type
>> - 8310242: Clarify the name parameter to Class::forName
>
> src/java.base/share/classes/java/lang/Class.java line 438:
>
>> 436: * representing primitive types or void, hidden classes or
>> interfaces,
>> 437: * or array classes whose element type is a hidden class or
>> interface.
>> 438: * If {@code name} denotes a primitive type or void, for example
>> {@code I},
>
> I think `{@code int}` would be a better example here as Java programmers
> aren't in touch with bytecode descriptors mostly.
One main point for this example to show is that it will find a class in unnamed
package named `I` - a class named `I` is not uncommon but it's unlikely to have
a class named `int`.
> src/java.base/share/classes/java/lang/Class.java line 459:
>
>> 457: * from {@code forName(}N{@code )} returns N.
>> 458: *
>> 459: * A call to {@code forName("[L}N{@code ;")} causes the
>> element type
>
> This appears true for multi-dimensional arrays as well, but the name format
> here only applies to single-dimension arrays.
I considered that. The spec and the example already describe
multi-dimensional array. I think one can infer from this single-dimensional
array example.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237904930
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237906383
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache Changes requested by liach (Author). About @Glavo's VH suggestion: I think it is feasible, since the VH field is not initialized until the method is used, so there should be no startup issue. On a side note, JDK itself has a `UUIDBench` that benchmarks toString as well: can run it with `make test TEST="micro:java.util.UUIDBench.toString"` once the configuration has JMH set up, which I will be using (against master and this patch) src/java.base/share/classes/java/lang/Long.java line 484: > 482: > 483: buf[off] = (byte) (i >> 8); > 484: buf[1 * charSize + off] = (byte) i; Suggestion: buf[off] = (byte) (i0 >> 8); buf[1 * charSize + off] = (byte) i0; Doesn't compile on my end - PR Review: https://git.openjdk.org/jdk/pull/14578#pullrequestreview-1492220846 PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601915404 PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237906554
Re: RFR: 8309819: Clarify API note in Class::getName and MethodType::toMethodDescriptorString [v7]
> The API specification for descriptorString not being a strict inverse of > Class::forName and MethodType::fromDescriptorString are not entirely correct. > > 1. Class::descriptorString was never an inverse of Class::forName, which > takes a binary name instead. Class::getName was a partial inverse instead. > 2. MethodType::toMethodDescriptorString ends with a meaningless sentence: > "fromMethodDescriptorString, because the latter requires a suitable class > loader argument.", and the "Note:" section can be replaced with an `@apiNote`. > 3. Both of these didn't mention hidden classes (or other > non-nominally-describable classes) as a reason that prevents the inversion > operation, in addition to distinct classloaders. > > A few user-defined anchor links are replaced with updated javadoc link tag > format as well. The explicit html-style links in `@see` tags are unchanged in > order to retain the non-code output. > > The rendered specifications: > https://cr.openjdk.org/~liach/8309819/04/java.base/java/lang/Class.html > https://cr.openjdk.org/~liach/8309819/04/java.base/java/lang/invoke/MethodType.html Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Missed recommendations from Alan - Changes: - all: https://git.openjdk.org/jdk/pull/14411/files - new: https://git.openjdk.org/jdk/pull/14411/files/bf17845e..be3e8cd3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14411=06 - incr: https://webrevs.openjdk.org/?repo=jdk=14411=05-06 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14411.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14411/head:pull/14411 PR: https://git.openjdk.org/jdk/pull/14411
Re: RFR: 8309819: Clarify API note in Class::getName and MethodType::toMethodDescriptorString [v5]
On Thu, 15 Jun 2023 10:01:31 GMT, Alan Bateman wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update the note for getName
>
> src/java.base/share/classes/java/lang/invoke/MethodType.java line 1233:
>
>> 1231: * (JVMS {@jvms 4.3.3}) and a suitable class loader argument. Two
>> distinct
>> 1232: * classes which share a common name but have different class
>> loaders will
>> 1233: * appear identical when viewed within descriptor strings.
>
> "will appear identical when viewed within descriptor strings". Would it be
> clearer to say that have equal descriptor strings?
How about this:
"A method type produced by changing a component type to a distinct class with
the same name but a different class loader will have the same descriptor string"
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14411#discussion_r1237878347
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache The Bots have removed the warning so the titles match. Please wait 24hrs to integrate to give anyone who has commented a chance to review and approve. - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601884222
RFR: 8310234: Refactor Locale tests to use JUnit
Please review this PR as apart of [JDK-8307843](https://bugs.openjdk.org/browse/JDK-8307843) which refactors some tests in Locale to use JUnit. Other cleanup and small changes are included as well. More refactoring in Locale tests will be done in separate PRs. If the test had a bugN.java name, it was also renamed to something more [descriptive](https://openjdk.org/jtreg/faq.html#how-should-i-name-a-test). Below is a list of all the changes, - Refactor Bug4316602.java as LocaleConstructors.java - Refactor Bug4210525.java as CaseCheckVariant.java - Refactor bug6277243.java as RootLocale.java - Refactor bug6312358.java as GetInstanceCheck.java - Refactor Bug8154797.java as CompareProviderFormats.java - Refactor Bug8004240.java as GetAdapterPreference.java - Refactor bug4122700.java into AvailableLocalesTest.java (and combined with StreamAvailableLocales.java) - Commit messages: - Minor cleanup to various files - Refactor Bug8004240.java as GetAdapterPreference.java - Revert "Rename bug4123285.java as LocalesInApplet.java + minor cleanup, do not refactor to junit as depends on Applet" - Typo in CompareProviderFormats.java - Refactor Bug8154797.java as CompareProviderFormats.java - Rename bug4123285.java as LocalesInApplet.java + minor cleanup, do not refactor to junit as depends on Applet - Refactor bug6312358.java as GetInstanceCheck.java - Refactor bug6277243.java as RootLocale.java - Case of vars in LocaleConstructors.java - Refactor Bug4210525 as CaseCheckVariant - ... and 4 more: https://git.openjdk.org/jdk/compare/3e0bbd29...02252fb5 Changes: https://git.openjdk.org/jdk/pull/14609/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14609=00 Issue: https://bugs.openjdk.org/browse/JDK-8310234 Stats: 1104 lines in 15 files changed: 597 ins; 507 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14609/head:pull/14609 PR: https://git.openjdk.org/jdk/pull/14609
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 01:03:11 GMT, Roger Riggs wrote: > fyi, the title of this PR need to match exactly the title of the bug > [JDK-8310502](https://bugs.openjdk.org/browse/JDK-8310502). The mismatch is > an Integration blocker. (See the comment in the description). i have made the changes. is it ok now? - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601878855
Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v7]
On Thu, 22 Jun 2023 00:13:06 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > move HEX256 to LongCache fyi, the title of this PR need to match exactly the title of the bug [JDK-8310502](https://bugs.openjdk.org/browse/JDK-8310502). The mismatch is an Integration blocker. (See the comment in the description). - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601874909
Re: RFR: JDK-8310571: Use inline @return tag on java.util.Objects
On Thu, 22 Jun 2023 00:19:03 GMT, Joe Darcy wrote:
> Small cleanup, minor differences in the wording of portions of
> toString(Object, String), nonNull(Object), requireNonNullElse, and
> requireNonNullElseGet.
Looks good. I had to refresh my understanding of the exact behavior of the
inline `@return` tag:
https://docs.oracle.com/en/java/javase/17/docs/specs/javadoc/doc-comment-spec.html
(search for `{@return`)
It adds a leading "Returns " and a trailing "." to the block's text, and
inserts a "Returns" section in the proper place. This seems oddly specific, but
it's tailored for this exact use case. Anyway, good cleanup.
-
Marked as reviewed by smarks (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14608#pullrequestreview-1492140246
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v6]
On Wed, 21 Jun 2023 22:02:16 GMT, Mandy Chung wrote:
>> This PR clarifies the spec of the 3-arg Class::forName regarding the format
>> of the name for an array type which is of the form: one or more of "[" +
>> binary name of the element type + ";'.
>
> Mandy Chung 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 eight additional
> commits since the last revision:
>
> - improve the specification of forName
> - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242
> - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242
> - Review comment. Add a test
> - missing 'L' for the array class name
> - review comment
> - Clarify for an array class of primitive type
> - 8310242: Clarify the name parameter to Class::forName
src/java.base/share/classes/java/lang/Class.java line 438:
> 436: * representing primitive types or void, hidden classes or
> interfaces,
> 437: * or array classes whose element type is a hidden class or
> interface.
> 438: * If {@code name} denotes a primitive type or void, for example
> {@code I},
I think `{@code int}` would be a better example here as Java programmers aren't
in touch with bytecode descriptors mostly.
src/java.base/share/classes/java/lang/Class.java line 459:
> 457: * from {@code forName(}N{@code )} returns N.
> 458: *
> 459: * A call to {@code forName("[L}N{@code ;")} causes the
> element type
This appears true for multi-dimensional arrays as well, but the name format
here only applies to single-dimension arrays.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237808121
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237809409
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v7]
On Thu, 22 Jun 2023 00:33:33 GMT, Mandy Chung wrote:
>> This PR clarifies the spec of the 3-arg Class::forName regarding the format
>> of the name for an array type which is of the form: one or more of "[" +
>> binary name of the element type + ";'.
>
> Mandy Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review comments
src/java.base/share/classes/java/lang/Class.java line 426:
> 424: /**
> 425: * Returns the {@code Class} object associated with the class or
> 426: * interface with the given string name, using the given class
> loader.
Should we update the summary to `... associated with the class or interface or
array with the given string name...`
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237860510
Re: RFR: 8309819: Clarify API note in Class::getName and MethodType::toMethodDescriptorString [v6]
> The API specification for descriptorString not being a strict inverse of > Class::forName and MethodType::fromDescriptorString are not entirely correct. > > 1. Class::descriptorString was never an inverse of Class::forName, which > takes a binary name instead. Class::getName was a partial inverse instead. > 2. MethodType::toMethodDescriptorString ends with a meaningless sentence: > "fromMethodDescriptorString, because the latter requires a suitable class > loader argument.", and the "Note:" section can be replaced with an `@apiNote`. > 3. Both of these didn't mention hidden classes (or other > non-nominally-describable classes) as a reason that prevents the inversion > operation, in addition to distinct classloaders. > > A few user-defined anchor links are replaced with updated javadoc link tag > format as well. The explicit html-style links in `@see` tags are unchanged in > order to retain the non-code output. > > The rendered specifications: > https://cr.openjdk.org/~liach/8309819/04/java.base/java/lang/Class.html > https://cr.openjdk.org/~liach/8309819/04/java.base/java/lang/invoke/MethodType.html Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision: - Review and updates for 8310242 - Merge branch 'master' into fix/descstring-spec - Update the note for getName - Convert the note in fromDescriptorString to apiNote - Address the other two review comments - Merge branch 'fix/descstring-spec' of https://github.com/liachmodded/jdk into fix/descstring-spec - Update src/java.base/share/classes/java/lang/Class.java Co-authored-by: Mandy Chung - Merge branch 'master' into fix/descstring-spec - 8309819: Fix specification about descriptor inverses in Class and MethodTypeDesc - Changes: - all: https://git.openjdk.org/jdk/pull/14411/files - new: https://git.openjdk.org/jdk/pull/14411/files/ff26c099..bf17845e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14411=05 - incr: https://webrevs.openjdk.org/?repo=jdk=14411=04-05 Stats: 15010 lines in 685 files changed: 8365 ins; 3131 del; 3514 mod Patch: https://git.openjdk.org/jdk/pull/14411.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14411/head:pull/14411 PR: https://git.openjdk.org/jdk/pull/14411
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v7]
> This PR clarifies the spec of the 3-arg Class::forName regarding the format > of the name for an array type which is of the form: one or more of "[" + > binary name of the element type + ";'. Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: review comments - Changes: - all: https://git.openjdk.org/jdk/pull/14528/files - new: https://git.openjdk.org/jdk/pull/14528/files/39e218ea..852bd774 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14528=06 - incr: https://webrevs.openjdk.org/?repo=jdk=14528=05-06 Stats: 15 lines in 2 files changed: 8 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/14528.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14528/head:pull/14528 PR: https://git.openjdk.org/jdk/pull/14528
Re: RFR: 8310502 : optimization for UUID#toString [v3]
On Wed, 21 Jun 2023 13:32:32 GMT, 温绍锦 wrote: >> 温绍锦 has updated the pull request incrementally with one additional commit >> since the last revision: >> >> 8310502 : hex literal > > * benchmark code > https://github.com/wenshao/jdk_8310502_test/blob/main/src/main/java/com/alibaba/openjdk/UUIDUtilsBenchmark.java > > > git clone https://github.com/wenshao/jdk_8310502_test > cd jdk_8310502_test > mvn clean package -Dmaven.test.skip > java -cp target/jdk_8310502_benchmark.jar > com.alibaba.openjdk.UUIDUtilsBenchmark > > > * benchmark result on Apple MacBook M1 Max > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.fast thrpt5 701840.078 ± 57597.624 ops/ms > UUIDUtilsBenchmark.jdk thrpt5 246409.000 ± 84564.009 ops/ms > > > * benchmark result on > [ecs.c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.fast thrpt5 131595.572 ? 7383.223 ops/ms > UUIDUtilsBenchmark.jdk thrpt5 129610.735 ? 455.752 ops/ms > > > * benchmark result on > [ecs.c8y.xlarge](https://help.aliyun.com/document_detail/25378.html#c8y) > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.fast thrpt5 129445.610 ? 1846.775 ops/ms > UUIDUtilsBenchmark.jdk thrpt5 104651.872 ? 1005.977 ops/ms > @wenshao Please do not rebase or force-push to an active PR as it invalidates > existing review comments. Note for future reference, the bots always squash > all changes into a single commit automatically as part of the integration. > See [OpenJDK Developers’ > Guide](https://openjdk.org/guide/#working-with-pull-requests) for more > information. got it - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1601851348
RFR: JDK-8310571: Use inline @return tag on java.util.Objects
Small cleanup, minor differences in the wording of portions of toString(Object, String), nonNull(Object), requireNonNullElse, and requireNonNullElseGet. - Commit messages: - JDK-8310571: Use inline @return tag on java.util.Objects Changes: https://git.openjdk.org/jdk/pull/14608/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14608=00 Issue: https://bugs.openjdk.org/browse/JDK-8310571 Stats: 41 lines in 1 file changed: 0 ins; 21 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/14608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14608/head:pull/14608 PR: https://git.openjdk.org/jdk/pull/14608
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v6]
On Wed, 21 Jun 2023 22:02:16 GMT, Mandy Chung wrote:
>> This PR clarifies the spec of the 3-arg Class::forName regarding the format
>> of the name for an array type which is of the form: one or more of "[" +
>> binary name of the element type + ";'.
>
> Mandy Chung 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 eight additional
> commits since the last revision:
>
> - improve the specification of forName
> - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242
> - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242
> - Review comment. Add a test
> - missing 'L' for the array class name
> - review comment
> - Clarify for an array class of primitive type
> - 8310242: Clarify the name parameter to Class::forName
A few small suggestions, otherwise looks good.
Thanks.
src/java.base/share/classes/java/lang/Class.java line 444:
> 442: * To obtain {@code Class} object associated with an array class,
> 443: * the name consists of one or more {@code '['} representing the
> depth
> 444: * of the array class, followed by the element type as encoded in
Suggestion
+ * To obtain the {@code Class} object associated with an array class,
+ * the name consists of one or more {@code '['} representing the depth
+ * of the array nesting, followed by the element type as encoded in
src/java.base/share/classes/java/lang/Class.java line 451:
> 449: * Class threadClass = Class.forName("java.lang.Thread", false,
> currentLoader);
> 450: * Class stringArrayClass = Class.forName("[Ljava.lang.String;",
> false, currentLoader);
> 451: * Class intArrayClass = Class.forName("[[[I", false,
> currentLoader);
The variable name doesn't match the actual class loaded. It might be clearer to
add a trailing comment `// Class of int[][][]` ?
test/jdk/java/lang/Class/forName/ForNameNames.java line 52:
> 50: @ParameterizedTest
> 51: @MethodSource("testCases")
> 52: void testForName(String cn, Class expected) throws
> ClassNotFoundException {
Should this also test that `c.getName().equals(cn)`?
-
Changes requested by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14528#pullrequestreview-1492109276
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237843279
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237843986
PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1237846267
Re: RFR: 8310502 : optimization for UUID#toString [v7]
On Wed, 21 Jun 2023 15:21:45 GMT, Roger Riggs wrote: > CDS can initialize arrays efficiently. The Long class already uses CDS to > initialize the cache of Long values. See the LongCache code for an example of > initializing from CDS with a fallback to direct initialization. You could > move the HEX256 array to the LongCache holder class and avoid any performance > hit on startup. done - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237840813
Re: RFR: 8310502 : optimization for UUID#toString [v7]
> By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: move HEX256 to LongCache - Changes: - all: https://git.openjdk.org/jdk/pull/14578/files - new: https://git.openjdk.org/jdk/pull/14578/files/ad53343f..e0bcf6cd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14578=06 - incr: https://webrevs.openjdk.org/?repo=jdk=14578=05-06 Stats: 54 lines in 1 file changed: 11 ins; 42 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
Re: RFR: 8310405: Linker.Option.firstVariadicArg should specify which index values are valid [v2]
On Wed, 21 Jun 2023 07:34:20 GMT, Per Minborg wrote: >> Possible suggestion/thing to try: use a bullet list to spell out all cases >> for `index`. E.g. we know there's index == 0 (all variadic). Then we know >> there's index = N (no variadic). Then there's index == m, 0 < m < N - which >> means layouts 0..m are non-variadic and m..N are variadic (where n..m >> denotes an interval with n included and m excluded). > >> Possible suggestion/thing to try: use a bullet list to spell out all cases >> for `index`. E.g. we know there's index == 0 (all variadic). Then we know >> there's index = N (no variadic). Then there's index == m, 0 < m < N - which >> means layouts 0..m are non-variadic and m..N are variadic (where n..m >> denotes an interval with n included and m excluded). > > I think this is a good suggestion. It makes it much easier to understand. Thanks for the review. I've added a bullet list, and switch same of the language to refer to the 'start of the variadic arguments passed to the function described by the function descriptor'. I think the latter avoids implying the index is an index into the argument layouts, but it feels like a bit of a mouthful (any suggestions?). I've also added a small note to the global variadic function doc to indicate that the index might not necessarily have a corresponding argument layout. How does the new version look? - PR Review Comment: https://git.openjdk.org/jdk/pull/14565#discussion_r1237819366
Re: RFR: 8310405: Linker.Option.firstVariadicArg should specify which index values are valid [v2]
> Improve the specification of `Linker.Option.firstVariadicArg`, by specifying > more clearly which index values are valid. Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: polish doc, review comments - Changes: - all: https://git.openjdk.org/jdk/pull/14565/files - new: https://git.openjdk.org/jdk/pull/14565/files/24bf486f..f6a111b9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14565=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14565=00-01 Stats: 17 lines in 1 file changed: 5 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/14565.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14565/head:pull/14565 PR: https://git.openjdk.org/jdk/pull/14565
Re: RFR: 8308995: Update Network IO JFR events to be static mirror events
On Wed, 21 Jun 2023 16:59:22 GMT, Stuart Marks wrote: > Are we using a convention of `implRead` or `readImpl`? Either is ok with me, > but I think we had been using `readImpl` and similar elsewhere. This code is already using implXXX so it's just be consistent. - PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1237319486
Re: RFR: 8308995: Update Network IO JFR events to be static mirror events
On Wed, 21 Jun 2023 09:46:35 GMT, Alan Bateman wrote:
>> The socket read/write JFR events currently use instrumentation of java.base
>> code using templates in the jdk.jfr modules. This results in some java.base
>> code residing in the jdk.jfr module which is undesirable.
>>
>> JDK19 added static support for event classes. The old instrumentor classes
>> should be replaced with mirror events using the static support.
>>
>> In the java.base module:
>> Added two new events, jdk.internal.event.SocketReadEvent and
>> jdk.internal.event.SocketWriteEvent.
>> java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of
>> the new events.
>>
>> In the jdk.jfr module:
>> jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were
>> changed to be mirror events.
>> In the package jdk.jfr.internal.instrument, the classes
>> SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and
>> SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated
>> to reflect all of those changes.
>>
>> The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the
>> new implementation:
>> Passed: jdk/jfr/event/io/TestSocketChannelEvents.java
>> Passed: jdk/jfr/event/io/TestSocketEvents.java
>>
>> I added a micro benchmark which measures the overhead of handling the jfr
>> socket events.
>> test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java.
>> It needs access the jdk.internal.event package, which is done at runtime
>> with annotations that add the extra arguments.
>> At compile time the build arguments had to be augmented in
>> make/test/BuildMicrobenchmark.gmk
>
> src/java.base/share/classes/java/net/Socket.java line 1114:
>
>> 1112: }
>> 1113:
>> 1114: private int read0(byte[] b, int off, int len) throws
>> IOException {
>
> Can you rename this to implRead?
Are we using a convention of `implRead` or `readImpl`? Either is ok with me,
but I think we had been using `readImpl` and similar elsewhere.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1237316630
RFR: 8308995: Update Network IO JFR events to be static mirror events
The socket read/write JFR events currently use instrumentation of java.base code using templates in the jdk.jfr modules. This results in some java.base code residing in the jdk.jfr module which is undesirable. JDK19 added static support for event classes. The old instrumentor classes should be replaced with mirror events using the static support. In the java.base module: Added two new events, jdk.internal.event.SocketReadEvent and jdk.internal.event.SocketWriteEvent. java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of the new events. In the jdk.jfr module: jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were changed to be mirror events. In the package jdk.jfr.internal.instrument, the classes SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated to reflect all of those changes. The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the new implementation: Passed: jdk/jfr/event/io/TestSocketChannelEvents.java Passed: jdk/jfr/event/io/TestSocketEvents.java I added a micro benchmark which measures the overhead of handling the jfr socket events. test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java. It needs access the jdk.internal.event package, which is done at runtime with annotations that add the extra arguments. At compile time the build arguments had to be augmented in make/test/BuildMicrobenchmark.gmk - Commit messages: - some changes from review. - fix copyright date - Added micro benchmark to measure socket event overhead. - Some changes from review. - remove unnecessary cast - 8308995: Update Network IO JFR events to be static mirror events Changes: https://git.openjdk.org/jdk/pull/14342/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14342=00 Issue: https://bugs.openjdk.org/browse/JDK-8308995 Stats: 896 lines in 12 files changed: 523 ins; 367 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/14342.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14342/head:pull/14342 PR: https://git.openjdk.org/jdk/pull/14342
Re: RFR: 8308995: Update Network IO JFR events to be static mirror events
On Tue, 6 Jun 2023 19:39:31 GMT, Tim Prinzing wrote:
> The socket read/write JFR events currently use instrumentation of java.base
> code using templates in the jdk.jfr modules. This results in some java.base
> code residing in the jdk.jfr module which is undesirable.
>
> JDK19 added static support for event classes. The old instrumentor classes
> should be replaced with mirror events using the static support.
>
> In the java.base module:
> Added two new events, jdk.internal.event.SocketReadEvent and
> jdk.internal.event.SocketWriteEvent.
> java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of
> the new events.
>
> In the jdk.jfr module:
> jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were
> changed to be mirror events.
> In the package jdk.jfr.internal.instrument, the classes
> SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and
> SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated
> to reflect all of those changes.
>
> The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the new
> implementation:
> Passed: jdk/jfr/event/io/TestSocketChannelEvents.java
> Passed: jdk/jfr/event/io/TestSocketEvents.java
>
> I added a micro benchmark which measures the overhead of handling the jfr
> socket events.
> test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java.
> It needs access the jdk.internal.event package, which is done at runtime with
> annotations that add the extra arguments.
> At compile time the build arguments had to be augmented in
> make/test/BuildMicrobenchmark.gmk
src/java.base/share/classes/java/net/Socket.java line 43:
> 41: import java.util.Collections;
> 42:
> 43: import jdk.internal.event.SocketWriteEvent;
You'll probably want to clean up the imports to avoid having import of
jdk.internal classes in two places.
src/java.base/share/classes/java/net/Socket.java line 1109:
> 1107: nbytes = read0(b, off, len);
> 1108: } finally {
> 1109: SocketReadEvent.checkForCommit(start, nbytes,
> parent.getRemoteSocketAddress(), parent.getSoTimeout());
So if read throws, this will commit a jdk.SocketReadEvent with size 0, maybe
this will change later to include the exception?
src/java.base/share/classes/java/net/Socket.java line 1114:
> 1112: }
> 1113:
> 1114: private int read0(byte[] b, int off, int len) throws
> IOException {
Can you rename this to implRead?
src/java.base/share/classes/java/net/Socket.java line 1215:
> 1213: return;
> 1214: }
> 1215: int bytesWritten = 0;
Is bytesWritten needed?
src/java.base/share/classes/java/net/Socket.java line 1226:
> 1224: }
> 1225:
> 1226: public void write0(byte[] b, int off, int len) throws
> IOException {
Can you change this to be private and rename to implWrite?
src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java line 421:
> 419: }
> 420:
> 421: private int read0(ByteBuffer buf) throws IOException {
Can you rename this to implRead too?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1236731407
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1237226982
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1236719052
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1236719603
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1236719373
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1237227861
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v6]
On Wed, 21 Jun 2023 22:02:16 GMT, Mandy Chung wrote: >> This PR clarifies the spec of the 3-arg Class::forName regarding the format >> of the name for an array type which is of the form: one or more of "[" + >> binary name of the element type + ";'. > > Mandy Chung 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 eight additional > commits since the last revision: > > - improve the specification of forName > - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242 > - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242 > - Review comment. Add a test > - missing 'L' for the array class name > - review comment > - Clarify for an array class of primitive type > - 8310242: Clarify the name parameter to Class::forName I improved the spec of `forName` to make it clear of the name for nested types and array types. It now refers to the table in `getName` about the name of an array class. @dholmes-ora does that clarify and address the concerns you had? - PR Comment: https://git.openjdk.org/jdk/pull/14528#issuecomment-1601750566
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v6]
> This PR clarifies the spec of the 3-arg Class::forName regarding the format > of the name for an array type which is of the form: one or more of "[" + > binary name of the element type + ";'. Mandy Chung 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 eight additional commits since the last revision: - improve the specification of forName - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242 - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8310242 - Review comment. Add a test - missing 'L' for the array class name - review comment - Clarify for an array class of primitive type - 8310242: Clarify the name parameter to Class::forName - Changes: - all: https://git.openjdk.org/jdk/pull/14528/files - new: https://git.openjdk.org/jdk/pull/14528/files/a75542ff..39e218ea Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14528=05 - incr: https://webrevs.openjdk.org/?repo=jdk=14528=04-05 Stats: 9202 lines in 408 files changed: 4713 ins; 1982 del; 2507 mod Patch: https://git.openjdk.org/jdk/pull/14528.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14528/head:pull/14528 PR: https://git.openjdk.org/jdk/pull/14528
Re: RFR: JDK-8310550: Adjust references to rt.jar
On Wed, 21 Jun 2023 15:18:19 GMT, Matthias Baesken wrote: > There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. Mostly seems okay - a couple of things need further adjusting I think. Thanks. src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java line 196: > 194: > 195: /** > 196: * Set whether or not to use ct.sym as an alternate As an alternate to what? This needs something else. test/langtools/tools/javap/4798312/JavapShouldLoadClassesFromRTJarTest.java line 1: > 1: /* The name of this test includes RTJar. It needs to be changed too I think. Does this test actually still test something? - Changes requested by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14593#pullrequestreview-1491961660 PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1237747922 PR Review Comment: https://git.openjdk.org/jdk/pull/14593#discussion_r1237749197
Re: Integrated: 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all
On Wed, 21 Jun 2023 21:05:11 GMT, Mikael Vidstedt wrote: >> A trivial fix to ProblemList >> java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads >> on linux-all > > Marked as reviewed by mikael (Reviewer). @vidmik - Thanks for your review also. - PR Comment: https://git.openjdk.org/jdk/pull/14606#issuecomment-1601686797
Re: Integrated: 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all
On Wed, 21 Jun 2023 21:02:58 GMT, David Holmes wrote: >> A trivial fix to ProblemList >> java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads >> on linux-all > > Marked as reviewed by dholmes (Reviewer). @dholmes-ora - Thanks for the lightning fast review! - PR Comment: https://git.openjdk.org/jdk/pull/14606#issuecomment-1601676432
Integrated: 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all
On Wed, 21 Jun 2023 20:59:43 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads > on linux-all This pull request has now been integrated. Changeset: ac44ef19 Author:Daniel D. Daugherty URL: https://git.openjdk.org/jdk/commit/ac44ef19d5a129c41a8e89e667a28cff38acdd42 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all Reviewed-by: dholmes, mikael - PR: https://git.openjdk.org/jdk/pull/14606
Re: Integrated: 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all
On Wed, 21 Jun 2023 20:59:43 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads > on linux-all Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14606#pullrequestreview-1491873353
Re: Integrated: 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all
On Wed, 21 Jun 2023 20:59:43 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads > on linux-all Marked as reviewed by dholmes (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14606#pullrequestreview-1491869387
Integrated: 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all
A trivial fix to ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all - Commit messages: - 8310586: ProblemList java/lang/ScopedValue/StressStackOverflow.java#default with virtual threads on linux-all Changes: https://git.openjdk.org/jdk/pull/14606/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14606=00 Issue: https://bugs.openjdk.org/browse/JDK-8310586 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14606.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14606/head:pull/14606 PR: https://git.openjdk.org/jdk/pull/14606
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
`jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java` is failing due to
the new API in Platform.java.
-
PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1601655977
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
Copyrights need updating.
-
Changes requested by cjplummer (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1491822264
Re: RFR: 8310502 : optimization for UUID#toString [v4]
On Wed, 21 Jun 2023 19:02:59 GMT, ExE Boss wrote: >> src/java.base/share/classes/java/lang/Long.java line 563: >> >>> 561: StringUTF16.putChar(buf, 7, (byte) i3); >>> 562: StringUTF16.putChar(buf, 8, '-'); >>> 563: StringUTF16.putChar(buf, 9, (byte) (i4 >> 8)); >> >> This might be cheating but you could avoid a store of 0 to the high byte >> (its already zero) by inlining the code from StringUTF16.putChar(); just >> double the index on the store of the byte. >> >> buf[0] = (byte) (i0 >> 8); >> buf[2] = (byte) i0; >> buf[4] = (byte) (i1 >> 8); >> buf[6] = (byte) i1; >> buf[8] = (byte) (i2 >> 8); >> ... > > Note that `StringUTF16::putChar` takes endian‑ness into account, so the above > code would only be correct on half of the supported endian types. i have simplified the code using ENDIAN and COMPACT_STRINGS - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237641739
Re: RFR: 8310502 : optimization for UUID#toString [v6]
> By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: import ByteOrder - Changes: - all: https://git.openjdk.org/jdk/pull/14578/files - new: https://git.openjdk.org/jdk/pull/14578/files/c73f7ed1..ad53343f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14578=05 - incr: https://webrevs.openjdk.org/?repo=jdk=14578=04-05 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
Re: RFR: 8310502 : optimization for UUID#toString [v5]
> By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: simplify code - Changes: - all: https://git.openjdk.org/jdk/pull/14578/files - new: https://git.openjdk.org/jdk/pull/14578/files/a952e10c..c73f7ed1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14578=04 - incr: https://webrevs.openjdk.org/?repo=jdk=14578=03-04 Stats: 86 lines in 1 file changed: 7 ins; 38 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
Copyrights need updating.
test/lib/jdk/test/lib/Platform.java line 264:
> 262:
> 263:
> 264: public static boolean hasPlistEntriesOSX() throws IOException {
Almost all of this is replicated from `isHardenedOSX()`. I wonder if there is a
way to do some sharing while still maintaining separate APIs. Combining them
into one API might make it harder to understand the code. Maybe a
`launchCodesign()` API that returns the BufferedReader would help.
test/lib/jdk/test/lib/Platform.java line 288:
> 286: }
> 287: }
> 288: return false;
Probably would be good to log that no Info.plist entry was found.
test/lib/jdk/test/lib/util/CoreUtils.java line 131:
> 129: return coreFileLocation; // success!
> 130: } else {
> 131: System.out.println("Core file not found, try to find a
> reason for this");
Suggestion:
System.out.println("Core file not found. Trying to find a reason
why...");
-
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1491685114
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237602062
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237587271
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237590277
Re: RFR: 8310502 : optimization for UUID#toString [v4]
On Wed, 21 Jun 2023 14:39:18 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > add annotation Stable src/java.base/share/classes/java/lang/Long.java line 548: > 546: buf[33] = (byte) i14; > 547: buf[34] = (byte) (i15 >> 8); > 548: buf[35] = (byte) i15; You may be able to use `jdk.internal.util.ByteArray` to simplify code and improve performance: Suggestion: ByteArray.setChar(buf, 0, i0); ByteArray.setChar(buf, 2, i1); ByteArray.setChar(buf, 4, i2); ByteArray.setChar(buf, 6, i3); buf[8] = '-'; ByteArray.setChar(buf, 9, i4); ByteArray.setChar(buf, 11, i5); buf[13] = '-'; ByteArray.setChar(buf, 14, i6); ByteArray.setChar(buf, 16, i7); buf[18] = '-'; ByteArray.setChar(buf, 19, i8); ByteArray.setChar(buf, 21, i9); buf[23] = '-'; ByteArray.setChar(buf, 24, i10); ByteArray.setChar(buf, 26, i11); ByteArray.setChar(buf, 28, i12); ByteArray.setChar(buf, 30, i13); ByteArray.setChar(buf, 32, i14); ByteArray.setChar(buf, 34, i15); I measured the throughput of `UUID::toString()` using JMH on my linux server (CPU: R7-5800X): Benchmark Mode Cnt Score Error Units - UUIDUtilsBenchmark.test thrpt5 76117.237 ± 461.679 ops/ms + UUIDUtilsBenchmark.test thrpt5 87291.496 ± 729.527 ops/ms The result of this benchmark test is that using `ByteArray` can improve peak throughput by nearly 15%. Note: `ByteArray` uses `VarHandle` internally, and the impact on startup time is unknown, so more testing is needed. - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237573456
Re: RFR: 8310502 : optimization for UUID#toString [v4]
On Wed, 21 Jun 2023 14:53:22 GMT, Roger Riggs wrote: >> 温绍锦 has updated the pull request incrementally with one additional commit >> since the last revision: >> >> add annotation Stable > > src/java.base/share/classes/java/lang/Long.java line 563: > >> 561: StringUTF16.putChar(buf, 7, (byte) i3); >> 562: StringUTF16.putChar(buf, 8, '-'); >> 563: StringUTF16.putChar(buf, 9, (byte) (i4 >> 8)); > > This might be cheating but you could avoid a store of 0 to the high byte (its > already zero) by inlining the code from StringUTF16.putChar(); just double > the index on the store of the byte. > > buf[0] = (byte) (i0 >> 8); > buf[2] = (byte) i0; > buf[4] = (byte) (i1 >> 8); > buf[6] = (byte) i1; > buf[8] = (byte) (i2 >> 8); > ... Note that `StringUTF16::putChar` takes endian‑ness into account, so the above code would only be correct on half of the supported endian types. - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237483291
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Wed, 21 Jun 2023 13:00:38 GMT, Alan Bateman wrote: >> That is, a clear connection to what's being described by that `@implSpec`. > > This method has an existing apiNote and implSpec. I suspect Joe meant to add > this sentence to the apiNote, not the implSpec. I did mean to add the cross-reference to Objects.toIdentityString to the implSpec section since earlier in the implSpec section the expression that is the basis for Objects.toIdentityString is listed. - PR Review Comment: https://git.openjdk.org/jdk/pull/14567#discussion_r1237347781
Re: [jdk21] RFR: 8307858: [REDO] JDK-8307194 Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries
On Wed, 21 Jun 2023 17:09:58 GMT, Kevin Rushforth wrote: > Since this Enhancement was rejected for JDK 21, this PR should be closed. Closing without integration accordingly, thanks. - PR Comment: https://git.openjdk.org/jdk21/pull/26#issuecomment-1601273074
[jdk21] Withdrawn: 8307858: [REDO] JDK-8307194 Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries
On Fri, 16 Jun 2023 20:36:07 GMT, Jiangli Zhou wrote: > 8307858: [REDO] JDK-8307194 Add make target for optionally building a > complete set of all JDK and hotspot libjvm static libraries This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk21/pull/26
Re: [jdk21] RFR: 8307858: [REDO] JDK-8307194 Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries
On Fri, 16 Jun 2023 20:36:07 GMT, Jiangli Zhou wrote: > 8307858: [REDO] JDK-8307194 Add make target for optionally building a > complete set of all JDK and hotspot libjvm static libraries Since this Enhancement was rejected for JDK 21, this PR should be closed. - PR Comment: https://git.openjdk.org/jdk21/pull/26#issuecomment-1601253343
Re: RFR: JDK-8310550: Adjust references to rt.jar
On Wed, 21 Jun 2023 15:18:19 GMT, Matthias Baesken wrote: > There are a few references to rt.jar in comments and in the codebase itself. > Some of them might be removed or adjusted. The update to Java.gmk is good. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14593#pullrequestreview-1491263836
Re: RFR: 8309853: StructuredTaskScope.join description improvements
On Mon, 12 Jun 2023 14:32:07 GMT, Alan Bateman wrote: > StructuredTaskScope's class description introduces the join method as waiting > for all subtasks to finish but the API docs for join/joinUntil are phrased in > terms of waiting for all threads to finish. ShutdownOnXXX join/joinUntil > inherit this description but would be clearer if their description said they > wait until all subtasks finished or a subtask to succeed or fail. Marked as reviewed by darcy (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14419#pullrequestreview-1491208534
Re: RFR: 8310033: Clarify return value of Java Time compareTo methods [v3]
> In java.time packages, clarify timeline order javadoc to mention "before" and > "after" in the value of the `compareTo` method return values. > Add javadoc @see tags to isBefore and isAfter methods > > Replace use of "negative" and positive with "less than zero" and "greater > than zero" in javadoc @return > The term "positive" is ambiguous, zero is considered positive and indicates > equality. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Clarify return values of date time classes - Changes: - all: https://git.openjdk.org/jdk/pull/14479/files - new: https://git.openjdk.org/jdk/pull/14479/files/cd997ede..a0acac19 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14479=02 - incr: https://webrevs.openjdk.org/?repo=jdk=14479=01-02 Stats: 38 lines in 13 files changed: 6 ins; 5 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/14479.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14479/head:pull/14479 PR: https://git.openjdk.org/jdk/pull/14479
Re: RFR: 8310033: Clarify return value of Java Time compareTo methods [v2]
On Sun, 18 Jun 2023 20:24:07 GMT, Stephen Colebourne
wrote:
>> Roger Riggs 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 five additional
>> commits since the last revision:
>>
>> - Use {@code xxx} to highlight the comparison against the arg.
>>Update copyrights.
>> - Merge branch 'master' into 8310033-time-compareto
>> - Clarify for Duration, AbstractChronology, and Chronology
>> - Correct javadoc of compareInstant
>> - 8310033: Improve Instant.compareTo javadoc to mention before and after
>>Refine timeline order to mention before and after
>>Add javadoc @see tags to isBefore and isAfter methods
>
> src/java.base/share/classes/java/time/MonthDay.java line 678:
>
>> 676: *
>> 677: * @param other the other month-day to compare to, not null
>> 678: * @return the comparator value is less than zero if the {@code
>> other} is before,
>
> Using before/after here could be confusing, as January could be considered to
> be before or after July (since the year is not defined).
The `isBefore` and `isAfter` methods use that terminology and Month is an enum
with defined order January thru December and use the `compareTo` method to
compute before/after.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14479#discussion_r1237231956
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
I made a few review suggestions. Does the symptom happen on both, arm64 and x64?
test/lib/jdk/test/lib/Platform.java line 267:
> 265: // Find the path to the java binary.
> 266: String jdkPath = System.getProperty("java.home");
> 267: Path javaPath = Paths.get(jdkPath + "/bin/java");
You could do it more concisely:
Path javaPath = Paths.get(System.getProperty("java.home") + "/bin/java");
if (Files.notExists(javaPath)) {
throw new FileNotFoundException("Could not find file " +
javaPath.toAbsolutePath().toString());
test/lib/jdk/test/lib/Platform.java line 274:
> 272:
> 273: // Run codesign on the java binary.
> 274: ProcessBuilder pb = new ProcessBuilder("codesign", "--display",
> "--verbose", javaFileName);
And then here 'ProcessBuilder pb = new ProcessBuilder("codesign", "--display",
"--verbose", javaPath.toAbsolutePath().toString());`
test/lib/jdk/test/lib/Platform.java line 277:
> 275: pb.redirectErrorStream(true); // redirect stderr to stdout
> 276: Process codesignProcess = pb.start();
> 277: BufferedReader is = new BufferedReader(new
> InputStreamReader(codesignProcess.getInputStream()));
Maybe do the BufferedReader stuff in a try-with-resources and then return false
instead of potentially throwing an IOException?
test/lib/jdk/test/lib/Platform.java line 280:
> 278: String line;
> 279: while ((line = is.readLine()) != null) {
> 280: System.out.println("STDOUT: " + line);
This output is for every line seems too much. Maybe just print the lines where
you find "Info.plist=not bound" or "Info.plist entries="?
test/lib/jdk/test/lib/util/CoreUtils.java line 153:
> 151: throw new SkippedException("Cannot produce core file
> with hardened binary on OSX 10.15 and later");
> 152: }
> 153: } else {
Maybe you could do just one case here:
`else if (!Platform.hasPlistEntriesOSX()) {`...
-
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1491050995
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237190071
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237190832
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237191976
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237194492
PR Review Comment: https://git.openjdk.org/jdk/pull/14562#discussion_r1237184922
RFR: JDK-8310550: Adjust references to rt.jar
There are a few references to rt.jar in comments and in the codebase itself. Some of them might be removed or adjusted. - Commit messages: - JDK-8310550 Changes: https://git.openjdk.org/jdk/pull/14593/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14593=00 Issue: https://bugs.openjdk.org/browse/JDK-8310550 Stats: 14 lines in 12 files changed: 0 ins; 7 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/14593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14593/head:pull/14593 PR: https://git.openjdk.org/jdk/pull/14593
Re: RFR: 8310502 : optimization for UUID#toString [v4]
On Wed, 21 Jun 2023 14:31:20 GMT, Chen Liang wrote: >> Not sure if this can be applied but some months ago, I optimized Bits to use >> VarHandles rather than explicitly shifting bits around. This gave us a >> significant performance increase: >> >> https://github.com/openjdk/jdk/pull/11840/files > > The key to the optimization was MethodHandles.byteArrayViewVarHandle. Don't > know if StringUTF16.putChar and StringUTF16.getChar can benefit from such an > update. CDS can initialize arrays efficiently. The Long class already uses CDS to initialize the cache of Long values. See the LongCache code for an example of initializing from CDS with a fallback to direct initialization. You could move the HEX256 array to the LongCache holder class and avoid any performance hit on startup. - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237189484
Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work
On Tue, 20 Jun 2023 13:23:16 GMT, Matthias Baesken wrote:
> Currently, a number of tests fail on macOS because they miss the core file
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build.
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.
LGTM.
Thanks for enhancing test analysis.
-
Marked as reviewed by lucy (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1491050558
Re: RFR: 8310530: PipedOutputStream.flush() accesses sink racily [v2]
On Wed, 21 Jun 2023 14:27:26 GMT, Chen Liang wrote:
>> src/java.base/share/classes/java/io/PipedOutputStream.java line 166:
>>
>>> 164: @Override
>>> 165: public synchronized void flush() throws IOException {
>>> 166: PipedInputStream sink = this.sink;
>>
>> Suggestion:
>>
>> var sink = this.sink;
>>
>> As seen in other methods.
>
> On second thought, this is probably not necessary; write to the sink field is
> in another synchronized method, and this method is synchronized already. Is
> the goal here to remove the synchronized on flush?
Good observation. Removing `synchronized` on flush might be a worthwhile goal
but possible side effects (including on potential subclasses) should be
carefully considered.
I support stashing `sink` in a local variable though, even if the pointer can't
be concurrently modified, just to make it clear that we only have one volatile
read.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14589#discussion_r1237152608
Re: RFR: 8310530: PipedOutputStream.flush() accesses sink racily [v2]
> Just a tiny clean-up to remove racy read within synchronized method Sergey Tsypanov has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/io/PipedOutputStream.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/14589/files - new: https://git.openjdk.org/jdk/pull/14589/files/c97cdfc2..4c98ddce Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14589=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14589=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14589.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14589/head:pull/14589 PR: https://git.openjdk.org/jdk/pull/14589
[jdk21] Integrated: 8310053: VarHandle and slice handle derived from layout are lacking alignment check
On Wed, 21 Jun 2023 00:08:03 GMT, Jorn Vernee wrote: > Hi all, > > This pull request contains a backport of commit > [e022e876](https://github.com/openjdk/jdk/commit/e022e876543b65b531027662326f35b497861f33) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Jorn Vernee on 21 Jun 2023 and > was reviewed by Maurizio Cimadamore. > > Thanks! This pull request has now been integrated. Changeset: 3985a4d5 Author:Jorn Vernee URL: https://git.openjdk.org/jdk21/commit/3985a4d534a697d6ec0d61ac8cf80e5cbb55cf94 Stats: 97 lines in 4 files changed: 81 ins; 4 del; 12 mod 8310053: VarHandle and slice handle derived from layout are lacking alignment check Reviewed-by: mcimadamore Backport-of: e022e876543b65b531027662326f35b497861f33 - PR: https://git.openjdk.org/jdk21/pull/42
Re: RFR: 8310502 : optimization for UUID#toString [v4]
On Wed, 21 Jun 2023 14:39:18 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > add annotation Stable src/java.base/share/classes/java/lang/Long.java line 563: > 561: StringUTF16.putChar(buf, 7, (byte) i3); > 562: StringUTF16.putChar(buf, 8, '-'); > 563: StringUTF16.putChar(buf, 9, (byte) (i4 >> 8)); This might be cheating but you could avoid a store of 0 to the high byte (its already zero) by inlining the code from StringUTF16.putChar(); just double the index on the store of the byte. buf[0] = (byte) (i0 >> 8); buf[2] = (byte) i0; buf[4] = (byte) (i1 >> 8); buf[6] = (byte) i1; buf[8] = (byte) (i2 >> 8); ... - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237143688
Re: RFR: 8310502 : optimization for UUID#toString [v4]
> By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: add annotation Stable - Changes: - all: https://git.openjdk.org/jdk/pull/14578/files - new: https://git.openjdk.org/jdk/pull/14578/files/8bc49c9e..a952e10c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14578=03 - incr: https://webrevs.openjdk.org/?repo=jdk=14578=02-03 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
Re: RFR: 8310502 : optimization for UUID#toString [v4]
On Wed, 21 Jun 2023 14:13:34 GMT, Per Minborg wrote:
>>> > Another thing to try is moving fastUUID out of Long - moving to an array
>>> > of precomputed hex values means it is not tied to Long internals anymore.
>>>
>>> A note about `@Stable`: `Integer.digits` and `HEX256` are not, and they
>>> might see performance improvements and change the benchmark results if they
>>> are declared so.
>>
>> use HEX256 optimize Integer.toHexString
>> https://github.com/wenshao/jdk_8310502_test/blob/main/src/main/java/com/alibaba/openjdk/HexUtils.java
>>
>>char[] hex256 = UUIDUtils.HEX256;
>>
>> int i0 = (i >> 24) & 255;
>> int i1 = (i >> 16) & 255;
>> int i2 = (i >> 8) & 255;
>> int i3 = i & 255;
>>
>> char c0 = hex256[i0];
>> char c1 = hex256[i1];
>> char c2 = hex256[i2];
>> char c3 = hex256[i3];
>>
>> byte[] bytes;
>> if (COMPACT_STRINGS) {
>> if ((i >> 4) == 0) {
>> bytes = new byte[1];
>> bytes[0] = (byte) c3;
>> } else if ((i >> 8) == 0) {
>> bytes = new byte[2];
>> bytes[0] = (byte) (c3 >> 8);
>> bytes[1] = (byte) c3;
>> } else if ((i >> 12) == 0) {
>> bytes = new byte[3];
>> bytes[0] = (byte) c2;
>> bytes[1] = (byte) (c3 >> 8);
>> bytes[2] = (byte) c3;
>> } else if ((i >> 16) == 0) {
>> bytes = new byte[4];
>> bytes[0] = (byte) (c2 >> 8);
>> bytes[1] = (byte) c2;
>> bytes[2] = (byte) (c3 >> 8);
>> bytes[3] = (byte) c3;
>> } else if ((i >> 20) == 0) {
>> bytes = new byte[5];
>> bytes[0] = (byte) c1;
>> bytes[1] = (byte) (c2 >> 8);
>> bytes[2] = (byte) c2;
>> bytes[3] = (byte) (c3 >> 8);
>> bytes[4] = (byte) c3;
>> } else if ((i >> 24) == 0) {
>> bytes = new byte[6];
>> bytes[0] = (byte) (c1 >> 8);
>> bytes[1] = (byte) c1;
>> bytes[2] = (byte) (c2 >> 8);
>> bytes[3] = (byte) c2;
>> bytes[4] = (byte) (c3 >> 8);
>> bytes[5] = (byte) c3;
>> } else if ((i >> 28) == 0) {
>> bytes = new byte[7];
>> bytes[0] = (byte) c0;
>> bytes[1] = (byte) (c1 >> 8);
>> bytes[2] = (byte) c1;
>> bytes[3] = (byte) (c2 >> 8);
>> ...
>
> Not sure if this can be applied but some months ago, I optimized Bits to use
> VarHandles rather than explicitly shifting bits around. This gave us a
> significant performance increase:
>
> https://github.com/openjdk/jdk/pull/11840/files
The key to the optimization was MethodHandles.byteArrayViewVarHandle. Don't
know if StringUTF16.putChar and StringUTF16.getChar can benefit from such an
update.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237109343
Re: RFR: 8310530: PipedOutputStream.flush() accesses sink racily
On Wed, 21 Jun 2023 14:03:23 GMT, Chen Liang wrote:
>> Just a tiny clean-up to remove racy read within synchronized method
>
> src/java.base/share/classes/java/io/PipedOutputStream.java line 166:
>
>> 164: @Override
>> 165: public synchronized void flush() throws IOException {
>> 166: PipedInputStream sink = this.sink;
>
> Suggestion:
>
> var sink = this.sink;
>
> As seen in other methods.
On second thought, this is probably not necessary; write to the sink field is
in another synchronized method, and this method is synchronized already. Is the
goal here to remove the synchronized on flush?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14589#discussion_r1237103593
Re: RFR: 8310502 : optimization for UUID#toString [v3]
On Wed, 21 Jun 2023 09:48:54 GMT, 温绍锦 wrote:
>>> Another thing to try is moving fastUUID out of Long - moving to an array of
>>> precomputed hex values means it is not tied to Long internals anymore.
>>
>> A note about `@Stable`: `Integer.digits` and `HEX256` are not, and they
>> might see performance improvements and change the benchmark results if they
>> are declared so.
>
>> > Another thing to try is moving fastUUID out of Long - moving to an array
>> > of precomputed hex values means it is not tied to Long internals anymore.
>>
>> A note about `@Stable`: `Integer.digits` and `HEX256` are not, and they
>> might see performance improvements and change the benchmark results if they
>> are declared so.
>
> use HEX256 optimize Integer.toHexString
> https://github.com/wenshao/jdk_8310502_test/blob/main/src/main/java/com/alibaba/openjdk/HexUtils.java
>
>char[] hex256 = UUIDUtils.HEX256;
>
> int i0 = (i >> 24) & 255;
> int i1 = (i >> 16) & 255;
> int i2 = (i >> 8) & 255;
> int i3 = i & 255;
>
> char c0 = hex256[i0];
> char c1 = hex256[i1];
> char c2 = hex256[i2];
> char c3 = hex256[i3];
>
> byte[] bytes;
> if (COMPACT_STRINGS) {
> if ((i >> 4) == 0) {
> bytes = new byte[1];
> bytes[0] = (byte) c3;
> } else if ((i >> 8) == 0) {
> bytes = new byte[2];
> bytes[0] = (byte) (c3 >> 8);
> bytes[1] = (byte) c3;
> } else if ((i >> 12) == 0) {
> bytes = new byte[3];
> bytes[0] = (byte) c2;
> bytes[1] = (byte) (c3 >> 8);
> bytes[2] = (byte) c3;
> } else if ((i >> 16) == 0) {
> bytes = new byte[4];
> bytes[0] = (byte) (c2 >> 8);
> bytes[1] = (byte) c2;
> bytes[2] = (byte) (c3 >> 8);
> bytes[3] = (byte) c3;
> } else if ((i >> 20) == 0) {
> bytes = new byte[5];
> bytes[0] = (byte) c1;
> bytes[1] = (byte) (c2 >> 8);
> bytes[2] = (byte) c2;
> bytes[3] = (byte) (c3 >> 8);
> bytes[4] = (byte) c3;
> } else if ((i >> 24) == 0) {
> bytes = new byte[6];
> bytes[0] = (byte) (c1 >> 8);
> bytes[1] = (byte) c1;
> bytes[2] = (byte) (c2 >> 8);
> bytes[3] = (byte) c2;
> bytes[4] = (byte) (c3 >> 8);
> bytes[5] = (byte) c3;
> } else if ((i >> 28) == 0) {
> bytes = new byte[7];
> bytes[0] = (byte) c0;
> bytes[1] = (byte) (c1 >> 8);
> bytes[2] = (byte) c1;
> bytes[3] = (byte) (c2 >> 8);
> bytes[4] = (byte) c2;
> bytes[5] = (byte) (c3 >> 8);
> bytes[6] = (byte) c3;
> ...
Not sure if this can be applied but some months ago, I optimized Bits to use
VarHandles rather than explicitly shifting bits around. This gave us a
significant performance increase:
https://github.com/openjdk/jdk/pull/11840/files
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1237075217
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Tue, 20 Jun 2023 17:13:26 GMT, Joe Darcy wrote:
> I had occasion to read over the javadoc sources in java.lang.Object recently
> and noticed a few items that could be updated.
>
> There are some new or expanded API notes referring to methods in
> java.util.Objects. I added these references as apiNote items rather than,
> say, `@see` tags since `@see` tag changes would propagate into classes that
> overrode the methods in questions.
>
> Changing toString to use an inline `@return` tag has the consequence of
> omitting a trailing period, "." in the "Returns" section of its javadoc. This
> also omits a trailing period in subclasses that use the `@return` statement
> of Object.toString in their own toString method. Likewise for hashCode.
src/java.base/share/classes/java/lang/Object.java line 160:
> 158: * general contract for the {@code hashCode} method, which states
> 159: * that equal objects must have equal hash codes.
> 160: * The two-argument {@link java.util.Objects#equals(Object,
I recommend a blank line before to make the paragraph break more obvious.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14567#discussion_r1237051269
Re: [jdk21] RFR: 8310053: VarHandle and slice handle derived from layout are lacking alignment check
On Wed, 21 Jun 2023 00:08:03 GMT, Jorn Vernee wrote: > Hi all, > > This pull request contains a backport of commit > [e022e876](https://github.com/openjdk/jdk/commit/e022e876543b65b531027662326f35b497861f33) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Jorn Vernee on 21 Jun 2023 and > was reviewed by Maurizio Cimadamore. > > Thanks! Marked as reviewed by mcimadamore (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/42#pullrequestreview-1490837380
Re: RFR: 8310530: PipedOutputStream.flush() accesses sink racily
On Wed, 21 Jun 2023 14:01:33 GMT, Sergey Tsypanov wrote:
> Just a tiny clean-up to remove racy read within synchronized method
src/java.base/share/classes/java/io/PipedOutputStream.java line 166:
> 164: @Override
> 165: public synchronized void flush() throws IOException {
> 166: PipedInputStream sink = this.sink;
Suggestion:
var sink = this.sink;
As seen in other methods.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14589#discussion_r1237059920
RFR: 8310530: PipedOutputStream.flush() accesses sink racily
Just a tiny clean-up to remove racy read within synchronized method - Commit messages: - 8310530: PipedOutputStream.flush() accesses sink racily Changes: https://git.openjdk.org/jdk/pull/14589/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14589=00 Issue: https://bugs.openjdk.org/browse/JDK-8310530 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14589.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14589/head:pull/14589 PR: https://git.openjdk.org/jdk/pull/14589
Re: RFR: 8310502 : optimization for UUID#toString [v3]
On Wed, 21 Jun 2023 11:06:08 GMT, 温绍锦 wrote: >> By optimizing the implementation of java.lang.Long#fastUUID, the performance >> of the java.util.UUID#toString method can be significantly improved. >> >> The following are the test results of JMH: >> >> Benchmark Mode Cnt Score Error Units >> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms >> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > 8310502 : hex literal * benchmark code https://github.com/wenshao/jdk_8310502_test/blob/main/src/main/java/com/alibaba/openjdk/UUIDUtilsBenchmark.java * benchmark result Benchmark Mode Cnt ScoreError Units HexUtilsBenchmark.fast thrpt5 2971.864 ± 17.277 ops/ms HexUtilsBenchmark.jdkthrpt5 1984.653 ± 11.733 ops/ms - PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1600843797
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Wed, 21 Jun 2023 11:49:54 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/java/lang/Object.java line 260:
>>
>>> 258: * }
>>> 259: * The {@link java.util.Objects#toIdentityString(Object)
>>> 260: * Objects.toIdentityString} method returns the string for an
>>
>> That last sentence doesn't feel like it needs to be part of `@implSpec`,
>> although there's definitely a connection.
>
> That is, a clear connection to what's being described by that `@implSpec`.
This method has an existing apiNote and implSpec. I suspect Joe meant to add
this sentence to the apiNote, not the implSpec.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14567#discussion_r1236963868
Re: RFR: JDK-8310267: Javadoc for Class#isPrimitive() is incorrect regarding Class objects for primitives
On Wed, 21 Jun 2023 00:00:54 GMT, Joe Darcy wrote:
> Correct misstatement that the Class object for a primitive type can only be
> be access via fields like java.lang.Integer.TYPE.
src/java.base/share/classes/java/lang/Class.java line 823:
> 821: *
> 822: * @apiNote
> 823: * These {@code Class} objects can be accessed via the {@code
It might be clearer if the API note starts with something like "A Class object
representing a primitive type" rather than "These Class objects".
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14574#discussion_r1236929019
Re: RFR: 8308780: Fix the Java Integer types on Windows [v4]
On Thu, 1 Jun 2023 11:49:24 GMT, Julian Waters wrote: >> On Windows, the basic Java Integer types are defined as long and __int64 >> respectively. In particular, the former is rather problematic since it >> breaks compilation as the Visual C++ becomes stricter and more compliant >> with every release, which means the way Windows code treats long as a >> typedef for int is no longer correct, especially with -permissive- enabled. >> Instead of changing every piece of broken code to match the jint = long >> typedef, which is far too time consuming, we can instead change jint to an >> int (which is still the same 32 bit number type as long), as there are far >> fewer problems caused by this definition. It's better to get this over and >> done with sooner than later when a future version of Visual C++ finally >> starts to break on existing code > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Fix the code that is actually warning Yeah, those were code cleanups I thought I could do out of convenience, I'll revert them all before this goes in - PR Comment: https://git.openjdk.org/jdk/pull/14125#issuecomment-1600719023
Re: RFR: 8308780: Fix the Java Integer types on Windows [v4]
On Thu, 1 Jun 2023 11:49:24 GMT, Julian Waters wrote: >> On Windows, the basic Java Integer types are defined as long and __int64 >> respectively. In particular, the former is rather problematic since it >> breaks compilation as the Visual C++ becomes stricter and more compliant >> with every release, which means the way Windows code treats long as a >> typedef for int is no longer correct, especially with -permissive- enabled. >> Instead of changing every piece of broken code to match the jint = long >> typedef, which is far too time consuming, we can instead change jint to an >> int (which is still the same 32 bit number type as long), as there are far >> fewer problems caused by this definition. It's better to get this over and >> done with sooner than later when a future version of Visual C++ finally >> starts to break on existing code > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Fix the code that is actually warning Compilation should be a good enough test for the `long` -> `jint` changes. These changes are supposed to address [this difference](https://learn.microsoft.com/en-us/cpp/overview/cpp-conformance-improvements-2019?view=msvc-170#overload-resolution-involving-integral-overloads-and-long-arguments) between MSVC behavior and the C++ standard. When compiled with `-permissive-` or with a compiler that puts more emphasis on standards conformance (like clang), the current code fails to compile. I verified some of the generated binaries by comparing the results of `dumpbin /all` before and after the change. Most of the time the changes were limited to timestamp, UUID and mangled function names. `Jaccesswalker.exe` had a few more changes because of a changed format string. None of the changed function names in client libs area are externally visible, but there are some observable changes to `c2v` functions exported from jvm.dll. I had to revert some of the `NULL`->`nullptr` changes to get this to compile; I assume this will be addressed before this PR is merged. Judging by the PR title, these changes don't belong here anyway. - PR Comment: https://git.openjdk.org/jdk/pull/14125#issuecomment-1600710570
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Tue, 20 Jun 2023 17:13:26 GMT, Joe Darcy wrote: > I had occasion to read over the javadoc sources in java.lang.Object recently > and noticed a few items that could be updated. > > There are some new or expanded API notes referring to methods in > java.util.Objects. I added these references as apiNote items rather than, > say, `@see` tags since `@see` tag changes would propagate into classes that > overrode the methods in questions. > > Changing toString to use an inline `@return` tag has the consequence of > omitting a trailing period, "." in the "Returns" section of its javadoc. This > also omits a trailing period in subclasses that use the `@return` statement > of Object.toString in their own toString method. Likewise for hashCode. While it's out of scope of this PR, I note that we could also use the inline variant of `@return` in those methods of `Objects` that the `Object` refers to: `hash(Object)`, `hashCode(Object...)`, and `equals(Object, Object)`. In fact, I think they are crying for it. - PR Comment: https://git.openjdk.org/jdk/pull/14567#issuecomment-1600698509
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Wed, 21 Jun 2023 11:39:43 GMT, Pavel Rappo wrote:
>> I had occasion to read over the javadoc sources in java.lang.Object recently
>> and noticed a few items that could be updated.
>>
>> There are some new or expanded API notes referring to methods in
>> java.util.Objects. I added these references as apiNote items rather than,
>> say, `@see` tags since `@see` tag changes would propagate into classes that
>> overrode the methods in questions.
>>
>> Changing toString to use an inline `@return` tag has the consequence of
>> omitting a trailing period, "." in the "Returns" section of its javadoc.
>> This also omits a trailing period in subclasses that use the `@return`
>> statement of Object.toString in their own toString method. Likewise for
>> hashCode.
>
> src/java.base/share/classes/java/lang/Object.java line 260:
>
>> 258: * }
>> 259: * The {@link java.util.Objects#toIdentityString(Object)
>> 260: * Objects.toIdentityString} method returns the string for an
>
> That last sentence doesn't feel like it needs to be part of `@implSpec`,
> although there's definitely a connection.
That is, a clear connection to what's being described by that `@implSpec`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14567#discussion_r1236873033
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Tue, 20 Jun 2023 17:13:26 GMT, Joe Darcy wrote:
> I had occasion to read over the javadoc sources in java.lang.Object recently
> and noticed a few items that could be updated.
>
> There are some new or expanded API notes referring to methods in
> java.util.Objects. I added these references as apiNote items rather than,
> say, `@see` tags since `@see` tag changes would propagate into classes that
> overrode the methods in questions.
>
> Changing toString to use an inline `@return` tag has the consequence of
> omitting a trailing period, "." in the "Returns" section of its javadoc. This
> also omits a trailing period in subclasses that use the `@return` statement
> of Object.toString in their own toString method. Likewise for hashCode.
src/java.base/share/classes/java/lang/Object.java line 260:
> 258: * }
> 259: * The {@link java.util.Objects#toIdentityString(Object)
> 260: * Objects.toIdentityString} method returns the string for an
Separately, is it just me or this sentence could be rephrased to not read like
`toIdentityString(Object)` accepts an object equal to the string (that would
...)?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14567#discussion_r1236870099
Re: RFR: JDK-8310453: Update javadoc of java.lang.Object
On Tue, 20 Jun 2023 17:13:26 GMT, Joe Darcy wrote:
> I had occasion to read over the javadoc sources in java.lang.Object recently
> and noticed a few items that could be updated.
>
> There are some new or expanded API notes referring to methods in
> java.util.Objects. I added these references as apiNote items rather than,
> say, `@see` tags since `@see` tag changes would propagate into classes that
> overrode the methods in questions.
>
> Changing toString to use an inline `@return` tag has the consequence of
> omitting a trailing period, "." in the "Returns" section of its javadoc. This
> also omits a trailing period in subclasses that use the `@return` statement
> of Object.toString in their own toString method. Likewise for hashCode.
src/java.base/share/classes/java/lang/Object.java line 260:
> 258: * }
> 259: * The {@link java.util.Objects#toIdentityString(Object)
> 260: * Objects.toIdentityString} method returns the string for an
That last sentence doesn't feel like it needs to be part of `@implSpec`,
although there's definitely a connection.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14567#discussion_r1236859323
Re: RFR: 8310502 : optimization for UUID#toString [v3]
> By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: 8310502 : hex literal - Changes: - all: https://git.openjdk.org/jdk/pull/14578/files - new: https://git.openjdk.org/jdk/pull/14578/files/35af153a..8bc49c9e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14578=02 - incr: https://webrevs.openjdk.org/?repo=jdk=14578=01-02 Stats: 47 lines in 1 file changed: 0 ins; 0 del; 47 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
Re: RFR: 8310502 : optimization for UUID#toString [v2]
On Wed, 21 Jun 2023 10:11:09 GMT, 温绍锦 wrote:
>> By optimizing the implementation of java.lang.Long#fastUUID, the performance
>> of the java.util.UUID#toString method can be significantly improved.
>>
>> The following are the test results of JMH:
>>
>> Benchmark Mode Cnt Score Error Units
>> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms
>> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit
> since the last revision:
>
> 8310502 : HEX256 initialization use constant values improved jvm startup
src/java.base/share/classes/java/lang/Long.java line 128:
> 126: 26209, 26210, 26211, 26212, 26213, 26214
> 127: };
> 128:
Can we perhaps have hex literals here? I think they are easier to visually
check than decimals.
static final char[] HEX256 = new char[]{
0x3030, 0x3031, 0x3032, 0x3033, 0x3034, 0x3035, 0x3036, 0x3037, ...
or
static final char[] HEX256 = new char[]{
0x30_30, 0x30_31, 0x30_32, 0x30_33, 0x30_34, 0x30_35, 0x30_36, 0x30_37,
...
src/java.base/share/classes/java/lang/Long.java line 492:
> 490: final char[] hex256 = HEX256;
> 491:
> 492: char i = hex256[((int) (msb >> 56)) & 255];
Masks are easier to read when expressed as hex literals.
char i = hex256[((int) (msb >> 56)) & 0xFF];
...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1236753357
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1236753461
Re: RFR: 8310502 : optimization for UUID#toString [v2]
> By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: 8310502 : HEX256 initialization use constant values improved jvm startup - Changes: - all: https://git.openjdk.org/jdk/pull/14578/files - new: https://git.openjdk.org/jdk/pull/14578/files/14592810..35af153a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14578=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14578=00-01 Stats: 40 lines in 1 file changed: 30 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v4]
On Wed, 21 Jun 2023 09:54:53 GMT, Alan Bateman wrote: >> Binary name is a long-standing behavior. It's a spec bug. > >> Binary name is a long-standing behavior. It's a spec bug. > > Yes, I view this PR as just specifying long standing behavior. The name used > for array class may be surprising but I don't think it can be changed. Such array names and binary name for nested/inner classes have always been returned by `Class::getName` as well, so they shouldn't be too surprising considering the old specification mentions it reuses the `getName` format. - PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1236736162
Re: RFR: 8310242: Clarify the name parameter to Class::forName [v4]
On Tue, 20 Jun 2023 17:45:03 GMT, Mandy Chung wrote: > Binary name is a long-standing behavior. It's a spec bug. Yes, I view this PR as just specifying long standing behavior. The name used for array class may be surprising but I don't think it can be changed. - PR Review Comment: https://git.openjdk.org/jdk/pull/14528#discussion_r1236728781
Re: RFR: 8310502 : optimization for UUID#toString
On Wed, 21 Jun 2023 08:58:53 GMT, Chen Liang wrote:
> > Another thing to try is moving fastUUID out of Long - moving to an array of
> > precomputed hex values means it is not tied to Long internals anymore.
>
> A note about `@Stable`: `Integer.digits` and `HEX256` are not, and they might
> see performance improvements and change the benchmark results if they are
> declared so.
use HEX256 optimize Integer.toHexString
https://github.com/wenshao/jdk_8310502_test/blob/main/src/main/java/com/alibaba/openjdk/HexUtils.java
char[] hex256 = UUIDUtils.HEX256;
int i0 = (i >> 24) & 255;
int i1 = (i >> 16) & 255;
int i2 = (i >> 8) & 255;
int i3 = i & 255;
char c0 = hex256[i0];
char c1 = hex256[i1];
char c2 = hex256[i2];
char c3 = hex256[i3];
byte[] bytes;
if (COMPACT_STRINGS) {
if ((i >> 4) == 0) {
bytes = new byte[1];
bytes[0] = (byte) c3;
} else if ((i >> 8) == 0) {
bytes = new byte[2];
bytes[0] = (byte) (c3 >> 8);
bytes[1] = (byte) c3;
} else if ((i >> 12) == 0) {
bytes = new byte[3];
bytes[0] = (byte) c2;
bytes[1] = (byte) (c3 >> 8);
bytes[2] = (byte) c3;
} else if ((i >> 16) == 0) {
bytes = new byte[4];
bytes[0] = (byte) (c2 >> 8);
bytes[1] = (byte) c2;
bytes[2] = (byte) (c3 >> 8);
bytes[3] = (byte) c3;
} else if ((i >> 20) == 0) {
bytes = new byte[5];
bytes[0] = (byte) c1;
bytes[1] = (byte) (c2 >> 8);
bytes[2] = (byte) c2;
bytes[3] = (byte) (c3 >> 8);
bytes[4] = (byte) c3;
} else if ((i >> 24) == 0) {
bytes = new byte[6];
bytes[0] = (byte) (c1 >> 8);
bytes[1] = (byte) c1;
bytes[2] = (byte) (c2 >> 8);
bytes[3] = (byte) c2;
bytes[4] = (byte) (c3 >> 8);
bytes[5] = (byte) c3;
} else if ((i >> 28) == 0) {
bytes = new byte[7];
bytes[0] = (byte) c0;
bytes[1] = (byte) (c1 >> 8);
bytes[2] = (byte) c1;
bytes[3] = (byte) (c2 >> 8);
bytes[4] = (byte) c2;
bytes[5] = (byte) (c3 >> 8);
bytes[6] = (byte) c3;
} else {
bytes = new byte[8];
bytes[0] = (byte) (c0 >> 8);
bytes[1] = (byte) c0;
bytes[2] = (byte) (c1 >> 8);
bytes[3] = (byte) c1;
bytes[4] = (byte) (c2 >> 8);
bytes[5] = (byte) c2;
bytes[6] = (byte) (c3 >> 8);
bytes[7] = (byte) c3;
}
return new Stringbytes, LATIN1);
}
//
* benchmark code
https://github.com/wenshao/jdk_8310502_test/blob/main/src/main/java/com/alibaba/openjdk/HexUtilsBenchmark.java
* benchmark result
BenchmarkMode Cnt ScoreError Units
HexUtilsBenchmark.fast thrpt5 2910.332 ± 20.051 ops/ms
HexUtilsBenchmark.jdk thrpt5 1966.712 ± 48.141 ops/ms
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1236721668
Re: RFR: 8308899: Introduce Classfile.Context and improve Classfile.Option(s) [v22]
> Classfile context object and multi-state options have been discussed at > https://mail.openjdk.org/pipermail/classfile-api-dev/2023-May/000321.html > This patch implements the proposed changes in Classfile API and fixes all > affected code across JDK sources and tests. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: Apply suggestions from code review Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/14180/files - new: https://git.openjdk.org/jdk/pull/14180/files/baa01584..4017e28b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14180=21 - incr: https://webrevs.openjdk.org/?repo=jdk=14180=20-21 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/14180.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14180/head:pull/14180 PR: https://git.openjdk.org/jdk/pull/14180
Re: RFR: 8310502 : optimization for UUID#toString
On Wed, 21 Jun 2023 08:40:43 GMT, 温绍锦 wrote:
>> src/java.base/share/classes/java/lang/Long.java line 97:
>>
>>> 95: + (lo < 10 ? '0' + lo : 'a' + lo - 10));
>>> 96: }
>>> 97: }
>>
>> Are you checking the impact on startup? I guess I would have expected to see
>> it loaded from the CDS archive. Given that the usage is just UUID::toString
>> then maybe you could move this to a holder class containing a Stable array,
>> at least until there is a better way to have lazily computed constants.
>> Another thing to try is moving fastUUID out of Long - moving to an array of
>> precomputed hex values means it is not tied to Long internals anymore.
>
> 1. HEX256 can be used to optimize all integers to hex strings, such as
> Integer#toHexString and Long#toHexString and byte [] to hexstring.
>
> 2. i am a newcomer, and i don't understand what 'loaded from the CDS archive'
> means, do you mean to use constants instead? such as
>
> HEX256 = new char[]{
> 12336, 12337, 12338, 12339, 12340, 12341, 12342, 12343, 12344, 12345,
> 12385, 12386, 12387, 12388, 12389, 12390, 12592, 12593, 12594, 12595,
> 12596, 12597, 12598, 12599, 12600, 12601, 12641, 12642, 12643, 12644,
> 12645, 12646, 12848, 12849, 12850, 12851, 12852, 12853, 12854, 12855,
> 12856, 12857, 12897, 12898, 12899, 12900, 12901, 12902, 13104, 13105,
> 13106, 13107, 13108, 13109, 13110, 13111, 13112, 13113, 13153, 13154,
> 13155, 13156, 13157, 13158, 13360, 13361, 13362, 13363, 13364, 13365,
> 13366, 13367, 13368, 13369, 13409, 13410, 13411, 13412, 13413, 13414,
> 13616, 13617, 13618, 13619, 13620, 13621, 13622, 13623, 13624, 13625,
> 13665, 13666, 13667, 13668, 13669, 13670, 13872, 13873, 13874, 13875,
> 13876, 13877, 13878, 13879, 13880, 13881, 13921, 13922, 13923, 13924,
> 13925, 13926, 14128, 14129, 14130, 14131, 14132, 14133, 14134, 14135,
> 14136, 14137, 14177, 14178, 14179, 14180, 14181, 14182, 14384, 14385,
> 14386, 14387, 14388, 14389, 14390, 14391, 14392, 14393, 14433, 14434,
> 14435, 14436, 14437, 14438, 14640, 14641, 14642, 14643, 14644, 14645,
> 14646, 14647, 14648, 14649, 14689, 14690, 14691, 14692, 14693, 14694,
> 24880, 24881, 24882, 24883, 24884, 24885, 24886, 24887, 24888, 24889,
> 24929, 24930, 24931, 24932, 24933, 24934, 25136, 25137, 25138, 25139,
> 25140, 25141, 25142, 25143, 25144, 25145, 25185, 25186, 25187, 25188,
> 25189, 25190, 25392, 25393, 25394, 25395, 25396, 25397, 25398, 25399,
> 25400, 25401, 25441, 25442, 25443, 25444, 25445, 25446, 25648, 25649,
> 25650, 25651, 25652, 25653, 25654, 25655, 25656, 25657, 25697, 25698,
> 25699, 25700, 25701, 25702, 25904, 25905, 25906, 25907, 25908, 25909,
> 25910, 25911, 25912, 25913, 25953, 25954, 25955, 25956, 25957, 25958,
> 26160, 26161, 26162, 26163, 26164, 26165, 26166, 26167, 26168, 26169,
> 26209, 26210, 26211, 26212, 26213, 26214
> };
> Another thing to try is moving fastUUID out of Long - moving to an array of
> precomputed hex values means it is not tied to Long internals anymore.
A note about `@Stable`: `Integer.digits` and `HEX256` are not, and they might
see performance improvements and change the benchmark results if they are
declared so.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1236656177
Re: RFR: 8310502 : optimization for UUID#toString
On Wed, 21 Jun 2023 07:58:11 GMT, Alan Bateman wrote:
>> By optimizing the implementation of java.lang.Long#fastUUID, the performance
>> of the java.util.UUID#toString method can be significantly improved.
>>
>> The following are the test results of JMH:
>>
>> Benchmark Mode Cnt Score Error Units
>> UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms
>> UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms
>
> src/java.base/share/classes/java/lang/Long.java line 97:
>
>> 95: + (lo < 10 ? '0' + lo : 'a' + lo - 10));
>> 96: }
>> 97: }
>
> Are you checking the impact on startup? I guess I would have expected to see
> it loaded from the CDS archive. Given that the usage is just UUID::toString
> then maybe you could move this to a holder class containing a Stable array,
> at least until there is a better way to have lazily computed constants.
> Another thing to try is moving fastUUID out of Long - moving to an array of
> precomputed hex values means it is not tied to Long internals anymore.
1. HEX256 can be used to optimize all integers to hex strings, such as
Integer#toHexString and Long#toHexString and byte [] to hexstring.
2. i am a newcomer, and i don't understand what 'loaded from the CDS archive'
means, do you mean to use constants instead? such as
HEX256 = new char[]{
12336, 12337, 12338, 12339, 12340, 12341, 12342, 12343, 12344, 12345,
12385, 12386, 12387, 12388, 12389, 12390, 12592, 12593, 12594, 12595,
12596, 12597, 12598, 12599, 12600, 12601, 12641, 12642, 12643, 12644,
12645, 12646, 12848, 12849, 12850, 12851, 12852, 12853, 12854, 12855,
12856, 12857, 12897, 12898, 12899, 12900, 12901, 12902, 13104, 13105,
13106, 13107, 13108, 13109, 13110, 13111, 13112, 13113, 13153, 13154,
13155, 13156, 13157, 13158, 13360, 13361, 13362, 13363, 13364, 13365,
13366, 13367, 13368, 13369, 13409, 13410, 13411, 13412, 13413, 13414,
13616, 13617, 13618, 13619, 13620, 13621, 13622, 13623, 13624, 13625,
13665, 13666, 13667, 13668, 13669, 13670, 13872, 13873, 13874, 13875,
13876, 13877, 13878, 13879, 13880, 13881, 13921, 13922, 13923, 13924,
13925, 13926, 14128, 14129, 14130, 14131, 14132, 14133, 14134, 14135,
14136, 14137, 14177, 14178, 14179, 14180, 14181, 14182, 14384, 14385,
14386, 14387, 14388, 14389, 14390, 14391, 14392, 14393, 14433, 14434,
14435, 14436, 14437, 14438, 14640, 14641, 14642, 14643, 14644, 14645,
14646, 14647, 14648, 14649, 14689, 14690, 14691, 14692, 14693, 14694,
24880, 24881, 24882, 24883, 24884, 24885, 24886, 24887, 24888, 24889,
24929, 24930, 24931, 24932, 24933, 24934, 25136, 25137, 25138, 25139,
25140, 25141, 25142, 25143, 25144, 25145, 25185, 25186, 25187, 25188,
25189, 25190, 25392, 25393, 25394, 25395, 25396, 25397, 25398, 25399,
25400, 25401, 25441, 25442, 25443, 25444, 25445, 25446, 25648, 25649,
25650, 25651, 25652, 25653, 25654, 25655, 25656, 25657, 25697, 25698,
25699, 25700, 25701, 25702, 25904, 25905, 25906, 25907, 25908, 25909,
25910, 25911, 25912, 25913, 25953, 25954, 25955, 25956, 25957, 25958,
26160, 26161, 26162, 26163, 26164, 26165, 26166, 26167, 26168, 26169,
26209, 26210, 26211, 26212, 26213, 26214
};
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1236632961
[jdk21] RFR: 8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX
8310191: com/sun/tools/attach/warnings/DynamicLoadWarningTest.java second failure on AIX - Commit messages: - Backport 6a63badd8ea3e79cd9fc3cb33aff499fc9a6d3f1 Changes: https://git.openjdk.org/jdk21/pull/45/files Webrev: https://webrevs.openjdk.org/?repo=jdk21=45=00 Issue: https://bugs.openjdk.org/browse/JDK-8310191 Stats: 20 lines in 1 file changed: 8 ins; 8 del; 4 mod Patch: https://git.openjdk.org/jdk21/pull/45.diff Fetch: git fetch https://git.openjdk.org/jdk21.git pull/45/head:pull/45 PR: https://git.openjdk.org/jdk21/pull/45
Re: RFR: 8308780: Fix the Java Integer types on Windows [v4]
On Thu, 1 Jun 2023 11:49:24 GMT, Julian Waters wrote: >> On Windows, the basic Java Integer types are defined as long and __int64 >> respectively. In particular, the former is rather problematic since it >> breaks compilation as the Visual C++ becomes stricter and more compliant >> with every release, which means the way Windows code treats long as a >> typedef for int is no longer correct, especially with -permissive- enabled. >> Instead of changing every piece of broken code to match the jint = long >> typedef, which is far too time consuming, we can instead change jint to an >> int (which is still the same 32 bit number type as long), as there are far >> fewer problems caused by this definition. It's better to get this over and >> done with sooner than later when a future version of Visual C++ finally >> starts to break on existing code > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Fix the code that is actually warning ... - PR Comment: https://git.openjdk.org/jdk/pull/14125#issuecomment-1600400436
[jdk21] Integrated: 8309937: Add @sealedGraph for some Panama FFM interfaces
On Mon, 19 Jun 2023 14:58:13 GMT, Per Minborg wrote: > Hi all, > > This pull request contains a backport of commit > [b412fc79](https://github.com/openjdk/jdk/commit/b412fc79c3c2548df10918090beedaf6b2d08d96) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Per Minborg on 16 Jun 2023 and > was reviewed by Maurizio Cimadamore. > > Thanks! This pull request has now been integrated. Changeset: ef0357f6 Author:Per Minborg URL: https://git.openjdk.org/jdk21/commit/ef0357f6cbb23f22a60266206f8937f1ad23d85c Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8309937: Add @sealedGraph for some Panama FFM interfaces Reviewed-by: mcimadamore Backport-of: b412fc79c3c2548df10918090beedaf6b2d08d96 - PR: https://git.openjdk.org/jdk21/pull/34
Re: RFR: 8308899: Introduce Classfile.Context and improve Classfile.Option(s) [v21]
On Thu, 15 Jun 2023 17:22:32 GMT, Adam Sotona wrote:
>> Classfile context object and multi-state options have been discussed at
>> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-May/000321.html
>> This patch implements the proposed changes in Classfile API and fixes all
>> affected code across JDK sources and tests.
>>
>> Please review.
>>
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional
> commit since the last revision:
>
> ClassfileBenchmark::transformWithNewMaps changed to transformWithAddedNOP
src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 57:
> 55:
> 56: /**
> 57: * {@return a new context with default options}
Suggestion:
* {@return a context with default options}
src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 272:
> 270: /**
> 271: * Build a classfile into a file using the provided constant pool
> 272: * builder (which encapsulates classfile processing options.)
The CP is now simply a data holder; it no longer holds the processing options.
Same update is needed for the other few build methods taking a CP instance.
Suggestion:
* builder.
src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 340:
> 338: *
> 339: * @implNote
> 340: * This method behaves as if:
Suggestion:
* This method behaves as if:
Redundant break
src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 343:
> 341: * {@snippet lang=java :
> 342: * Classfile.of().build(thisClass(),
> ConstantPoolBuilder.of(this),
> 343: * b -> b.transform(this, transform));
Suggestion:
* this.build(model.thisClass(), ConstantPoolBuilder.of(model),
* b -> b.transform(model, transform));
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1236575941
PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1236572387
PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1236548304
PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1236570476
Re: [jdk21] RFR: 8309937: Add @sealedGraph for some Panama FFM interfaces
On Mon, 19 Jun 2023 14:58:13 GMT, Per Minborg wrote: > Hi all, > > This pull request contains a backport of commit > [b412fc79](https://github.com/openjdk/jdk/commit/b412fc79c3c2548df10918090beedaf6b2d08d96) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Per Minborg on 16 Jun 2023 and > was reviewed by Maurizio Cimadamore. > > Thanks! Marked as reviewed by mcimadamore (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/34#pullrequestreview-1490063531
Re: RFR: 8310502 : optimization for UUID#toString
On Wed, 21 Jun 2023 07:28:54 GMT, 温绍锦 wrote: > By optimizing the implementation of java.lang.Long#fastUUID, the performance > of the java.util.UUID#toString method can be significantly improved. > > The following are the test results of JMH: > > Benchmark Mode Cnt Score Error Units > UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms > UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms src/java.base/share/classes/java/lang/Long.java line 97: > 95: + (lo < 10 ? '0' + lo : 'a' + lo - 10)); > 96: } > 97: } Are you checking the impact on startup? I guess I would have expected to see it loaded from the CDS archive. Given that the usage is just UUID::toString then maybe you could move this to a holder class containing a Stable array, at least until there is a better way to have lazily computed constants. Another thing to try is moving fastUUID out of Long - moving to an array of precomputed hex values means it is not tied to Long internals anymore. - PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1236556426
Re: RFR: JDK-8310265: (process) jspawnhelper should not use argv[0] [v2]
On Wed, 21 Jun 2023 07:55:31 GMT, David Holmes wrote: >> I'm not aware of any other uses either; its not intended to be used outside >> of ProcessImpl especially since the addition of the new protocol to >> communicate parameters and confirm launching of the child. > > This curiosity has been present since Rob's first version: > https://mail.openjdk.org/pipermail/core-libs-dev/2012-November/012417.html I assumed that at some point in its life, maybe just in the first unpublished versions, the jspawnhelper had a variable number of arguments, possibly from different callers. But the fd-string was always supposed to be the last one. Then this would have made perfect sense. - PR Review Comment: https://git.openjdk.org/jdk/pull/14531#discussion_r1236569635
Re: RFR: JDK-8310265: (process) jspawnhelper should not use argv[0] [v2]
On Tue, 20 Jun 2023 16:27:38 GMT, Roger Riggs wrote: >> I wondered about this, it looked so deliberate. So I wondered whether >> jspawnhelper is used outside of the posix_spawn context, but cannot find >> anything. Maybe historic? > > I'm not aware of any other uses either; its not intended to be used outside > of ProcessImpl especially since the addition of the new protocol to > communicate parameters and confirm launching of the child. This curiosity has been present since Rob's first version: https://mail.openjdk.org/pipermail/core-libs-dev/2012-November/012417.html - PR Review Comment: https://git.openjdk.org/jdk/pull/14531#discussion_r1236549488
Re: RFR: 8308286 Fix clang warnings in linux code [v5]
On Sun, 11 Jun 2023 16:38:31 GMT, Artem Semenov wrote: >> When using the clang compiler to build OpenJDk on Linux, we encounter >> various "warnings as errors". >> They can be fixed with small changes. > > Artem Semenov has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - update > - update > - update > - update > - 8308286 Fix clang warnings in linux code Please update copyright years where they aren't at 2023 yet. I verified these changes on Ubuntu 20.04 + clang 10; they are both necessary and sufficient to make the build pass with warnings as errors. I saw one more warning: clang: warning: argument unused during compilation: '-static-libstdc++' [-Wunused-command-line-argument] in `BUILD_SYSLOOKUPLIB_link.log`, but that one does not break the build. I'm going to approve this once you fix the mentioned issues. Thanks for working on this! make/modules/java.desktop/lib/Awt2dLibraries.gmk line 235: > 233: DISABLED_WARNINGS_gcc := int-to-pointer-cast, \ > 234: DISABLED_WARNINGS_gcc_awt_Taskbar.c := parentheses, \ > 235: DISABLED_WARNINGS_clang_awt_Taskbar.c := parentheses, \ please group the disabled warnings by compiler (gcc, then clang, then clang_aix) - PR Review: https://git.openjdk.org/jdk/pull/14033#pullrequestreview-1489972241 PR Review Comment: https://git.openjdk.org/jdk/pull/14033#discussion_r1236510850
Re: RFR: 8310405: Linker.Option.firstVariadicArg should specify which index values are valid
On Wed, 21 Jun 2023 00:07:54 GMT, Maurizio Cimadamore wrote: > Possible suggestion/thing to try: use a bullet list to spell out all cases > for `index`. E.g. we know there's index == 0 (all variadic). Then we know > there's index = N (no variadic). Then there's index == m, 0 < m < N - which > means layouts 0..m are non-variadic and m..N are variadic (where n..m denotes > an interval with n included and m excluded). I think this is a good suggestion. It makes it much easier to understand. - PR Review Comment: https://git.openjdk.org/jdk/pull/14565#discussion_r1236509508
RFR: 8310502 : optimization for UUID#toString
By optimizing the implementation of java.lang.Long#fastUUID, the performance of the java.util.UUID#toString method can be significantly improved. The following are the test results of JMH: Benchmark Mode Cnt Score Error Units UUIDUtilsBenchmark.new thrpt5 92676.550 ± 292.213 ops/ms UUIDUtilsBenchmark.original thrpt5 37040.165 ± 1023.532 ops/ms - Commit messages: - 8310502 : optimization for UUID#toString Changes: https://git.openjdk.org/jdk/pull/14578/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14578=00 Issue: https://bugs.openjdk.org/browse/JDK-8310502 Stats: 105 lines in 1 file changed: 88 ins; 5 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/14578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578 PR: https://git.openjdk.org/jdk/pull/14578
