Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v3]
On Wed, 13 Sep 2023 12:27:39 GMT, Soumadipta Roy wrote: >> This test is running in tier1, and takes about 400 seconds to complete. >> Thus, it drags the execution time of tier1 on large machines. The commit >> includes changing the sequential run of test cases in >> java/util/concurrent/tck/JSR166TestCase.java to the best possible >> combination of parallelization to reduce the total runtime of the overall >> test and causing least possible change to user and system times. The below >> comparison shows existing and newer combinations of parallelizations: >> >> * before : **200.64s user 20.94s system 204% cpu >> 1:48.47 total** >> * fully parallel : **308.84s user 23.75s system 479% cpu >> 1:09.42 total** >> * default-details-tck : **244.69s user 22.03s system 314% cpu 1:24.94 >> total** >> * default-fjp_details-others : **260.12s user 24.47s system 404% cpu 1:10.31 >> total** >> >> Based on the comparison above, the fourth combination has a result very >> close to full parallelization and at the same time having the least >> deviation of system and user times among the newer combinations. Hence the >> commit includes the changes corresponding to the fourth combination. > > Soumadipta Roy has updated the pull request incrementally with one additional > commit since the last revision: > > Adding summary for each parallelized section. Marked as reviewed by martin (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15619#pullrequestreview-1625807482
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v2]
On Thu, 7 Sep 2023 16:22:18 GMT, Soumadipta Roy wrote: >> test/jdk/java/util/concurrent/tck/JSR166TestCase.java line 45: >> >>> 43: * @modules java.management >>> 44: * @run junit/othervm/timeout=1000 JSR166TestCase >>> 45: * @run junit/othervm/timeout=1000 -Djava.security.manager=allow >>> JSR166TestCase >> >> security manager testing is relatively less important. I would move this >> to a lower tier, while moving a test with >> -Djsr166.testImplementationDetails=true into tier1 > > Hi Martin, > > I will raise another commit trying to add summary for the segregations I still think the test with -Djava.security.manager=allow is relatively low value, so would also move it into its own @test id=security-manager - PR Review Comment: https://git.openjdk.org/jdk/pull/15619#discussion_r1325339151
Re: Java 21 clinit deadlock
Hi Simone, On 13/09/2023 4:16 pm, Simone Bordet wrote: Hi, On Tue, Sep 12, 2023 at 11:44 PM David Holmes wrote: Hi Simone, Thanks for the logs. Unfortunately they shed no light on what is happening. There is no sign of the MutableHttpFields class starting actual initialization, though we can see that it was linked (verified). And there are no exceptions indicated relevant to the class loading and initialization process that I can discern. The class initialization logging is unfortunately rather sparse and I think we would need to add some additional logging to shed further light on this. Are you in a position to patch the sources, create a custom build and test again with that build? Yes we can do this. I've created a draft PR here: https://github.com/openjdk/jdk/pull/15732 can you apply that patch, build and re-test? Hopefully that will show which thread is doing the missing initialization. Thanks, David
Re: RFR: 8316235: Optimization for DateTimeFormatter::format
On Wed, 13 Sep 2023 18:52:21 GMT, ExE Boss wrote:
>> In many scenarios, DateTimeFormatter::format is a slower operation.
>>
>> For example, the following business scenarios
>> 1. The json library
>> gson/jackson/[fastjson2](https://github.com/alibaba/fastjson2) formats
>> Instant/LocalDate/LocalTime/LocalDateTime/ZonedDateTim into strings.
>> 2. In data integration scenarios, for projects like
>> [datax](https://github.com/alibaba/datax)/[canal](https://github.com/alibaba/canal),
>> if the input type is Date/Instant and the output type is String, formatting
>> is required.
>>
>> This PR provides format performance optimization for commonly used date
>> patterns.
>>
>> ISO_INSTANT
>> ISO_LOCAL_TIME
>> ISO_LOCAL_DATE
>> ISO_LOCAL_DATETIME
>> HH:mm:ss
>> HH:mm:ss.SSS
>> -MM-dd
>> -MM-dd HH:mm:ss
>> -MM-dd'T'HH:mm:ss
>> -MM-dd HH:mm:ss.SSS
>> -MM-dd'T'HH:mm:ss.SSS
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 2661:
>
>> 2659: } else {
>> 2660: return super.format(context, buf);
>> 2661: }
>
> This can use `instanceof `:
> Suggestion:
>
> if (temporal instanceof LocalTime lt) {
> time = lt;
> } else if (temporal instanceof LocalDateTime ldt) {
> time = ldt.toLocalTime();
> } else if (temporal instanceof ZonedDateTime zdt) {
> time = zdt.toLocalTime();
> } else if (temporal instanceof OffsetDateTime odt) {
> time = odt.toLocalTime();
> } else if (temporal instanceof OffsetTime ot) {
> time = ot.toLocalTime();
> } else {
> return super.format(context, buf);
> }
>
>
> Or even a pattern switch:
> Suggestion:
>
> switch (temporal) {
> case LocalTime lt -> time = lt;
> case LocalDateTime ldt -> time = ldt.toLocalTime();
> case ZonedDateTime zdt -> time = zdt.toLocalTime();
> case OffsetDateTime odt -> time = odt.toLocalTime();
> case OffsetTime ot -> time = ot.toLocalTime();
> default -> {
> return super.format(context, buf);
> }
> }
It's a good idea to use the new switch syntax
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 2683:
>
>> 2681: * Composite printer and parser.
>> 2682: */
>> 2683: static class CompositePrinterParser implements
>> DateTimePrinterParser {
>
> This class can be `sealed`:
> Suggestion:
>
> static sealed class CompositePrinterParser implements
> DateTimePrinterParser {
Changes have been made based on your suggestions
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1325113374
PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1325112780
Re: RFR: 8316235: Optimization for DateTimeFormatter::format
On Wed, 13 Sep 2023 14:56:15 GMT, 温绍锦 wrote: > In many scenarios, DateTimeFormatter::format is a slower operation. > > For example, the following business scenarios > 1. The json library > gson/jackson/[fastjson2](https://github.com/alibaba/fastjson2) formats > Instant/LocalDate/LocalTime/LocalDateTime/ZonedDateTim into strings. > 2. In data integration scenarios, for projects like > [datax](https://github.com/alibaba/datax)/[canal](https://github.com/alibaba/canal), > if the input type is Date/Instant and the output type is String, formatting > is required. > > This PR provides format performance optimization for commonly used date > patterns. > > ISO_INSTANT > ISO_LOCAL_TIME > ISO_LOCAL_DATE > ISO_LOCAL_DATETIME > HH:mm:ss > HH:mm:ss.SSS > -MM-dd > -MM-dd HH:mm:ss > -MM-dd'T'HH:mm:ss > -MM-dd HH:mm:ss.SSS > -MM-dd'T'HH:mm:ss.SSS Performance comparison data is as follows: ## 1. Script bash configure make images sh make/devkit/createJMHBundle.sh bash configure --with-jmh=build/jmh/jars make test TEST="micro:java.time.format.DateTimeFormatterBench.*" ## 2. MacBookPro benchmark -Benchmark (pattern) Mode Cnt Score Error Units (baseline) -DateTimeFormatterBench.formatInstantsHH:mm:ss thrpt 15 14.888 ? 0.109 ops/ms -DateTimeFormatterBench.formatInstantsHH:mm:ss.SSS thrpt 15 10.132 ? 0.046 ops/ms -DateTimeFormatterBench.formatInstants -MM-dd'T'HH:mm:ss thrpt 15 8.993 ? 0.039 ops/ms -DateTimeFormatterBench.formatInstants -MM-dd'T'HH:mm:ss.SSS thrpt 15 7.400 ? 0.035 ops/ms -DateTimeFormatterBench.formatZonedDateTime HH:mm:ss thrpt 15 21.460 ? 0.056 ops/ms -DateTimeFormatterBench.formatZonedDateTime HH:mm:ss.SSS thrpt 15 14.439 ? 0.264 ops/ms -DateTimeFormatterBench.formatZonedDateTime -MM-dd'T'HH:mm:ss thrpt 15 12.742 ? 0.055 ops/ms -DateTimeFormatterBench.formatZonedDateTime -MM-dd'T'HH:mm:ss.SSS thrpt 15 9.059 ? 0.124 ops/ms +Benchmark (pattern) Mode Cnt Score Error Units (optimized) +DateTimeFormatterBench.formatInstantsHH:mm:ss thrpt 15 28.082 ? 0.284 ops/ms (+88.62%) +DateTimeFormatterBench.formatInstantsHH:mm:ss.SSS thrpt 15 25.483 ? 0.109 ops/ms (+151.51%) +DateTimeFormatterBench.formatInstants -MM-dd'T'HH:mm:ss thrpt 15 19.950 ? 0.444 ops/ms (+121.83%) +DateTimeFormatterBench.formatInstants -MM-dd'T'HH:mm:ss.SSS thrpt 15 18.391 ? 0.327 ops/ms (+148.52%) +DateTimeFormatterBench.formatZonedDateTime HH:mm:ss thrpt 15 57.870 ? 0.641 ops/ms (+169.66%) +DateTimeFormatterBench.formatZonedDateTime HH:mm:ss.SSS thrpt 15 43.783 ? 0.300 ops/ms (+203.22%) +DateTimeFormatterBench.formatZonedDateTime -MM-dd'T'HH:mm:ss thrpt 15 36.220 ? 0.194 ops/ms (+184.25%) +DateTimeFormatterBench.formatZonedDateTime -MM-dd'T'HH:mm:ss.SSS thrpt 15 32.512 ? 0.583 ops/ms (+258.89%) - PR Comment: https://git.openjdk.org/jdk/pull/15722#issuecomment-1717870613
Re: RFR: 8316235: Optimization for DateTimeFormatter::format
On Wed, 13 Sep 2023 14:56:15 GMT, 温绍锦 wrote:
> In many scenarios, DateTimeFormatter::format is a slower operation.
>
> For example, the following business scenarios
> 1. The json library
> gson/jackson/[fastjson2](https://github.com/alibaba/fastjson2) formats
> Instant/LocalDate/LocalTime/LocalDateTime/ZonedDateTim into strings.
> 2. In data integration scenarios, for projects like
> [datax](https://github.com/alibaba/datax)/[canal](https://github.com/alibaba/canal),
> if the input type is Date/Instant and the output type is String, formatting
> is required.
>
> This PR provides format performance optimization for commonly used date
> patterns.
>
> ISO_INSTANT
> ISO_LOCAL_TIME
> ISO_LOCAL_DATE
> ISO_LOCAL_DATETIME
> HH:mm:ss
> HH:mm:ss.SSS
> -MM-dd
> -MM-dd HH:mm:ss
> -MM-dd'T'HH:mm:ss
> -MM-dd HH:mm:ss.SSS
> -MM-dd'T'HH:mm:ss.SSS
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
2594:
> 2592: } else {
> 2593: return super.format(context, buf);
> 2594: }
This can use `instanceof `:
Suggestion:
if (temporal instanceof LocalDateTime ldt) {
date = ldt.toLocalDate();
} else if (temporal instanceof LocalDate ld) {
date = ld;
} else if (temporal instanceof ZonedDateTime zdt) {
date = zdt.toLocalDate();
} else if (temporal instanceof OffsetDateTime odt) {
date = odt.toLocalDate();
} else {
return super.format(context, buf);
}
Or even a pattern switch:
Suggestion:
switch (temporal) {
case LocalDateTime ldt -> date = ldt.toLocalDate();
case LocalDate ld -> date = ld;
case ZonedDateTime zdt -> date = zdt.toLocalDate();
case OffsetDateTime odt -> date = odt.toLocalDate();
default -> {
return super.format(context, buf);
}
}
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
2661:
> 2659: } else {
> 2660: return super.format(context, buf);
> 2661: }
This can use `instanceof `:
Suggestion:
if (temporal instanceof LocalTime lt) {
time = lt;
} else if (temporal instanceof LocalDateTime ldt) {
time = ldt.toLocalTime();
} else if (temporal instanceof ZonedDateTime zdt) {
time = zdt.toLocalTime();
} else if (temporal instanceof OffsetDateTime odt) {
time = odt.toLocalTime();
} else if (temporal instanceof OffsetTime ot) {
time = ot.toLocalTime();
} else {
return super.format(context, buf);
}
Or even a pattern switch:
Suggestion:
switch (temporal) {
case LocalTime lt -> time = lt;
case LocalDateTime ldt -> time = ldt.toLocalTime();
case ZonedDateTime zdt -> time = zdt.toLocalTime();
case OffsetDateTime odt -> time = odt.toLocalTime();
case OffsetTime ot -> time = ot.toLocalTime();
default -> {
return super.format(context, buf);
}
}
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
2683:
> 2681: * Composite printer and parser.
> 2682: */
> 2683: static class CompositePrinterParser implements
> DateTimePrinterParser {
This class can be `sealed`:
Suggestion:
static sealed class CompositePrinterParser implements DateTimePrinterParser
{
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1324933754
PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1324937172
PR Review Comment: https://git.openjdk.org/jdk/pull/15722#discussion_r1324941838
RFR: 8316235: Optimization for DateTimeFormatter::format
In many scenarios, DateTimeFormatter::format is a slower operation. For example, the following business scenarios 1. The json library gson/jackson/[fastjson2](https://github.com/alibaba/fastjson2) formats Instant/LocalDate/LocalTime/LocalDateTime/ZonedDateTim into strings. 2. In data integration scenarios, for projects like [datax](https://github.com/alibaba/datax)/[canal](https://github.com/alibaba/canal), if the input type is Date/Instant and the output type is String, formatting is required. This PR provides format performance optimization for commonly used date patterns. ISO_INSTANT ISO_LOCAL_TIME ISO_LOCAL_DATE ISO_LOCAL_DATETIME HH:mm:ss HH:mm:ss.SSS -MM-dd -MM-dd HH:mm:ss -MM-dd'T'HH:mm:ss -MM-dd HH:mm:ss.SSS -MM-dd'T'HH:mm:ss.SSS - Commit messages: - fix format LocalTime/LocalDateTime use utc - fix format ZonedDateTime & OffsetDateTime & OffsetTime use utc - sealed class CompositePrinterParser - use Pattern Matching switch - remove LocalTimePrinterParser::format support Instant - optimization support pattern '-MM-dd HH:mm:ss.SSS' - remove digit_k - optimize DateTimeFormatter::format - fix InstantPrinterParser miss nanos - remove TimeCompositePrinterParser, fix build error - ... and 1 more: https://git.openjdk.org/jdk/compare/e0845163...564ae18b Changes: https://git.openjdk.org/jdk/pull/15722/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15722=00 Issue: https://bugs.openjdk.org/browse/JDK-8316235 Stats: 418 lines in 6 files changed: 341 ins; 51 del; 26 mod Patch: https://git.openjdk.org/jdk/pull/15722.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15722/head:pull/15722 PR: https://git.openjdk.org/jdk/pull/15722
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v36]
On Wed, 13 Sep 2023 23:00:23 GMT, Srinivas Vamsi Parasa
wrote:
>> The goal is to develop faster sort routines for x86_64 CPUs by taking
>> advantage of AVX512 instructions. This enhancement provides an order of
>> magnitude speedup for Arrays.sort() using int, long, float and double arrays.
>>
>> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and
>> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the
>> performance data below.
>>
>>
>> **Arrays.sort performance data using JMH benchmarks for arrays with random
>> data**
>>
>> |Arrays.sort benchmark | Array Size | Baseline
>> (us/op)| AVX512 Sort (us/op) | Speedup |
>> |--- | --- | --- | --- | ---
>> |
>> |ArraysSort.doubleSort | 10 | 0.034 | 0.035
>> | 1.0 |
>> |ArraysSort.doubleSort | 25 | 0.116 | 0.089
>> | 1.3 |
>> |ArraysSort.doubleSort | 50 | 0.282 | 0.291
>> | 1.0 |
>> |ArraysSort.doubleSort | 75 | 0.474 | 0.358
>> | 1.3 |
>> |ArraysSort.doubleSort | 100 | 0.654 | 0.623
>> | 1.0 |
>> |ArraysSort.doubleSort | 1000| 9.274 | 6.331
>> | 1.5 |
>> |ArraysSort.doubleSort | 1 | 323.339 | 71.228
>> | **4.5** |
>> |ArraysSort.doubleSort | 10 | 4471.871|
>> 1002.748| **4.5** |
>> |ArraysSort.doubleSort | 100 | 51660.742 |
>> 12921.295 | **4.0** |
>> |ArraysSort.floatSort| 10 | 0.045 | 0.046
>> | 1.0 |
>> |ArraysSort.floatSort| 25 | 0.103 | 0.084
>> | 1.2 |
>> |ArraysSort.floatSort| 50 | 0.285 | 0.33
>> | 0.9 |
>> |ArraysSort.floatSort| 75 | 0.492 | 0.346
>> | 1.4 |
>> |ArraysSort.floatSort| 100 | 0.597 | 0.326
>> | 1.8 |
>> |ArraysSort.floatSort| 1000| 9.811 | 5.294
>> | 1.9 |
>> |ArraysSort.floatSort| 1 | 323.955 | 50.547
>> | **6.4** |
>> |ArraysSort.floatSort| 10 | 4326.38 | 731.152
>> | **5.9** |
>> |ArraysSort.floatSort| 100 | 52413.88|
>> 8409.193| **6.2** |
>> |ArraysSort.intSort | 10 | 0.033 | 0.033
>> | 1.0 |
>> |ArraysSort.intSort | 25 | 0.086 | 0.051
>> | 1.7 |
>> |ArraysSort.intSort | 50 | 0.236 | 0.151
>> | 1.6 |
>> |ArraysSort.intSort | 75 | 0.416 | 0.332
>> | 1.3 |
>> |ArraysSort.intSort | 100 | 0.63| 0.521
>> | 1.2 |
>> |ArraysSort.intSort | 1000| 10.518 | 4.698
>> | 2.2 |
>> |ArraysSort.intSort | 1 | 309.659 | 42.518
>> | **7.3** |
>> |ArraysSort.intSort | 10 | 4130.917|
>> 573.956 | **7.2** |
>> |ArraysSort.intSort | 100 | 49876.307 |
>> 6712.812| **7.4** |
>> |ArraysSort.longSort | 10 | 0.036 | 0.037
>> | 1.0 |
>> |ArraysSort.longSort | 25 | 0.094 | 0.08
>> | 1.2 |
>> |ArraysSort.longSort | 50 | 0.218 | 0.227
>> | 1.0 |
>> |ArraysSort.longSort | 75 | 0.466 | 0.402
>> | 1.2 |
>> |ArraysSort.longSort | 100 | 0.76| 0.58
>> | 1.3 |
>> |ArraysSort.longSort | 1000| 10.449 | 6
>
> Srinivas Vamsi Parasa has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Refactor the sort and partition intrinsics to accept method references for
> fallback functions
Hello Paul,
As per your suggestion, I've made the changes to pass a method reference to the
sort and partition intrinsics (to be used as the fallback method) using the
code snippet inspired by Vector API you showed below. It looks much cleaner now.
Could you please have a look at the changes in `DualPivotQuicksort.java` and
provide your feedback?
>
> ```java
> @FunctionalInterface
> interface SortOperation {
> void sort(A a, int low, int end int high);
> }
>
> // A erases to Object in bytecode so i think it should work
>
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v36]
> The goal is to develop faster sort routines for x86_64 CPUs by taking > advantage of AVX512 instructions. This enhancement provides an order of > magnitude speedup for Arrays.sort() using int, long, float and double arrays. > > This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and upto > ~4.5x improvement for 64-bit datatypes (long, double) as shown in the > performance data below. > > > **Arrays.sort performance data using JMH benchmarks for arrays with random > data** > > | Arrays.sort benchmark | Array Size | Baseline > (us/op)| AVX512 Sort (us/op) | Speedup | > | --- | --- | --- | --- | --- > | > | ArraysSort.doubleSort | 10 | 0.034 | 0.035 > | 1.0 | > | ArraysSort.doubleSort | 25 | 0.116 | 0.089 > | 1.3 | > | ArraysSort.doubleSort | 50 | 0.282 | 0.291 > | 1.0 | > | ArraysSort.doubleSort | 75 | 0.474 | 0.358 > | 1.3 | > | ArraysSort.doubleSort | 100 | 0.654 | 0.623 > | 1.0 | > | ArraysSort.doubleSort | 1000| 9.274 | 6.331 > | 1.5 | > | ArraysSort.doubleSort | 1 | 323.339 | 71.228 > | **4.5** | > | ArraysSort.doubleSort | 10 | 4471.871| > 1002.748| **4.5** | > | ArraysSort.doubleSort | 100 | 51660.742 | > 12921.295 | **4.0** | > | ArraysSort.floatSort| 10 | 0.045 | 0.046 > | 1.0 | > | ArraysSort.floatSort| 25 | 0.103 | 0.084 > | 1.2 | > | ArraysSort.floatSort| 50 | 0.285 | 0.33 > | 0.9 | > | ArraysSort.floatSort| 75 | 0.492 | 0.346 > | 1.4 | > | ArraysSort.floatSort| 100 | 0.597 | 0.326 > | 1.8 | > | ArraysSort.floatSort| 1000| 9.811 | 5.294 > | 1.9 | > | ArraysSort.floatSort| 1 | 323.955 | 50.547 > | **6.4** | > | ArraysSort.floatSort| 10 | 4326.38 | 731.152 > | **5.9** | > | ArraysSort.floatSort| 100 | 52413.88| > 8409.193| **6.2** | > | ArraysSort.intSort | 10 | 0.033 | 0.033 > | 1.0 | > | ArraysSort.intSort | 25 | 0.086 | 0.051 > | 1.7 | > | ArraysSort.intSort | 50 | 0.236 | 0.151 > | 1.6 | > | ArraysSort.intSort | 75 | 0.416 | 0.332 > | 1.3 | > | ArraysSort.intSort | 100 | 0.63| 0.521 > | 1.2 | > | ArraysSort.intSort | 1000| 10.518 | 4.698 > | 2.2 | > | ArraysSort.intSort | 1 | 309.659 | 42.518 > | **7.3** | > | ArraysSort.intSort | 10 | 4130.917| > 573.956 | **7.2** | > | ArraysSort.intSort | 100 | 49876.307 | > 6712.812| **7.4** | > | ArraysSort.longSort | 10 | 0.036 | 0.037 > | 1.0 | > | ArraysSort.longSort | 25 | 0.094 | 0.08 > | 1.2 | > | ArraysSort.longSort | 50 | 0.218 | 0.227 > | 1.0 | > | ArraysSort.longSort | 75 | 0.466 | 0.402 > | 1.2 | > | ArraysSort.longSort | 100 | 0.76| 0.58 > | 1.3 | > | ArraysSort.longSort | 1000| 10.449 | 6.239 > | 1.7 | > | ArraysSort.longSort | 1 | 307.074 | 70.284 > | **4.4** | > | ArraysSor... Srinivas Vamsi Parasa has updated the pull request incrementally with one additional commit since the last revision: Refactor the sort and partition intrinsics to accept method references for fallback functions - Changes: - all: https://git.openjdk.org/jdk/pull/14227/files - new: https://git.openjdk.org/jdk/pull/14227/files/ed8b95c9..172b2d3e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14227=35 - incr: https://webrevs.openjdk.org/?repo=jdk=14227=34-35 Stats: 251 lines in 14 files changed: 63 ins; 106 del; 82 mod Patch: https://git.openjdk.org/jdk/pull/14227.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14227/head:pull/14227 PR: https://git.openjdk.org/jdk/pull/14227
Re: RFR: 8316207: Fix typos in java.io.StreamTokenizer
On Wed, 13 Sep 2023 15:43:45 GMT, Pavel Rappo wrote: > This is a simple PR to fix a few typos in the doc comments of > java.io.StreamTokenizer. When reviewing it, please double-check my proposal > for L481. For this, you should ideally read the complete comment to the > `lowerCaseMode` method. Thanks! Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15723#pullrequestreview-1625562814
Re: RFR: 8316187: Modernize an example in StringTokenizer
On Wed, 13 Sep 2023 12:39:00 GMT, Pavel Rappo wrote: > This modernizes an example to use the extended for-statement introduced in > JDK 1.5. > > I understand that StringTokenizer is a legacy class. But legacy or not, a > class shouldn't promote older constructs when newer fit better. Especially > when advising on preferred alternatives to itself. > > That said, I wouldn't go as far as to use `var` anywhere in that example: JDK > 10, which introduced `var`, might still be relatively new to some. Nor would > I inline the call to `String.split` in the for-statement to dispense with the > `String[] result` variable: I reckon it's good for a reader unfamiliar with > `String.split` to see the type it returns. > > Perhaps one additional thing to ponder is this: we could either add `@see` to > point to `String.split` or make the whole example a `@snippet`, which > `@link`s code to the definition of `String.split`. Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15716#pullrequestreview-1625561688
Re: RFR: 8315999: Improve Date toString performance [v13]
On Wed, 13 Sep 2023 21:22:43 GMT, 温绍锦 wrote:
> Based on the use cases cited, if your library needs specific performance
> improvements for specific formats, they should be done in that library.
Not only JSON libraries, toString optimization of Date/Instant/LocalDateTime
and other classes will benefit in many places, such as logging in business
systems, etc.
Instant bizTime = ...;
LOG.log(Level.INFO, "bizDate {0}", bizTime);
-
PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1718401009
Re: RFR: 8315999: Improve Date toString performance [v13]
On Wed, 13 Sep 2023 14:22:35 GMT, 温绍锦 wrote: >> improve date toString performance, includes: >> >> java.util.Date.toString >> java.util.Date.toGMTString >> java.time.Instant.toString >> java.time.LocalDate.toString >> java.time.LocalDateTime.toString >> java.time.LocalTime.toString > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > simplify LocalDate::getChars > As a consideration to core-libs-dev readers, push commits when you are > convinced are ready for review and you don't intend to make more changes. It > will improve the signal to noise ratio. Thanks > As a consideration to core-libs-dev readers, push commits when you are > convinced are ready for review and you don't intend to make more changes. It > will improve the signal to noise ratio. Thanks Sorry, I will make sure to do more preparation before submitting any PRs in the future. - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1718372907
Re: RFR: 8315999: Improve Date toString performance [v13]
On Wed, 13 Sep 2023 19:21:34 GMT, Roger Riggs wrote: > Based on the use cases cited, if your library needs specific performance > improvements for specific formats, they should be done in that library. I already do this in [fastjson2](https://github.com/alibaba/fastjson2) for now, but more libraries would benefit if improved in jdk. - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1718334638
Integrated: 8315789: Minor HexFormat performance improvements
On Wed, 6 Sep 2023 13:36:22 GMT, Claes Redestad wrote: > This PR seeks to improve formatting of hex digits using `java.util.HexFormat` > somewhat. > > This is achieved getting rid of a couple of lookup tables, caching the result > of `HexFormat.of().withUpperCase()`, and removing tiny allocation that > happens in the `formatHex(A, byte)` method. Improvements range from 20-40% on > throughput, and some operations allocate less: > > > Name Cnt Base Error Test Error Unit > Diff% > HexFormatBench.appenderLower15 1,330 ± 0,021 1,065 ± 0,067 us/op > 19,9% (p = 0,000*) > :gc.alloc.rate15 11,481 ± 0,185 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.appenderLowerCached 15 1,317 ± 0,013 1,065 ± 0,054 us/op > 19,1% (p = 0,000*) > :gc.alloc.rate15 11,590 ± 0,111 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.appenderUpper15 1,330 ± 0,022 1,065 ± 0,036 us/op > 19,9% (p = 0,000*) > :gc.alloc.rate15 34,416 ± 0,559 0,007 ± 0,000 MB/sec > -100,0% (p = 0,000*) > :gc.alloc.rate.norm 15 48,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 0,000 0,000 counts > HexFormatBench.appenderUpperCached 15 1,353 ± 0,009 1,033 ± 0,014 us/op > 23,6% (p = 0,000*) > :gc.alloc.rate15 11,284 ± 0,074 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.toHexLower 15 0,198 ± 0,001 0,119 ± 0,008 us/op > 40,1% (p = 0,000*) > :gc.alloc.rate15 0,007 ± 0,000 0,007 ± 0,000 MB/sec > -0,0% (p = 0,816 ) > :gc.alloc.rate.norm 15 0,001 ± 0,000 0,001 ± 0,000 B/op > -40,1% (p = 0,000*) > :gc.count 15 0,000 0,000... This pull request has now been integrated. Changeset: 92ad4a23 Author:Claes Redestad URL: https://git.openjdk.org/jdk/commit/92ad4a2399bb06b36b167a60c00d2299917fca9f Stats: 210 lines in 2 files changed: 182 ins; 9 del; 19 mod 8315789: Minor HexFormat performance improvements Reviewed-by: rriggs - PR: https://git.openjdk.org/jdk/pull/15591
Re: RFR: 8315789: Minor HexFormat performance improvements [v2]
On Fri, 8 Sep 2023 10:32:40 GMT, Claes Redestad wrote: >> This PR seeks to improve formatting of hex digits using >> `java.util.HexFormat` somewhat. >> >> This is achieved getting rid of a couple of lookup tables, caching the >> result of `HexFormat.of().withUpperCase()`, and removing tiny allocation >> that happens in the `formatHex(A, byte)` method. Improvements range from >> 20-40% on throughput, and some operations allocate less: >> >> >> Name Cnt Base Error Test Error Unit >> Diff% >> HexFormatBench.appenderLower15 1,330 ± 0,021 1,065 ± 0,067 us/op >> 19,9% (p = 0,000*) >> :gc.alloc.rate15 11,481 ± 0,185 0,007 ± 0,000 MB/sec >> -99,9% (p = 0,000*) >> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 3,000 0,000 counts >> :gc.time 3 2,000ms >> HexFormatBench.appenderLowerCached 15 1,317 ± 0,013 1,065 ± 0,054 us/op >> 19,1% (p = 0,000*) >> :gc.alloc.rate15 11,590 ± 0,111 0,007 ± 0,000 MB/sec >> -99,9% (p = 0,000*) >> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 3,000 0,000 counts >> :gc.time 3 2,000ms >> HexFormatBench.appenderUpper15 1,330 ± 0,022 1,065 ± 0,036 us/op >> 19,9% (p = 0,000*) >> :gc.alloc.rate15 34,416 ± 0,559 0,007 ± 0,000 MB/sec >> -100,0% (p = 0,000*) >> :gc.alloc.rate.norm 15 48,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 0,000 0,000 counts >> HexFormatBench.appenderUpperCached 15 1,353 ± 0,009 1,033 ± 0,014 us/op >> 23,6% (p = 0,000*) >> :gc.alloc.rate15 11,284 ± 0,074 0,007 ± 0,000 MB/sec >> -99,9% (p = 0,000*) >> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 3,000 0,000 counts >> :gc.time 3 2,000ms >> HexFormatBench.toHexLower 15 0,198 ± 0,001 0,119 ± 0,008 us/op >> 40,1% (p = 0,000*) >> :gc.alloc.rate15 0,007 ± 0,000 0,007 ± 0,000 MB/sec >> -0,0% (p = 0,816 ) >> :gc.alloc.rate.norm 15 0,001 ± 0,000 0,001 ± 0,000 B/op >> -40,1% (p = 0,000*) >> :gc > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add toHexDigitsByte|Short|Int|Long microbenchmarks This non-lookup version looks fine; it won't invalidate caches when used and the code is easy to understand. Thanks for the cleanup and re-checking the performance benefits. - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15591#pullrequestreview-1625413399
Re: RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set [v3]
On Wed, 13 Sep 2023 20:21:20 GMT, Justin Lu wrote: >> Please review this PR which is a continuation of >> [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused >> code from the _sun.util.Calendar_ classes. >> >> `forceStandardTime` is always false. >> >> In addition, `locale` is never by used by _CalendarDate_ or any inheritors >> and can be removed. >> >> As a result, _ImmutableGregorianDate_ no longer needs to override the >> _setLocale_ method and throw UnsupportedOperationException. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Reflect review comments Marked as reviewed by aturbanov (Committer). - PR Review: https://git.openjdk.org/jdk/pull/15726#pullrequestreview-1625404590
Re: RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set [v3]
> Please review this PR which is a continuation of > [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused > code from the _sun.util.Calendar_ classes. > > `forceStandardTime` is always false. > > In addition, `locale` is never by used by _CalendarDate_ or any inheritors > and can be removed. > > As a result, _ImmutableGregorianDate_ no longer needs to override the > _setLocale_ method and throw UnsupportedOperationException. Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Reflect review comments - Changes: - all: https://git.openjdk.org/jdk/pull/15726/files - new: https://git.openjdk.org/jdk/pull/15726/files/91bd5a3f..5a3375ec Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15726=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15726=01-02 Stats: 5 lines in 1 file changed: 1 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15726/head:pull/15726 PR: https://git.openjdk.org/jdk/pull/15726
Integrated: 6228794: java.text.ChoiceFormat pattern behavior is not well documented.
On Tue, 22 Aug 2023 19:18:35 GMT, Justin Lu wrote: > Please review this PR and associated > [CSR](https://bugs.openjdk.org/browse/JDK-8314546) which expands on the > `java.text.ChoiceFormat` specification regarding its pattern. > > `j.text.ChoiceFormat` provides an example pattern in the class description, > but beyond that it does not specify any well-defined syntax for creating a > pattern. In addition, methods related to the pattern String are > under-specified. > > The wording for `getLimits()` and `getFormats()` was also adjusted, as there > are other ways to set the limits and formats beyond the constructor. > > The pattern syntax may be easier to view -> > https://cr.openjdk.org/~jlu/api/java.base/java/text/ChoiceFormat.html This pull request has now been integrated. Changeset: ce93d27f Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/ce93d27fe5725af6424573ceb29cc12f20165f69 Stats: 134 lines in 1 file changed: 87 ins; 18 del; 29 mod 6228794: java.text.ChoiceFormat pattern behavior is not well documented. Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/15392
Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v6]
> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8315720) > which refines the spec of `equals()` and `hashCode()` in `java.text.Format` > related classes. > > The current spec for most of these methods is either "_Overrides > _" or are incomplete/wrong (i.e. see `ChoiceFormat`). > > This fix adjusts the spec to provide a consistent definition for the > overridden methods and specify what is being compared/used to generate a hash > code value. > > For implementations that use at most a few fields, the values are stated, > otherwise a more general term is used as a substitution (i.e. see > `DecimalFormat`). Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Replace non-static with instance - Changes: - all: https://git.openjdk.org/jdk/pull/15459/files - new: https://git.openjdk.org/jdk/pull/15459/files/6fe64f17..b4e7ddd1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15459=05 - incr: https://webrevs.openjdk.org/?repo=jdk=15459=04-05 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15459.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15459/head:pull/15459 PR: https://git.openjdk.org/jdk/pull/15459
RFR: 8296246: Update Unicode Data Files to Version 15.1.0
This PR is to incorporate the latest Unicode 15.1, which was released
yesterday. Besides the usual character data update, an upgraded implementation
of RegEx which reflects the Indic Consonant Break specified in the latest
Unicode Annex #29 ("Unicode Text Segmentation") is included. A corresponding
CSR has also been drafted.
-
Commit messages:
- TR29 final version
- .md file update
- Final 8/28
- Draft 8/11
- GenerateExtraProperties tool
- initial commit
Changes: https://git.openjdk.org/jdk/pull/15728/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=15728=00
Issue: https://bugs.openjdk.org/browse/JDK-8296246
Stats: 1529 lines in 22 files changed: 1298 ins; 15 del; 216 mod
Patch: https://git.openjdk.org/jdk/pull/15728.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/15728/head:pull/15728
PR: https://git.openjdk.org/jdk/pull/15728
Re: RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set [v2]
On Wed, 13 Sep 2023 19:46:08 GMT, Justin Lu wrote:
>> Please review this PR which is a continuation of
>> [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused
>> code from the _sun.util.Calendar_ classes.
>>
>> `forceStandardTime` is always false.
>>
>> In addition, `locale` is never by used by _CalendarDate_ or any inheritors
>> and can be removed.
>>
>> As a result, _ImmutableGregorianDate_ no longer needs to override the
>> _setLocale_ method and throw UnsupportedOperationException.
>
> Justin Lu has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove isStandardTime() and inline as false
src/java.base/share/classes/sun/util/calendar/AbstractCalendar.java line 169:
> 167: }
> 168: // adjust time zone and daylight saving
> 169: int[] offsets = new int[2];
Let's move array allocation only to `if (zi instanceof ZoneInfo) {` case
src/java.base/share/classes/sun/util/calendar/AbstractCalendar.java line 177:
> 175: //as 1:30am DT/0:30am ST (before transition)
> 176: if (zi instanceof ZoneInfo) {
> 177: zoneOffset = ((ZoneInfo)zi).getOffsetsByWall(ms,
> offsets);
let's use pattern matching for instanceof
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15726#discussion_r1325015164
PR Review Comment: https://git.openjdk.org/jdk/pull/15726#discussion_r1325015642
Re: RFR: 8315789: Minor HexFormat performance improvements [v2]
On Fri, 8 Sep 2023 10:32:40 GMT, Claes Redestad wrote: >> This PR seeks to improve formatting of hex digits using >> `java.util.HexFormat` somewhat. >> >> This is achieved getting rid of a couple of lookup tables, caching the >> result of `HexFormat.of().withUpperCase()`, and removing tiny allocation >> that happens in the `formatHex(A, byte)` method. Improvements range from >> 20-40% on throughput, and some operations allocate less: >> >> >> Name Cnt Base Error Test Error Unit >> Diff% >> HexFormatBench.appenderLower15 1,330 ± 0,021 1,065 ± 0,067 us/op >> 19,9% (p = 0,000*) >> :gc.alloc.rate15 11,481 ± 0,185 0,007 ± 0,000 MB/sec >> -99,9% (p = 0,000*) >> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 3,000 0,000 counts >> :gc.time 3 2,000ms >> HexFormatBench.appenderLowerCached 15 1,317 ± 0,013 1,065 ± 0,054 us/op >> 19,1% (p = 0,000*) >> :gc.alloc.rate15 11,590 ± 0,111 0,007 ± 0,000 MB/sec >> -99,9% (p = 0,000*) >> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 3,000 0,000 counts >> :gc.time 3 2,000ms >> HexFormatBench.appenderUpper15 1,330 ± 0,022 1,065 ± 0,036 us/op >> 19,9% (p = 0,000*) >> :gc.alloc.rate15 34,416 ± 0,559 0,007 ± 0,000 MB/sec >> -100,0% (p = 0,000*) >> :gc.alloc.rate.norm 15 48,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 0,000 0,000 counts >> HexFormatBench.appenderUpperCached 15 1,353 ± 0,009 1,033 ± 0,014 us/op >> 23,6% (p = 0,000*) >> :gc.alloc.rate15 11,284 ± 0,074 0,007 ± 0,000 MB/sec >> -99,9% (p = 0,000*) >> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op >> -100,0% (p = 0,000*) >> :gc.count 15 3,000 0,000 counts >> :gc.time 3 2,000ms >> HexFormatBench.toHexLower 15 0,198 ± 0,001 0,119 ± 0,008 us/op >> 40,1% (p = 0,000*) >> :gc.alloc.rate15 0,007 ± 0,000 0,007 ± 0,000 MB/sec >> -0,0% (p = 0,816 ) >> :gc.alloc.rate.norm 15 0,001 ± 0,000 0,001 ± 0,000 B/op >> -40,1% (p = 0,000*) >> :gc > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add toHexDigitsByte|Short|Int|Long microbenchmarks I ran some experiments with a lookup-table approach (based on `HexDigits` and I can get some of these to be marginally faster when combining a lookup-table approach with the `ByteArray` hack, but there's no win when using one or the other in isolation. So I think much of the win is actually not from using a lookup-table, but from tickling the JIT to inline more and optimize a bit more aggressively. So I think this might be a case of micros telling us sweet little lies, and we should favor the intuition that lookup tables should be avoided unless absolutely necessary. I prefer the simplicity of this PR as it stands and think we should backtrack on some of the lookup tables we've recently added in `jdk.internal.util.Hex|Decimal|OctalDigits`. - PR Comment: https://git.openjdk.org/jdk/pull/15591#issuecomment-1718230389
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v21]
On Tue, 12 Sep 2023 10:49:38 GMT, Jorn Vernee wrote:
>> This patch contains the implementation of the foreign linker & memory API
>> JEP for Java 22. The initial patch is composed of commits brought over
>> directly from the [panama-foreign
>> repo](https://github.com/openjdk/panama-foreign). The main changes found in
>> this patch come from the following PRs:
>>
>> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous
>> iterations supported converting Java strings to and from native strings in
>> the UTF-8 encoding, we've extended the supported encoding to all the
>> encodings found in the `java.nio.charset.StandardCharsets` class.
>> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the
>> `MemoryLayout::sequenceLayout` factory method which inferred the size of the
>> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A
>> client is now required to explicitly specify the sequence size.
>> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added:
>> `Linker::canonicalLayouts`, which exposes a map containing the
>> platform-specific mappings of common C type names to memory layouts.
>> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access
>> varhandles, as well as byte offset and slice handles derived from memory
>> layouts, now feature an additional 'base offset' coordinate that is added to
>> the offset computed by the handle. This allows composing these handles with
>> other offset computation strategies that may not be based on the same memory
>> layout. This addresses use-cases where clients are working with 'dynamic'
>> layouts, whose size might not be known statically, such as variable length
>> arrays, or variable size matrices.
>> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now
>> redundant API. Clients can simply use the difference between the base
>> address of two memory segments.
>> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of
>> `SegmentAllocator::allocateArray`, by renaming methods that both allocate +
>> initialize memory segments to `allocateFrom`. (see the original PR for the
>> problematic case)
>> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the
>> documentation for variadic functions.
>> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to
>> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking
>> linux-x86 as a test bed.
>> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API
>> required. The `Linker::nativeLinker` method is not longer allowed to throw
>> an `UnsupportedO...
>
> Jorn Vernee has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - add missing space + reflow lines
> - Fix typo
>
>Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java
line 311:
> 309: };
> 310:
> 311: CANONICAL_LAYOUTS = Map.ofEntries(
`LibFallback::wcharSize()` and other getters for `LibFallback.NativeConstants`
fields can throw an error when `LibFallback.SUPPORTED` is `false` due to the
`fallbackLinker` library not being present, so this static initializer should
be made into a method instead:
Suggestion:
static final Map CANONICAL_LAYOUTS =
initCanonicalLayouts();
private static Map initCanonicalLayouts() {
if (!isSupported()) {
return null;
}
int wchar_size = LibFallback.wcharSize();
MemoryLayout wchartLayout = switch(wchar_size) {
case 2 -> JAVA_CHAR; // prefer JAVA_CHAR
default -> FFIType.layoutFor(wchar_size);
};
return Map.ofEntries(
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1324996396
Re: RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set [v2]
On Wed, 13 Sep 2023 19:40:04 GMT, Justin Lu wrote:
>> Please review this PR which is a continuation of
>> [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused
>> code from the _sun.util.Calendar_ classes.
>>
>> `forceStandardTime` is always false.
>>
>> In addition, `locale` is never by used by _CalendarDate_ or any inheritors
>> and can be removed.
>>
>> As a result, _ImmutableGregorianDate_ no longer needs to override the
>> _setLocale_ method and throw UnsupportedOperationException.
>
> Justin Lu has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove isStandardTime() and inline as false
src/java.base/share/classes/sun/util/calendar/AbstractCalendar.java line 170:
> 168: // adjust time zone and daylight saving
> 169: int[] offsets = new int[2];
> 170: if (date.isStandardTime()) {
This seems a little suspicious considering `isStandardTime()` is always false.
However, at this point, there shouldn't be any behavioral changes probably.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15726#discussion_r1324987799
Re: RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set [v2]
> Please review this PR which is a continuation of > [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused > code from the _sun.util.Calendar_ classes. > > `forceStandardTime` is always false. > > In addition, `locale` is never by used by _CalendarDate_ or any inheritors > and can be removed. > > As a result, _ImmutableGregorianDate_ no longer needs to override the > _setLocale_ method and throw UnsupportedOperationException. Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Remove isStandardTime() and inline as false - Changes: - all: https://git.openjdk.org/jdk/pull/15726/files - new: https://git.openjdk.org/jdk/pull/15726/files/d31bafae..91bd5a3f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15726=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15726=00-01 Stats: 34 lines in 3 files changed: 0 ins; 25 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/15726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15726/head:pull/15726 PR: https://git.openjdk.org/jdk/pull/15726
Re: jpackage Windows Wix v4
> On Sep 13, 2023, at 2:27 PM, Michael Hall wrote: > > > >> On Sep 13, 2023, at 2:21 PM, Alexey Semenyuk >> wrote: >> >> jpoackage requires at least Wix v3.0. If it doesn't recognize Wix4, it is a >> bug. >> >> - Alexey >> >> On 9/12/2023 6:50 PM, Michael Hall wrote: >>> Curiosity. I was setting up a new Windows 10 VirtualBox image to do >>> jpackage. It told me I needed Wix. First I needed dotnet to install wix. I >>> then got a Wix v4. jpackage told me I needed candle.exe and light.exe from >>> v3. I installed v3 but haven’t had a chance to try it. >>> >>> However, if v4 isn’t backward compatible with v3 how will jpackage continue >>> to work with this? Or did I miss something in my v4 install? >> > > Wix 4 looks pretty different was my concern. The dotnet install put into my > user directory in a .dotnet directory. .dotnet\tool\wix or something pretty > close to that. I had to add that to path. There didn’t appear to be any > accompanying candle or light exe’s that jpackage seemed to expect. Assuming > you are going to sometime have to support Wix v4 it might be worth looking > at. Again, possibly there is some other, more complete, way to get Wix v4 that I missed that would be compatible with jpackage going forward. I’m not remembering exactly what led my do getting v4. Possibly it’s not even GA yet?
Re: RFR: 8315999: Improve Date toString performance [v13]
On Wed, 13 Sep 2023 14:22:35 GMT, 温绍锦 wrote: >> improve date toString performance, includes: >> >> java.util.Date.toString >> java.util.Date.toGMTString >> java.time.Instant.toString >> java.time.LocalDate.toString >> java.time.LocalDateTime.toString >> java.time.LocalTime.toString > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > simplify LocalDate::getChars As a consideration to core-libs-dev readers, push commits when you are convinced are ready for review and you don't intend to make more changes. It will improve the signal to noise ratio. Thanks - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1718192179
Re: jpackage Windows Wix v4
> On Sep 13, 2023, at 2:21 PM, Alexey Semenyuk > wrote: > > jpoackage requires at least Wix v3.0. If it doesn't recognize Wix4, it is a > bug. > > - Alexey > > On 9/12/2023 6:50 PM, Michael Hall wrote: >> Curiosity. I was setting up a new Windows 10 VirtualBox image to do >> jpackage. It told me I needed Wix. First I needed dotnet to install wix. I >> then got a Wix v4. jpackage told me I needed candle.exe and light.exe from >> v3. I installed v3 but haven’t had a chance to try it. >> >> However, if v4 isn’t backward compatible with v3 how will jpackage continue >> to work with this? Or did I miss something in my v4 install? > Wix 4 looks pretty different was my concern. The dotnet install put into my user directory in a .dotnet directory. .dotnet\tool\wix or something pretty close to that. I had to add that to path. There didn’t appear to be any accompanying candle or light exe’s that jpackage seemed to expect. Assuming you are going to sometime have to support Wix v4 it might be worth looking at.
Re: RFR: 8315999: Improve Date toString performance [v13]
On Wed, 13 Sep 2023 14:22:35 GMT, 温绍锦 wrote: >> improve date toString performance, includes: >> >> java.util.Date.toString >> java.util.Date.toGMTString >> java.time.Instant.toString >> java.time.LocalDate.toString >> java.time.LocalDateTime.toString >> java.time.LocalTime.toString > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > simplify LocalDate::getChars Based on the use cases cited, if your library needs specific performance improvements for specific formats, they should be done in that library. - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1718186604
Re: jpackage Windows Wix v4
jpoackage requires at least Wix v3.0. If it doesn't recognize Wix4, it is a bug. - Alexey On 9/12/2023 6:50 PM, Michael Hall wrote: Curiosity. I was setting up a new Windows 10 VirtualBox image to do jpackage. It told me I needed Wix. First I needed dotnet to install wix. I then got a Wix v4. jpackage told me I needed candle.exe and light.exe from v3. I installed v3 but haven’t had a chance to try it. However, if v4 isn’t backward compatible with v3 how will jpackage continue to work with this? Or did I miss something in my v4 install?
Re: RFR: 8316050: Use hexadecimal encoding in MemorySegment::toString [v2]
On Wed, 13 Sep 2023 12:51:19 GMT, Per Minborg wrote: >> This PR proposes to use hexadecimal representation for a memory segment >> address rather than a decimal one. > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Change from limit to byteSize in MS::toString Making changes after commits are approved suggests that additional time should be allowed to the approvers to re-check. - PR Comment: https://git.openjdk.org/jdk/pull/15670#issuecomment-1718165093
Re: RFR: 8315810: Reimplement sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles [v3]
On Wed, 13 Sep 2023 18:40:07 GMT, Mandy Chung wrote: >> This reimplements >> `sun.reflect.ReflectionFactory::newConstructorForSerialization` with method >> handles. >> >> This API currently generates the bytecode which fails the verification >> because `new C; invokespecial A()` where the given class `C` and invoke a >> no-arg constructor of `C`'s first non-`Serializable` superclass `A` is not a >> valid operation per the VM specification. VM special cases the classes >> generated for reflection to skip verification for the constructors generated >> for serialization and externalization. This change will allow such VM hack >> to be removed. >> >> A `jdk.reflect.useOldSerializableConstructor` system property can be set to >> use the old implementation in case if customers run into any compatibility >> issue. I expect this change has very low compatibility risk. This system >> property is undocumented and will be removed in a future release. > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > review feedback LGTM - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15600#pullrequestreview-1625212661
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v2]
On Wed, 13 Sep 2023 18:12:15 GMT, Naoto Sato wrote: > Looks good to me, although I did not look at each l10n file, but sampled > some. Thanks for tackling this conversion. Thanks for the review; (In addition to testing), I ran a script to verify only white space escape sequences exist in JDK .properties files. (Excluding escape sequences in test files that should remain as is for the purpose of the test) - PR Comment: https://git.openjdk.org/jdk/pull/15694#issuecomment-1718139807
Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v5]
> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8315720) > which refines the spec of `equals()` and `hashCode()` in `java.text.Format` > related classes. > > The current spec for most of these methods is either "_Overrides > _" or are incomplete/wrong (i.e. see `ChoiceFormat`). > > This fix adjusts the spec to provide a consistent definition for the > overridden methods and specify what is being compared/used to generate a hash > code value. > > For implementations that use at most a few fields, the values are stated, > otherwise a more general term is used as a substitution (i.e. see > `DecimalFormat`). Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Adjust spec to not spec ALL, prevent potential maintenance/peformance issue - Changes: - all: https://git.openjdk.org/jdk/pull/15459/files - new: https://git.openjdk.org/jdk/pull/15459/files/0b0aeb51..6fe64f17 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15459=04 - incr: https://webrevs.openjdk.org/?repo=jdk=15459=03-04 Stats: 13 lines in 4 files changed: 0 ins; 7 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/15459.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15459/head:pull/15459 PR: https://git.openjdk.org/jdk/pull/15459
Re: RFR: 8315810: Reimplement sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles [v3]
> This reimplements > `sun.reflect.ReflectionFactory::newConstructorForSerialization` with method > handles. > > This API currently generates the bytecode which fails the verification > because `new C; invokespecial A()` where the given class `C` and invoke a > no-arg constructor of `C`'s first non-`Serializable` superclass `A` is not a > valid operation per the VM specification. VM special cases the classes > generated for reflection to skip verification for the constructors generated > for serialization and externalization. This change will allow such VM hack > to be removed. > > A `jdk.reflect.useOldSerializableConstructor` system property can be set to > use the old implementation in case if customers run into any compatibility > issue. I expect this change has very low compatibility risk. This system > property is undocumented and will be removed in a future release. Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/15600/files - new: https://git.openjdk.org/jdk/pull/15600/files/fb3bf590..c4e39d12 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15600=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15600=01-02 Stats: 10 lines in 2 files changed: 0 ins; 5 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/15600.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15600/head:pull/15600 PR: https://git.openjdk.org/jdk/pull/15600
Re: RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set
On Wed, 13 Sep 2023 17:52:13 GMT, Justin Lu wrote:
> Please review this PR which is a continuation of
> [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused
> code from the _sun.util.Calendar_ classes.
>
> `forceStandardTime` is always false.
>
> In addition, `locale` is never by used by _CalendarDate_ or any inheritors
> and can be removed.
>
> As a result, _ImmutableGregorianDate_ no longer needs to override the
> _setLocale_ method and throw UnsupportedOperationException.
src/java.base/share/classes/sun/util/calendar/CalendarDate.java line 293:
> 291:
> 292:
> 293: public boolean isStandardTime() {
Can we remove(inline) this method?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15726#discussion_r1324902104
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v2]
On Wed, 13 Sep 2023 17:38:28 GMT, Justin Lu wrote: >> JDK .properties files still use ISO-8859-1 encoding with escape sequences. >> It would improve readability to see the native characters instead of escape >> sequences (especially for the L10n process). The majority of files changed >> are localized resource files. >> >> This change converts the Unicode escape sequences in the JDK .properties >> files (both in src and test) to UTF-8 native characters. Additionally, the >> build logic is adjusted to read the .properties files in UTF-8 while >> generating the ListResourceBundle files. >> >> The only escape sequence not converted was `\u0020` as this is used to >> denote intentional trailing white space. (E.g. `key=This is the >> value:\u0020`) >> >> The conversion was done using native2ascii with options `-reverse -encoding >> UTF-8`. >> >> If this PR is integrated, the IDE default encoding for .properties files >> need to be updated to UTF-8. (IntelliJ IDEA locks .properties files as >> ISO-8859-1 unless manually changed). > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Replace InputStreamReader with BufferedReader Looks good to me, although I did not look at each l10n file, but sampled some. Thanks for tackling this conversion. - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15694#pullrequestreview-1625154951
RFR: 8313813: Field sun.util.calendar.CalendarDate#forceStandardTime is never set
Please review this PR which is a continuation of [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused code from the _sun.util.Calendar_ classes. `forceStandardTime` is always false. In addition, `locale` is never by used by _CalendarDate_ or any inheritors and can be removed. As a result, _ImmutableGregorianDate_ no longer needs to override the _setLocale_ method and throw UnsupportedOperationException. - Commit messages: - init Changes: https://git.openjdk.org/jdk/pull/15726/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15726=00 Issue: https://bugs.openjdk.org/browse/JDK-8313813 Stats: 14 lines in 2 files changed: 0 ins; 13 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15726.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15726/head:pull/15726 PR: https://git.openjdk.org/jdk/pull/15726
Re: RFR: 8267509: Improve IllegalAccessException message to include the cause of the exception [v3]
> This PR improves IllegalAccessException message thrown by `Lookup::findXXX` > APIs if the method's variable arity modifier bit is set and > `asVarargsCollector` fails. It will increase the exception message thrown by > asVarargsCollector`. Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: revised message - Changes: - all: https://git.openjdk.org/jdk/pull/15698/files - new: https://git.openjdk.org/jdk/pull/15698/files/921f1397..9cdd1f32 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15698=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15698=01-02 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15698.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15698/head:pull/15698 PR: https://git.openjdk.org/jdk/pull/15698
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v2]
> JDK .properties files still use ISO-8859-1 encoding with escape sequences. It > would improve readability to see the native characters instead of escape > sequences (especially for the L10n process). The majority of files changed > are localized resource files. > > This change converts the Unicode escape sequences in the JDK .properties > files (both in src and test) to UTF-8 native characters. Additionally, the > build logic is adjusted to read the .properties files in UTF-8 while > generating the ListResourceBundle files. > > The only escape sequence not converted was `\u0020` as this is used to denote > intentional trailing white space. (E.g. `key=This is the value:\u0020`) > > The conversion was done using native2ascii with options `-reverse -encoding > UTF-8`. > > If this PR is integrated, the IDE default encoding for .properties files need > to be updated to UTF-8. (IntelliJ IDEA locks .properties files as ISO-8859-1 > unless manually changed). Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Replace InputStreamReader with BufferedReader - Changes: - all: https://git.openjdk.org/jdk/pull/15694/files - new: https://git.openjdk.org/jdk/pull/15694/files/0f3698a5..ceb48bbe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15694=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15694=00-01 Stats: 18 lines in 2 files changed: 6 ins; 8 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15694.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15694/head:pull/15694 PR: https://git.openjdk.org/jdk/pull/15694
Re: RFR: 8314236: Overflow in Collections.rotate
On Mon, 14 Aug 2023 20:23:13 GMT, Aleksey Shipilev wrote:
>> `Collections.rotate` method contains a bug. This method throws
>> IndexOutOfBoundsException on arrays larger than $2^{30}$ elements. The way
>> to reproduce:
>>
>> final int size = (1 << 30) + 1;
>> final List list = new ArrayList<>(size);
>> for (int i = 0; i < size; ++i)
>> list.add((byte) 0);
>> Collections.rotate(list, size - 1);
>>
>> Output:
>> ```Exception in thread "main" java.lang.IndexOutOfBoundsException: Index
>> -2147483648 out of bounds for length 1073741825```
>>
>> In that case private method `Collections.rotate1` will be called. And the
>> line:
>> `i += distance;`
>> will cause overflow. I fixed this method and wrote a test for it.
>>
>> I've signed the Oracle Contributor Agreement, but I don't have permission to
>> raise a bug in the JDK Bug System.
>>
>> Kindly ask you to raise a bug.
>
> Submitted: [JDK-8314236](https://bugs.openjdk.org/browse/JDK-8314236)
>
> Please change the PR synopsis to: "8314236: Overflow in Collections.rotate".
>
> Also go to https://github.com/nikita-sakharin/jdk/actions, and enable testing
> workflows.
Aleksey Shipilëv (@shipilev), kindly remind you about the PR.
Could I ask you to review it, please?
-
PR Comment: https://git.openjdk.org/jdk/pull/15270#issuecomment-1718034420
Re: RFR: 8267509: Improve IllegalAccessException message to include the cause of the exception [v2]
On Wed, 13 Sep 2023 05:08:58 GMT, Chen Liang wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> use member (not method handle)
>
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 1722:
>
>> 1720: return this.withVarargs(true);
>> 1721: } catch (IllegalArgumentException ex) {
>> 1722: throw new IllegalAccessException("cannot make variable
>> arity: " + this + " because " + ex.getMessage());
>
> This now drops the member information (owner and name) from the message. Is
> that intended?
>
> In addition, the original message appears to include the method type, which
> can be used to diagnose why varargs is not possible (not an array for
> trailing parameter). I fail to see what extra information this patch offers
> that actually makes debugging easier as claimed in the JBS issue.
Good catch. It's not intended. Fixed.
I think in general it would be useful to make it clear in the exception message
of the reason especially for those who are new in using method handles.
Looking it again, it can simply hardcode the message (more friendly than IAE
exception message):
java.lang.IllegalAccessException: cannot make variable arity:
MyClass.m(Object[],int)void/invokeStatic because not an array type: int
vs
java.lang.IllegalAccessException: cannot make variable arity: trailing
parameter type is not an array type: MyClass.m(Object[],int)void/invokeStatic
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15698#discussion_r1324780782
Re: RFR: 8267509: Improve IllegalAccessException message to include the cause of the exception [v2]
> This PR improves IllegalAccessException message thrown by `Lookup::findXXX` > APIs if the method's variable arity modifier bit is set and > `asVarargsCollector` fails. It will increase the exception message thrown by > asVarargsCollector`. Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: use member (not method handle) - Changes: - all: https://git.openjdk.org/jdk/pull/15698/files - new: https://git.openjdk.org/jdk/pull/15698/files/8d5f1b16..921f1397 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15698=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15698=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15698.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15698/head:pull/15698 PR: https://git.openjdk.org/jdk/pull/15698
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v32]
> Addresses Jdk 8288899 : java/util/concurrent/ExecutorService/CloseTest.java > failed with "InterruptedException: sleep interrupted" and related issues. > > This is a major ForkJoin update (and hard to review -- sorry) that finally > addresses incompatibilities between ExecutorService and ForkJoinPool (which > claims to implement it), with the goal of avoiding continuing bug reports and > incompatibilities. Doing this required reworking internal control to use > phaser/seqlock-style versioning schemes (affecting nearly every method) that > ensure consistent data structures and actions without requiring global > synchronization or locking on every task execution that would massively > degrade performance. The previous lack of a solution to this was the main > reason for these incompatibilities. Doug Lea 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 77 additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8288899 - Strengthen translation of getAndX atomics; revert or adapt FJP accordingly - Merge branch 'openjdk:master' into JDK-8288899 - Force more orderings; improve diagnostics - Simplify signalling - Always help terminate when stopping - Merge branch 'openjdk:master' into JDK-8288899 - Use non-recursive tasks in close tests - Allow ThreadGroup access in tck tests - Avoid needing test threads - ... and 67 more: https://git.openjdk.org/jdk/compare/35251845...d04ce80f - Changes: - all: https://git.openjdk.org/jdk/pull/14301/files - new: https://git.openjdk.org/jdk/pull/14301/files/e8d7b75f..d04ce80f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14301=31 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=30-31 Stats: 5410 lines in 768 files changed: 2649 ins; 1894 del; 867 mod Patch: https://git.openjdk.org/jdk/pull/14301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301 PR: https://git.openjdk.org/jdk/pull/14301
RFR: 8316207: Fix typos in java.io.StreamTokenizer
This is a simple PR to fix a few typos in the doc comments of java.io.StreamTokenizer. When reviewing it, please double-check my proposal for L481. For this, you should ideally read the complete comment to the `lowerCaseMode` method. Thanks! - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk/pull/15723/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15723=00 Issue: https://bugs.openjdk.org/browse/JDK-8316207 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15723.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15723/head:pull/15723 PR: https://git.openjdk.org/jdk/pull/15723
Integrated: 8316087: Test SignedLoggerFinderTest.java is still failing
On Wed, 13 Sep 2023 11:26:30 GMT, Sean Coffey wrote: > Some log messages from the test may be dropped if the bootstraplogger is in > use at time of log call. (bootstap logger logs at INFO level, the security > event logger logs at DEBUG level) > > Modify the test to use a patched EventHelper class to let it log at INFO > level also, ensuring the bootstrap logger handles such logs. > > I'll log a follow on enhancement where the default logging level for > bootstrap logger can be modified (perhaps via the existing > `jdk.system.logger.level` system property) This pull request has now been integrated. Changeset: ff240a91 Author:Sean Coffey URL: https://git.openjdk.org/jdk/commit/ff240a9135e0f0c78ecffadbef38edb3b0479653 Stats: 15 lines in 2 files changed: 9 ins; 3 del; 3 mod 8316087: Test SignedLoggerFinderTest.java is still failing Reviewed-by: dfuchs - PR: https://git.openjdk.org/jdk/pull/15711
Re: RFR: 8316087: Test SignedLoggerFinderTest.java is still failing
On Wed, 13 Sep 2023 11:26:30 GMT, Sean Coffey wrote: > Some log messages from the test may be dropped if the bootstraplogger is in > use at time of log call. (bootstap logger logs at INFO level, the security > event logger logs at DEBUG level) > > Modify the test to use a patched EventHelper class to let it log at INFO > level also, ensuring the bootstrap logger handles such logs. > > I'll log a follow on enhancement where the default logging level for > bootstrap logger can be modified (perhaps via the existing > `jdk.system.logger.level` system property) Looks fine to me as a shorter term solution until the enhancement is done. An alternative enhancement could be to have another system property to configure at which level the EventHelper should log. - Marked as reviewed by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15711#pullrequestreview-1624746847
Re: RFR: 8315999: Improve Date toString performance [v13]
> improve date toString performance, includes: > > java.util.Date.toString > java.util.Date.toGMTString > java.time.Instant.toString > java.time.LocalDate.toString > java.time.LocalDateTime.toString > java.time.LocalTime.toString 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: simplify LocalDate::getChars - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/d3ad4906..b7a3528c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=12 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=11-12 Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15658/head:pull/15658 PR: https://git.openjdk.org/jdk/pull/15658
Re: RFR: 8315999: Improve Date toString performance [v13]
On Tue, 12 Sep 2023 22:53:34 GMT, Claes Redestad wrote: >> The reason for not using off++ is that it can be executed out of order, >> which may result in better performance. > > I don't think that would outweigh the extra branch and the increased code > size. I'd opt for simplicity. I've simplified LocalDate::getChars based on your suggestion - PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1324587746
Re: RFR: 8316150: Refactor get chars and string size [v3]
On Wed, 13 Sep 2023 02:17:00 GMT, 温绍锦 wrote: >> 1. Reduce duplicate stringSize code >> 2. Move java.lang.StringLatin1.getChars to >> jdk.internal.util.DecimalDigits::getCharLatin1，not only java.lang, other >> packages also need to use this method > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > fix build error @cl4es can you help me to review this PR? - PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1717697947
Re: RFR: 8316150: Refactor get chars and string size [v4]
> 1. Reduce duplicate stringSize code > 2. Move java.lang.StringLatin1.getChars to > jdk.internal.util.DecimalDigits::getCharLatin1，not only java.lang, other > packages also need to use this method 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: add comment - Changes: - all: https://git.openjdk.org/jdk/pull/15699/files - new: https://git.openjdk.org/jdk/pull/15699/files/fe5263c3..1ef45c5f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15699=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15699=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15699/head:pull/15699 PR: https://git.openjdk.org/jdk/pull/15699
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v31]
> Addresses Jdk 8288899 : java/util/concurrent/ExecutorService/CloseTest.java > failed with "InterruptedException: sleep interrupted" and related issues. > > This is a major ForkJoin update (and hard to review -- sorry) that finally > addresses incompatibilities between ExecutorService and ForkJoinPool (which > claims to implement it), with the goal of avoiding continuing bug reports and > incompatibilities. Doing this required reworking internal control to use > phaser/seqlock-style versioning schemes (affecting nearly every method) that > ensure consistent data structures and actions without requiring global > synchronization or locking on every task execution that would massively > degrade performance. The previous lack of a solution to this was the main > reason for these incompatibilities. Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Strengthen translation of getAndX atomics; revert or adapt FJP accordingly - Changes: - all: https://git.openjdk.org/jdk/pull/14301/files - new: https://git.openjdk.org/jdk/pull/14301/files/729e34f6..e8d7b75f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14301=30 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=29-30 Stats: 130 lines in 2 files changed: 22 ins; 19 del; 89 mod Patch: https://git.openjdk.org/jdk/pull/14301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301 PR: https://git.openjdk.org/jdk/pull/14301
Integrated: 8316050: Use hexadecimal encoding in MemorySegment::toString
On Mon, 11 Sep 2023 21:17:27 GMT, Per Minborg wrote: > This PR proposes to use hexadecimal representation for a memory segment > address rather than a decimal one. This pull request has now been integrated. Changeset: f9ab115a Author:Per Minborg URL: https://git.openjdk.org/jdk/commit/f9ab115acb5044f25e2553521a09c35ae02c9b84 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8316050: Use hexadecimal encoding in MemorySegment::toString Reviewed-by: rriggs, mcimadamore - PR: https://git.openjdk.org/jdk/pull/15670
Re: RFR: 8316050: Use hexadecimal encoding in MemorySegment::toString [v2]
> This PR proposes to use hexadecimal representation for a memory segment > address rather than a decimal one. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Change from limit to byteSize in MS::toString - Changes: - all: https://git.openjdk.org/jdk/pull/15670/files - new: https://git.openjdk.org/jdk/pull/15670/files/ed5c76c9..0a021318 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15670=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15670=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15670.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15670/head:pull/15670 PR: https://git.openjdk.org/jdk/pull/15670
RFR: 8316087: Test SignedLoggerFinderTest.java is still failing
Some log messages from the test may be dropped if the bootstraplogger is in use at time of log call. (bootstap logger logs at INFO level, the security event logger logs at DEBUG level) Modify the test to use a patched EventHelper class to let it log at INFO level also, ensuring the bootstrap logger handles such logs. I'll log a follow on enhancement where the default logging level for bootstrap logger can be modified (perhaps via the existing `jdk.system.logger.level` system property) - Commit messages: - More whitespace - Whitespace correction - 8316087 Changes: https://git.openjdk.org/jdk/pull/15711/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15711=00 Issue: https://bugs.openjdk.org/browse/JDK-8316087 Stats: 15 lines in 2 files changed: 9 ins; 3 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15711.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15711/head:pull/15711 PR: https://git.openjdk.org/jdk/pull/15711
RFR: 8316187: Modernize an example in StringTokenizer
This modernizes an example to use the extended for-statement introduced in JDK 1.5. I understand that StringTokenizer is a legacy class. But legacy or not, a class shouldn't promote older constructs when newer fit better. Especially when advising on preferred alternatives to itself. That said, I wouldn't go as far as to use `var` anywhere in that example: JDK 10, which introduced `var`, might still be relatively new to some. Nor would I inline the call to `String.split` in the for-statement to dispense with the `String[] result` variable: I reckon it's good for a reader unfamiliar with `String.split` to see the type it returns. Perhaps one additional thing to ponder is this: we could either add `@see` to point to `String.split` or make the whole example a `@snippet`, which `@link`s code to the definition of `String.split`. - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk/pull/15716/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15716=00 Issue: https://bugs.openjdk.org/browse/JDK-8316187 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15716.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15716/head:pull/15716 PR: https://git.openjdk.org/jdk/pull/15716
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v2]
On Thu, 7 Sep 2023 16:08:16 GMT, Martin Buchholz wrote: > Hello Soumadipta - pleased to "meet" you. > > If the intent is to split the tests into relatively more and less important > variants for placing into separate tiers, that should be clear from the > @summary's in the tests. Hi @Martin-Buchholz the latest commit addresses the comment regarding missing summary for each section of tests. Also from the JBS bug, I can see it has been agreed to have a separate PR for improvements to the test itself and segregation into appropriate tiers. - PR Comment: https://git.openjdk.org/jdk/pull/15619#issuecomment-1717526009
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v3]
> This test is running in tier1, and takes about 400 seconds to complete. Thus, > it drags the execution time of tier1 on large machines. The commit includes > changing the sequential run of test cases in > java/util/concurrent/tck/JSR166TestCase.java to the best possible combination > of parallelization to reduce the total runtime of the overall test and > causing least possible change to user and system times. The below comparison > shows existing and newer combinations of parallelizations: > > * before : **200.64s user 20.94s system 204% cpu > 1:48.47 total** > * fully parallel : **308.84s user 23.75s system 479% cpu > 1:09.42 total** > * default-details-tck : **244.69s user 22.03s system 314% cpu 1:24.94 > total** > * default-fjp_details-others : **260.12s user 24.47s system 404% cpu 1:10.31 > total** > > Based on the comparison above, the fourth combination has a result very close > to full parallelization and at the same time having the least deviation of > system and user times among the newer combinations. Hence the commit includes > the changes corresponding to the fourth combination. Soumadipta Roy has updated the pull request incrementally with one additional commit since the last revision: Adding summary for each parallelized section. - Changes: - all: https://git.openjdk.org/jdk/pull/15619/files - new: https://git.openjdk.org/jdk/pull/15619/files/1482259c..12e1a02a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15619=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15619=01-02 Stats: 8 lines in 1 file changed: 5 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15619.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15619/head:pull/15619 PR: https://git.openjdk.org/jdk/pull/15619
Integrated: 8311207: Cleanup for Optimization for UUID.toString
On Sat, 1 Jul 2023 01:44:15 GMT, 温绍锦 wrote: > [PR 14578 ](https://github.com/openjdk/jdk/pull/14578) still has unresolved > discussions, continue to make improvements. > > # Benchmark Result > > > sh make/devkit/createJMHBundle.sh > bash configure --with-jmh=build/jmh/jars > make test TEST="micro:java.util.UUIDBench.toString" > > > ## 1. > [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) > * cpu : intel xeon sapphire rapids (x64) > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 62.019 ± 0.622 ops/us > > +Benchmark (size) Mode Cnt Score Error Units > +UUIDBench.toString 2 thrpt 15 82.998 ± 0.739 ops/us (+33.82%) > > > ## 2. > [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) > * cpu : amd epc genoa (x64) > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 88.668 ± 0.672 ops/us > > +Benchmark (size) Mode Cnt Score Error Units > +UUIDBench.toString 2 thrpt 15 89.229 ± 0.271 ops/us (+0.63%) > > > > ## 3. > [aliyun_ecs_c8y.xlarge](https://help.aliyun.com/document_detail/25378.html#c8y) > * cpu : aliyun yitian 710 (aarch64) > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 49.382 ± 2.160 ops/us > > +Benchmark (size) Mode Cnt Score Error Units > +UUIDBench.toString 2 thrpt 15 49.636 ± 1.974 ops/us (+0.51%) > > > ## 4. MacBookPro M1 Pro > ``` diff > -Benchmark (size) Mode CntScore Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 103.543 ± 0.963 ops/us > > +Benchmark (size) Mode CntScore Error Units > +UUIDBench.toString 2 thrpt 15 110.976 ± 0.685 ops/us (+7.17%) > > > ## 5. Orange Pi 5 Plus > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 33.532 ± 0.396 ops/us > > +Benchmark (size) Mode Cnt Score Error Units (PR) > +UUIDBench.toString 2 thrpt 15 33.054 ± 0.190 ops/us (-4.42%) This pull request has now been integrated. Changeset: f8df754b Author:shaojin.wensj Committer: Claes Redestad URL: https://git.openjdk.org/jdk/commit/f8df754b9a3f58ff5f26e63de70d02f3433a9384 Stats: 55 lines in 2 files changed: 5 ins; 12 del; 38 mod 8311207: Cleanup for Optimization for UUID.toString Reviewed-by: liach, rriggs - PR: https://git.openjdk.org/jdk/pull/14745
Re: Java 21 clinit deadlock
Hi, On Tue, Sep 12, 2023 at 11:44 PM David Holmes wrote: > > Hi Simone, > > Thanks for the logs. Unfortunately they shed no light on what is > happening. There is no sign of the MutableHttpFields class starting > actual initialization, though we can see that it was linked (verified). > And there are no exceptions indicated relevant to the class loading and > initialization process that I can discern. > > The class initialization logging is unfortunately rather sparse and I > think we would need to add some additional logging to shed further light > on this. Are you in a position to patch the sources, create a custom > build and test again with that build? Yes we can do this. -- Simone Bordet --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
