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
Changes requested by liach (Author).
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 216:
> 214: */
> 215: public static int getCharsLatin1(int i, int index, byte[] buf) {
> 216: // Used by trusted callers. Assumes all necessary bounds checks
> have been done by the caller.
Can you move this into the javadoc, like
Caller must ensure buf has enough capacity for the value to be
written!
-
PR Review: https://git.openjdk.org/jdk/pull/15699#pullrequestreview-1623691652
PR Review Comment: https://git.openjdk.org/jdk/pull/15699#discussion_r1323970440
Re: RFR: 8267509: Improve IllegalAccessException message to include the cause of the exception
On Wed, 13 Sep 2023 01:12:52 GMT, Mandy Chung wrote:
> 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`.
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.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15698#discussion_r1323975225
Re: RFR: 8315999: Improve Date toString performance [v12]
> 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 two additional commits since the last revision: - restore ZoneOffset::buildId, reduce changes - restore ZoneOffset::buildId, reduce changes - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/8471814c..d3ad4906 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=11 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=10-11 Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 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 [v12]
On Tue, 12 Sep 2023 23:10:43 GMT, Claes Redestad wrote: >>> Have you considered potentially more generalizable optimizations to >>> `DateTimeFormatter.ISO_INSTANT.format(this)` here? >>> >>> Hand-rolling a fixed-length buffer, skipping the `StringBuilder` .. >>> understandably this can have a performance edge, but perhaps a >>> `DateTimeFormatter` like `ISO_INSTANT` can be optimized to get closer to >>> whatever speed-up this gets you - with broader implications. >> >> I submitted an optimization for the commonly used format of >> DateTimeFormatter::format > > Do you have a link to that PR? Is there an RFE filed for it? Optimization of DateTimeFormatter::format should be another PR, I created a [branche](https://github.com/wenshao/jdk/tree/optim_for_datetime_format) but the work is unfinished. - PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323870413
Re: RFR: 8315999: Improve Date toString performance [v11]
> 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: restore ZoneOffset::buildId, reduce changes - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/8c76799c..8471814c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=10 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=09-10 Stats: 36 lines in 1 file changed: 0 ins; 23 del; 13 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: 8316150: Refactor get chars and string size [v3]
> 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 - Changes: - all: https://git.openjdk.org/jdk/pull/15699/files - new: https://git.openjdk.org/jdk/pull/15699/files/1ab86e2a..fe5263c3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15699=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15699=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8316150: Refactor get chars and string size [v2]
> 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 - Changes: - all: https://git.openjdk.org/jdk/pull/15699/files - new: https://git.openjdk.org/jdk/pull/15699/files/f553fb16..1ab86e2a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15699=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15699=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
RFR: 8316150: Refactor get chars and string size
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 - Commit messages: - move StringLatin1::getChars to jdk.internal.util.DecimalDigits::getCharsLatin1 - move *::stringSize to jdk.internal.util.DecimalDigits::stringSize Changes: https://git.openjdk.org/jdk/pull/15699/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15699=00 Issue: https://bugs.openjdk.org/browse/JDK-8316150 Stats: 374 lines in 9 files changed: 153 ins; 190 del; 31 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
RFR: 8267509: Improve IllegalAccessException message to include the cause of the exception
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`. - Commit messages: - 8267509: Improve IllegalAccessException message to include the cause of the exception Changes: https://git.openjdk.org/jdk/pull/15698/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15698=00 Issue: https://bugs.openjdk.org/browse/JDK-8267509 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: 8315999: Improve Date toString performance [v9]
On Tue, 12 Sep 2023 17:53:47 GMT, ExE Boss wrote:
>> 温绍锦 has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> merge from master
>
> src/java.base/share/classes/java/time/LocalTime.java line 141:
>
>> 139: @Stable
>> 140: static final int[] DIGITS_K = new int[1000];
>> 141:
>
> Suggestion:
DIGITS_K has been removed
> src/java.base/share/classes/java/time/LocalTime.java line 179:
>
>> 177: int c3 = i % 10 + '0';
>> 178: DIGITS_K[i] = c0 + (c1 << 8) + (c2 << 16) + (c3 << 24);
>> 179: }
>
> Suggestion:
DIGITS_K has been removed
> src/java.base/share/classes/java/time/LocalTime.java line 1724:
>
>> 1722: v = DIGITS_K[rem2];
>> 1723: } else {
>> 1724: v = DIGITS_K[div - div2 * 1000];
>
> Suggestion:
>
> v = DecimalDigits.digitTriple(div - div2 * 1000);
DIGITS_K has been removed
> src/java.base/share/classes/java/time/LocalTime.java line 1740:
>
>> 1738: buf,
>> 1739: off,
>> 1740: DIGITS_K[rem1] & 0xff00 | (v >> 24)
>
> Suggestion:
>
> DecimalDigits.digitTriple(rem1) & 0xff00 | (v >> 24)
DIGITS_K has been removed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323811383
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323811309
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323811510
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323811428
Re: RFR: 8315999: Improve Date toString performance [v10]
On Wed, 13 Sep 2023 01:15:38 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: > > remove DIGITS_K > Do you have a link to that PR? Is there an RFE filed for it? @cl4es Optimization of DateTimeFormatter::format should be another PR, I created a [branche](https://github.com/wenshao/jdk/tree/optim_for_datetime_format) but the work is unfinished. - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1716790719
Re: RFR: 8315999: Improve Date toString performance [v9]
On Tue, 12 Sep 2023 23:08:25 GMT, Claes Redestad wrote:
>> 温绍锦 has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> merge from master
>
> src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 77:
>
>> 75:
>> 76: static {
>> 77: int[] digits_k = new int[1000];
>
> I'm very skeptical that adding a 1000 element lookup table for is
> worthwhile. That's a lookup table that'd span many cache lines, with plenty
> of cache misses. And even _if_ it is to be considered it should be split out
> and considered in isolation from this PR.
>
> What does the 2, 1 or 0 value you put in the lowest byte signify?
DIGITS_K has been removed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323808508
Re: RFR: 8315999: Improve Date toString performance [v10]
> 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: remove DIGITS_K - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/e4c5b67b..8c76799c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=09 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=08-09 Stats: 59 lines in 2 files changed: 6 ins; 38 del; 15 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: 8311207: Cleanup for Optimization for UUID.toString [v15]
On Tue, 12 Sep 2023 22:44:35 GMT, 温绍锦 wrote: >> src/java.base/share/classes/jdk/internal/util/HexDigits.java line 112: >> >>> 110: | (DIGITS[b1 & 0xff] << 16) >>> 111: | (((long) DIGITS[b2 & 0xff]) << 32) >>> 112: | (((long) DIGITS[b3 & 0xff]) << 48); >> >> Can you reverse the order of these source lines to put the shifts of the >> higher order bits before the lower order bit shifts. ``. >> Its easier to understand where the bits end up in the long. >> The rest of the change is better focused. > > if reverse packDigits order, performance will be slow, I don't know why yet. > > The following is the data running on MacBookPro M1 Max : > > make test TEST="micro:java.util.UUIDBench.toString" > > Benchmark (size) Mode Cnt Score Error Units (current order > 4f6ed3e6) > UUIDBench.toString 2 thrpt 15 96.396 ? 0.946 ops/us > > > Benchmark (size) Mode Cnt Score Error Units (reverse > packDigits order) > UUIDBench.toString 2 thrpt 15 86.496 ? 0.542 ops/us Looks like something that might be an interesting puzzler for JIT compiler folks. Perhaps added implicit casts to long messes something up? - PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1323773899
Re: RFR: 8315999: Improve Date toString performance [v9]
On Tue, 12 Sep 2023 13:10:42 GMT, 温绍锦 wrote: >> Of course, the optimization of DateTimeFormatter is more general, and we can >> spend time doing it later. The format of toString is fixed, we can not use >> DateTimeFormatter. > >> Have you considered potentially more generalizable optimizations to >> `DateTimeFormatter.ISO_INSTANT.format(this)` here? >> >> Hand-rolling a fixed-length buffer, skipping the `StringBuilder` .. >> understandably this can have a performance edge, but perhaps a >> `DateTimeFormatter` like `ISO_INSTANT` can be optimized to get closer to >> whatever speed-up this gets you - with broader implications. > > I submitted an optimization for the commonly used format of > DateTimeFormatter::format Do you have a link to that PR? Is there an RFE filed for it? - PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323705970
Re: RFR: 8315999: Improve Date toString performance [v9]
On Tue, 12 Sep 2023 17:23:00 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:
>
> merge from master
I think there's still too much going on in this PR, and it should probably be
split up into multiple PRs.
I think some of this is going a bit overboard. For starters I don't want to see
us adding a 1000-element lookup table for "`DecimalDigits::digitTriple`"
without significant evidence that that is worth it at application level. As has
been pointed out elsewhere lookup tables are prone to look great on
microbenchmarks but may behave poorly and even regress real world applications
by competing for cache. Any further additions needs to be justified with a
thorough examination and benchmarks that better simulate noisy real world
applications.
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 77:
> 75:
> 76: static {
> 77: int[] digits_k = new int[1000];
I'm very skeptical that adding a 1000 element lookup table for is worthwhile.
That's a lookup table that'd span many cache lines, with plenty of cache
misses. And even _if_ it is to be considered it should be split out and
considered in isolation from this PR.
What does the 2, 1 or 0 value you put in the lowest byte signify?
-
Changes requested by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15658#pullrequestreview-1623337467
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323702300
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native
On Tue, 12 Sep 2023 21:57:31 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). make/jdk/src/classes/build/tools/compileproperties/CompileProperties.java line 227: > 225: try (FileInputStream input = new FileInputStream(propertiesPath); > 226: // Read in JDK .properties files in UTF-8 > 227: InputStreamReader streamReader = new > InputStreamReader(input, StandardCharsets.UTF_8) Can we just uses `Files.newBufferedReader(Path.of(propertiesPath))` instead? - PR Review Comment: https://git.openjdk.org/jdk/pull/15694#discussion_r1323716978
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v35]
> 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 stub handling to use a generic function for all types - Changes: - all: https://git.openjdk.org/jdk/pull/14227/files - new: https://git.openjdk.org/jdk/pull/14227/files/c096ff62..ed8b95c9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14227=34 - incr: https://webrevs.openjdk.org/?repo=jdk=14227=33-34 Stats: 178 lines in 8 files changed: 35 ins; 100 del; 43 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: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v34]
On Mon, 11 Sep 2023 18:17:41 GMT, Jatin Bhateja wrote: >> Srinivas Vamsi Parasa has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix regression when intrinsics are disabled; enable insertion sort in >> intrinsic, change library name to libsimdsort > > src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 4205: > >> 4203: >> 4204: snprintf(ebuf_, sizeof(ebuf_), "avx512_sort_double"); >> 4205: StubRoutines::_arraysort_double = >> (address)os::dll_lookup(libsimdsort, ebuf_); > > Hi @vamsi-parasa , If we align these C++ stub entry points with java > intrinsic entry points which accept an element class argument, it will > significantly cleanup the code referring to these stub entry points. We can > pass an extra int type flag to stubs based on which C++ implementation can > appropriately call type specific leaf level routines. Hello Jatin, the suggested refactoring change is incorporated in the latest commit. - PR Review Comment: https://git.openjdk.org/jdk/pull/14227#discussion_r1323688986
Integrated: JDK-8315946: DecimalFormat and CompactNumberFormat do allow U+FFFE and U+FFFF in the pattern
On Fri, 8 Sep 2023 23:02:02 GMT, Justin Lu wrote:
> Please review this change which adjusts the pattern syntax specification for
> the two classes to represent the actual behavior. That is, U+FFFE and U+
> are allowed in the suffix/prefix. (Additionally; 'Unicode' is dropped from
> the definitions, as a Java character is composed of Unicode code points).
>
> See code below, no exception is thrown.
>
>
> String uFFFE = "\uFFFE";
> String u = "\u";
> var a = new DecimalFormat("prefixStart"+uFFFE+"0.00"+u+"SuffixEnd");
> a.format(1); // returns "prefixStart�1.00�SuffixEnd"
> var b = new CompactNumberFormat(a.toPattern(), a.getDecimalFormatSymbols(),
> new String[] {""});
> b.format(1); // returns "prefixStart�1�SuffixEnd"
This pull request has now been integrated.
Changeset: dde11551
Author:Justin Lu
URL:
https://git.openjdk.org/jdk/commit/dde11551e26dedd28168d2d4528e9dd66ed82999
Stats: 8 lines in 2 files changed: 2 ins; 0 del; 6 mod
8315946: DecimalFormat and CompactNumberFormat do allow U+FFFE and U+ in
the pattern
Reviewed-by: naoto
-
PR: https://git.openjdk.org/jdk/pull/15648
Re: RFR: 8315999: Improve Date toString performance [v9]
On Mon, 11 Sep 2023 16:13:01 GMT, 温绍锦 wrote: >> It was a suggestion, implicitly paired with removing ` + (year < 0 ? 1 : 0)` >> from line 2188. > > 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. - PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323687644
jpackage Windows Wix v4
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: 8311207: Cleanup for Optimization for UUID.toString [v15]
On Tue, 12 Sep 2023 19:23:13 GMT, Roger Riggs wrote: >> 温绍锦 has updated the pull request with a new target base due to a merge or a >> rebase. The pull request now contains 19 commits: >> >> - merge from master >> - Merge branch 'master' into optimization_for_uuid_to_string >> >># Conflicts: >># src/java.base/share/classes/java/util/UUID.java >># src/java.base/share/classes/jdk/internal/util/HexDigits.java >> - lo | hi => hi | lo >> - add DIGITS description >> - reversed how & hi >> - Merge branch 'master' into optimization_for_uuid_to_string >> - remove redundant parentheses >> - fix java doc, big-endian -> little-endian >> - Merge branch 'master' into optimization_for_uuid_to_string >> - use ByteArrayLittleEndian >> - ... and 9 more: https://git.openjdk.org/jdk/compare/e0845163...4f6ed3e6 > > src/java.base/share/classes/jdk/internal/util/HexDigits.java line 112: > >> 110: | (DIGITS[b1 & 0xff] << 16) >> 111: | (((long) DIGITS[b2 & 0xff]) << 32) >> 112: | (((long) DIGITS[b3 & 0xff]) << 48); > > Can you reverse the order of these source lines to put the shifts of the > higher order bits before the lower order bit shifts. ``. Its > easier to understand where the bits end up in the long. > The rest of the change is better focused. if change packDigits order, performance will be slow, I don't know why yet. The following is the data running on MacBookPro M1 Max : make test TEST="micro:java.util.UUIDBench.toString" Benchmark (size) Mode Cnt Score Error Units (current order) UUIDBench.toString 2 thrpt 15 96.396 ? 0.946 ops/us Benchmark (size) Mode Cnt Score Error Units (change packDigits order) UUIDBench.toString 2 thrpt 15 86.496 ? 0.542 ops/us - PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1323682688
RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native
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). - Commit messages: - Update header / copyright for CurrencyFormat - Adjust CurrencyFormat test to read in .properties with UTF-8 - Convert unicode escape sequences to native - Add clarifying comment in Bug6204853 for lack of conversion - Read JDK properties files in UTF-8 during build process for LRB Changes: https://git.openjdk.org/jdk/pull/15694/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15694=00 Issue: https://bugs.openjdk.org/browse/JDK-8301991 Stats: 28966 lines in 488 files changed: 14 ins; 0 del; 28952 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: Java 21 clinit deadlock
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? Cheers, David On 12/09/2023 6:35 pm, Simone Bordet wrote: David, On Mon, Sep 11, 2023 at 9:29 AM Simone Bordet wrote: Hi, On Mon, Sep 11, 2023 at 7:22 AM David Holmes wrote: I've looked at the dump and there is no sign that the MutableHttpFields is actively in use. It suggests to me that class initialization has failed but the class state has not been correctly updated and the waiters released. There were some changes in JDK 21 about how failures in this area were handled, so it is possible I/we got something wrong. Is it possible to try running this with additional logging enabled e.g. -Xlog:class+init=debug -Xlog:exceptions=debug We will try this, than you! Here is another failed run: JVM threads dump: https://gist.github.com/olamy/b3e20a76f0fc77232882b9be95db47e1 JVM output (gist is truncated, download full file): https://gist.githubusercontent.com/olamy/86f0a1215c722e5e9acf96cae597422e/raw/4404693c39fd767122d34a7a3dde1d797afa6f25/gistfile1.txt The JVM stops outputting at 2.360s, then we have a Jetty idle timeout after 30 seconds, then we see: [51.472s][debug][exceptions] Thread::clear_pending_exception: cleared exception:java.lang.VirtualMachineError '. And then more log lines follow after another 5 minutes. Let us know your findings. Thanks!
Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass [v4]
On Tue, 12 Sep 2023 21:09:25 GMT, Mandy Chung wrote:
>> test/micro/org/openjdk/bench/java/lang/CallerClassBench.java line 45:
>>
>>> 43: public class CallerClassBench {
>>> 44: static final StackWalker INST =
>>> StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
>>> 45:
>>
>> Could `DROP_METHOD_INFO` also be used here?
>
> yes but it's irrelevant in this benchmark as `getCallerClass` should be
> independent to `DROP_METHOD_INFO` option. The implementation always drops
> method info in the implementation. It does not affect the result.
Ah - understood, thanks.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15642#discussion_r1323600936
Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass [v4]
On Tue, 12 Sep 2023 21:06:15 GMT, Brent Christian wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> cleanup
>
> test/micro/org/openjdk/bench/java/lang/CallerClassBench.java line 45:
>
>> 43: public class CallerClassBench {
>> 44: static final StackWalker INST =
>> StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
>> 45:
>
> Could `DROP_METHOD_INFO` also be used here?
yes but it's irrelevant in this benchmark as `getCallerClass` should be
independent to `DROP_METHOD_INFO` option. The implementation always drops
method info in the implementation. It does not affect the result.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15642#discussion_r1323580483
Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass [v4]
On Mon, 11 Sep 2023 17:52:56 GMT, Mandy Chung wrote:
>> Typically it will find the caller class at the second stack frame from the
>> frame of executing `StackWalker::getCallerClass`. The initial size of the
>> buffer can be changed from 8 to 4 (the top 2 elements are reserved for
>> implementation use). If it fetches another batch, `getCallerClass` may be
>> invoked via core reflection, so subsequent batches can be increased to a
>> larger size. This PR also moves the benchmark for `getCallerClass` in its
>> own file because it does not need to test with different depth and can be
>> separated from StackWalkBench.
>
> Mandy Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> cleanup
test/micro/org/openjdk/bench/java/lang/CallerClassBench.java line 45:
> 43: public class CallerClassBench {
> 44: static final StackWalker INST =
> StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
> 45:
Could `DROP_METHOD_INFO` also be used here?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15642#discussion_r1323574924
Re: RFR: 8315810: Reimplement sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles [v2]
On Tue, 12 Sep 2023 19:11:30 GMT, Roger Riggs 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 three additional >> commits since the last revision: >> >> - Merge branch 'master' of https://github.com/openjdk/jdk into >> reflect-serializable-ctor >> - minor cleanup >> - Reimplement ReflectionFactory::newConstructorForSerialization with method >> handle > > src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 3552: > >> 3550: } >> 3551: } >> 3552: return false; > > Does it break encapsulation too much to export and use the same method from > jdk.internal.reflect.MethodHandleAccessorFactory. > Maybe not worth it for a small utility method. It's a small utility and I think it's ok to leave it this way. - PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1323533889
Integrated: 8267174: Many test files have the wrong Copyright header
On Tue, 5 Sep 2023 22:49:41 GMT, Erik Joelsson wrote: > There are a number of files in the `test` directory that have an incorrect > copyright header, which includes the "classpath" exception text. This patch > removes that text from all test files that I could find it in. I did this > using a combination of `sed` and `grep`. Reviewing this patch is probably > easier using the raw patch file or a suitable webrev format. > > It's my assumption that these headers were introduced by mistake as it's > quite easy to copy the wrong template when creating new files. This pull request has now been integrated. Changeset: 020255a7 Author:Erik Joelsson URL: https://git.openjdk.org/jdk/commit/020255a72dc374ba0bdd44772047f14a8bfe69a9 Stats: 1944 lines in 648 files changed: 0 ins; 1296 del; 648 mod 8267174: Many test files have the wrong Copyright header Reviewed-by: valeriep, aivanov, iris, dholmes, ihse - PR: https://git.openjdk.org/jdk/pull/15573
Re: RFR: 8267174: Many test files have the wrong Copyright header
On Tue, 5 Sep 2023 22:49:41 GMT, Erik Joelsson wrote: > There are a number of files in the `test` directory that have an incorrect > copyright header, which includes the "classpath" exception text. This patch > removes that text from all test files that I could find it in. I did this > using a combination of `sed` and `grep`. Reviewing this patch is probably > easier using the raw patch file or a suitable webrev format. > > It's my assumption that these headers were introduced by mistake as it's > quite easy to copy the wrong template when creating new files. Thanks for reviews! - PR Comment: https://git.openjdk.org/jdk/pull/15573#issuecomment-1716362585
RFR: 8316144: Remove unused field jdk.internal.util.xml.impl.XMLStreamWriterImpl.Element._Depth
A field `short _Depth` in the `jdk.internal.util.xml.impl.XMLStreamWriterImpl.Element` class is unused and can be removed. - Commit messages: - [PATCH] Remove unused field jdk.internal.util.xml.impl.XMLStreamWriterImpl.Element#_Depth Changes: https://git.openjdk.org/jdk/pull/15692/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15692=00 Issue: https://bugs.openjdk.org/browse/JDK-8316144 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15692.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15692/head:pull/15692 PR: https://git.openjdk.org/jdk/pull/15692
RFR: 8301247: JPackage app-image exe launches multiple exe's in JDK 17+
On Windows, restart app launcher in a way that when the parent app launcher process terminates, the child app launcher process is automatically terminated. - Commit messages: - 8301247: JPackage app-image exe launches multiple exe's in JDK 17+ Changes: https://git.openjdk.org/jdk/pull/15690/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15690=00 Issue: https://bugs.openjdk.org/browse/JDK-8301247 Stats: 515 lines in 5 files changed: 282 ins; 217 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/15690.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15690/head:pull/15690 PR: https://git.openjdk.org/jdk/pull/15690
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v15]
On Tue, 12 Sep 2023 16:58:20 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%) > > 温绍锦 has updated the pull request with a new target base due to a merge or a > rebase. The pull request now contains 19 commits: > > - merge from master > - Merge branch 'master' into optimization_for_uuid_to_string > ># Conflicts: ># src/java.base/share/classes/java/util/UUID.java ># src/java.base/share/classes/jdk/internal/util/HexDigits.java > - lo | hi => hi | lo > - add DIGITS description > - reversed how & hi > - Merge branch 'master' into optimization_for_uuid_to_string > - remove redundant parentheses > - fix java doc, big-endian -> little-endian > - Merge branch 'master' into optimization_for_uuid_to_string > - use ByteArrayLittleEndian > - ... and 9 more: https://git.openjdk.org/jdk/compare/e0845163...4f6ed3e6 src/java.base/share/classes/jdk/internal/util/HexDigits.java line 112: > 110: | (DIGITS[b1 & 0xff] << 16) > 111: | (((long) DIGITS[b2 & 0xff]) << 32) > 112: | (((long) DIGITS[b3 & 0xff]) << 48); Can you reverse the order of these source lines to put the shifts of the higher order bits before the lower order bit shifts. ``. Its easier to understand where the bits end up in the long. The rest of the change is better focused. - PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1323465285
Re: RFR: 8315810: Reimplement sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles [v2]
On Tue, 12 Sep 2023 16:37:28 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 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 three additional > commits since the last revision: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > reflect-serializable-ctor > - minor cleanup > - Reimplement ReflectionFactory::newConstructorForSerialization with method > handle src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 3552: > 3550: } > 3551: } > 3552: return false; Does it break encapsulation too much to export and use the same method from jdk.internal.reflect.MethodHandleAccessorFactory. Maybe not worth it for a small utility method. src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 384: > 382: > getConstructorAnnotations(constructorToCall), > 383: langReflectAccess. > 384: > getConstructorParameterAnnotations(constructorToCall)); This would be easier to read if either the line length or the indentation was modified. src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 400: > 398: } > 399: > 400: Extra blank line. - PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1323453214 PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1318801680 PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1318803452
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v7]
> 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 Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Use description tags to create syntax, include negative symbol in Number section - Changes: - all: https://git.openjdk.org/jdk/pull/15392/files - new: https://git.openjdk.org/jdk/pull/15392/files/216e5ac7..ff1be6ad Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15392=06 - incr: https://webrevs.openjdk.org/?repo=jdk=15392=05-06 Stats: 53 lines in 1 file changed: 30 ins; 0 del; 23 mod Patch: https://git.openjdk.org/jdk/pull/15392.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15392/head:pull/15392 PR: https://git.openjdk.org/jdk/pull/15392
Re: RFR: 8315999: Improve Date toString performance [v9]
On Tue, 12 Sep 2023 17:23:00 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:
>
> merge from master
`LocalTime::getNanoChars(byte[], int, int)` can use
`DecimalDigits::digitTriple(int)` instead of a local copy of
`DecimalDigits.DIGITS_K`:
src/java.base/share/classes/java/time/LocalTime.java line 141:
> 139: @Stable
> 140: static final int[] DIGITS_K = new int[1000];
> 141:
Suggestion:
src/java.base/share/classes/java/time/LocalTime.java line 179:
> 177: int c3 = i % 10 + '0';
> 178: DIGITS_K[i] = c0 + (c1 << 8) + (c2 << 16) + (c3 << 24);
> 179: }
Suggestion:
src/java.base/share/classes/java/time/LocalTime.java line 1710:
> 1708: buf,
> 1709: off,
> 1710: DIGITS_K[div2] & 0xff00 | '.'
Suggestion:
DecimalDigits.digitTriple(div2) & 0xff00 | '.'
src/java.base/share/classes/java/time/LocalTime.java line 1722:
> 1720: }
> 1721:
> 1722: v = DIGITS_K[rem2];
Suggestion:
v = DecimalDigits.digitTriple(rem2);
src/java.base/share/classes/java/time/LocalTime.java line 1724:
> 1722: v = DIGITS_K[rem2];
> 1723: } else {
> 1724: v = DIGITS_K[div - div2 * 1000];
Suggestion:
v = DecimalDigits.digitTriple(div - div2 * 1000);
src/java.base/share/classes/java/time/LocalTime.java line 1740:
> 1738: buf,
> 1739: off,
> 1740: DIGITS_K[rem1] & 0xff00 | (v >> 24)
Suggestion:
DecimalDigits.digitTriple(rem1) & 0xff00 | (v >> 24)
-
PR Review: https://git.openjdk.org/jdk/pull/15658#pullrequestreview-1622831447
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323381033
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323381182
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323379816
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323380364
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323380525
PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323380651
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v6]
On Fri, 8 Sep 2023 21:11:11 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 > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Drop Unicode from definition, implied src/java.base/share/classes/java/text/ChoiceFormat.java line 160: > 158: * Note:The relation is not equivalent to > 159: * > 160: * Below is an example of constructing a ChoiceFormat with a pattern: FWIW, in other parts of core libs such as java.lang.Double, I've used a definition list `` with terms `` and definitions `` to represent such grammars. - PR Review Comment: https://git.openjdk.org/jdk/pull/15392#discussion_r1323380975
Re: RFR: 8315999: Improve Date toString performance [v9]
> 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: merge from master - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/9040ea0c..e4c5b67b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=08 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=07-08 Stats: 100 lines in 1 file changed: 100 ins; 0 del; 0 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 [v8]
> 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 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 18 additional commits since the last revision: - merge from master & revert DateTimeFormatterBuilder - Merge branch 'master' into optim_for_date_to_string_2 # Conflicts: #src/java.base/share/classes/java/lang/StringUTF16.java #src/java.base/share/classes/jdk/internal/util/DecimalDigits.java - improved DateTimeFormatter.format - improved ISO_INSTANT format - revert un-related changes - revert un-related changes - revert un-related changes - rebase PR #15651 last version - base PR #15651 API - move java.lang.StringLatin1::getChars to jdk.internal.util.DecimalDigits::getCharsLatin1 - ... and 8 more: https://git.openjdk.org/jdk/compare/1cd96ce2...9040ea0c - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/8b3f0637..9040ea0c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=07 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=06-07 Stats: 1245 lines in 45 files changed: 528 ins; 609 del; 108 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: 8315585: Optimization for decimal to string [v14]
> BigDecimal is a commonly used class in business development, It is often > necessary to perform toString or toPlainString operations on BigDecimal. > > The current version uses StringBuilder resulting in multiple memory > allocations, I made a modification to improve performance. > > Because BigDecimal uses stringCache to cache the result of toString, the > performance of toString needs special treatment before testing, such as > clearing stringCache by unsafe operation before each test, The performance of > toString is similar to that of toEngineering. > > The performance data is as follows: > > ## 1. benchmark script > > sh make/devkit/createJMHBundle.sh > bash configure --with-jmh=build/jmh/jars > make test TEST="micro:java.math.BigDecimals.*ToPlainString" > make test TEST="micro:java.math.BigDecimals.*ToEngineering" > > > ## 2. benchmark environment > * virtual machine : > [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/zh/ecs/user-guide/overview-of-instance-families#c8i) > * cpu intel xeon sapphire rapids (x64) > > ## 3. benchmark result > > -BigDecimals.testHugeToPlainString avgt 15 188.691 ± 0.822 > ns/op (baseline) > -BigDecimals.testLargeToPlainStringavgt 1536.656 ± 0.065 ns/op > -BigDecimals.testSmallToPlainStringavgt 1534.342 ± 0.068 ns/op > -BigDecimals.testToPlainString avgt 15 1719.494 ± 24.886 ns/op > > +Benchmark Mode Cnt ScoreError > Units (optimize) > +BigDecimals.testHugeToPlainString avgt 15 133.972 ? 0.328 > ns/op (+40.84%) > +BigDecimals.testLargeToPlainStringavgt 1514.957 ? 0.047 > ns/op (145.07%) > +BigDecimals.testSmallToPlainStringavgt 1512.045 ? 0.036 > ns/op (+185.11) > +BigDecimals.testToPlainString avgt 15 1643.500 ? 3.217 > ns/op (+4.62%) > > -Benchmark Mode Cnt ScoreError > Units (baseline) > -BigDecimals.testHugeToEngineeringString avgt 15 207.621 ± 5.018 ns/op > -BigDecimals.testLargeToEngineeringString avgt 1535.658 ± 3.144 ns/op > -BigDecimals.testSmallToEngineeringString avgt 1515.142 ± 0.053 ns/op > -BigDecimals.testToEngineeringString avgt 15 1813.959 ± 12.842 ns/op > > +Benchmark Mode Cnt ScoreError > Units (optimize) > +BigDecimals.testHugeToEngineeringString avgt 15 142.110 ? 0.987 > ns/op (+45.09%) > +BigDecimals.testLargeToEngineeringString avgt 1512.509 ? 0.056 > ns/op (+185.05%) > +BigDecimals.testSmallToEngineer... 温绍锦 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 17 additional commits since the last revision: - merge from master - Merge branch 'master' into optimization_for_decimal_to_string - improved huge BigDecimal to string - Merge branch 'master' into optimization_for_decimal_to_string - fix scale 19 error & add testcases - bug fix & add testcases - remove jla.newStringLatin1NoRepl - rename jla.digit to digitPair - fix adjusted >= Integer.MAX_VALUE - Continue to optimize and reduce duplicate code - ... and 7 more: https://git.openjdk.org/jdk/compare/3925ed53...4ffbb235 - Changes: - all: https://git.openjdk.org/jdk/pull/1/files - new: https://git.openjdk.org/jdk/pull/1/files/a3035e8f..4ffbb235 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=1=13 - incr: https://webrevs.openjdk.org/?repo=jdk=1=12-13 Stats: 4933 lines in 136 files changed: 3138 ins; 1125 del; 670 mod Patch: https://git.openjdk.org/jdk/pull/1.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/1/head:pull/1 PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v15]
> [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%) 温绍锦 has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits: - merge from master - Merge branch 'master' into optimization_for_uuid_to_string # Conflicts: #src/java.base/share/classes/java/util/UUID.java #src/java.base/share/classes/jdk/internal/util/HexDigits.java - lo | hi => hi | lo - add DIGITS description - reversed how & hi - Merge branch 'master' into optimization_for_uuid_to_string - remove redundant parentheses - fix java doc, big-endian -> little-endian - Merge branch 'master' into optimization_for_uuid_to_string - use ByteArrayLittleEndian - ... and 9 more: https://git.openjdk.org/jdk/compare/e0845163...4f6ed3e6 - Changes: https://git.openjdk.org/jdk/pull/14745/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14745=14 Stats: 55 lines in 2 files changed: 5 ins; 12 del; 38 mod Patch: https://git.openjdk.org/jdk/pull/14745.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14745/head:pull/14745 PR: https://git.openjdk.org/jdk/pull/14745
Integrated: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication
On Sun, 10 Sep 2023 16:15:01 GMT, 温绍锦 wrote: > java.util.DecimalDigits::DIGITS and java.lang.StringLatin1.PACKED_DIGITS are > duplicates, We need to move > java.util.Digits/OctalDigits/DecimalDigits/HexDigits to the jdk.internal.util > package, and modify these classes to public class, so that classes in other > packages can also access them. > > DecimalDigits::DIGITS provides a new digitPair static method, used to replace > StringLatin1.PACKED_DIGITS access. > > In order to be consistent with the original StringLatin1.PACKED_DIGITS, > OctalDigits::DIGITS and DecimalDigits::DIGITS are modified to little-endian. > HexDigits::DIGITS will also be modified after PR #14745 is merged. > > These changes will be used by PR #15658 #1 This pull request has now been integrated. Changeset: e0845163 Author:shaojin.wensj Committer: Claes Redestad URL: https://git.openjdk.org/jdk/commit/e0845163aa57cc8f68b11e1a553885676358f2a6 Stats: 530 lines in 10 files changed: 257 ins; 260 del; 13 mod 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication Reviewed-by: rriggs, liach, redestad - PR: https://git.openjdk.org/jdk/pull/15651
Re: RFR: 8315810: Reimplement sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles [v2]
> 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 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 three additional commits since the last revision: - Merge branch 'master' of https://github.com/openjdk/jdk into reflect-serializable-ctor - minor cleanup - Reimplement ReflectionFactory::newConstructorForSerialization with method handle - Changes: - all: https://git.openjdk.org/jdk/pull/15600/files - new: https://git.openjdk.org/jdk/pull/15600/files/e98b5528..fb3bf590 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15600=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15600=00-01 Stats: 33895 lines in 1007 files changed: 19846 ins; 9174 del; 4875 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
Integrated: 8285447: StackWalker minimal batch size should be optimized for getCallerClass
On Fri, 8 Sep 2023 16:57:14 GMT, Mandy Chung wrote: > Typically it will find the caller class at the second stack frame from the > frame of executing `StackWalker::getCallerClass`. The initial size of the > buffer can be changed from 8 to 4 (the top 2 elements are reserved for > implementation use). If it fetches another batch, `getCallerClass` may be > invoked via core reflection, so subsequent batches can be increased to a > larger size. This PR also moves the benchmark for `getCallerClass` in its > own file because it does not need to test with different depth and can be > separated from StackWalkBench. This pull request has now been integrated. Changeset: d75d9774 Author:Mandy Chung URL: https://git.openjdk.org/jdk/commit/d75d9774c806e4bf73caa69cd78c31a132e4c812 Stats: 83 lines in 3 files changed: 65 ins; 12 del; 6 mod 8285447: StackWalker minimal batch size should be optimized for getCallerClass Reviewed-by: simonis - PR: https://git.openjdk.org/jdk/pull/15642
Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass [v4]
On Tue, 12 Sep 2023 14:38:07 GMT, Volker Simonis wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> cleanup
>
> src/java.base/share/classes/java/lang/StackStreamFactory.java line 766:
>
>> 764: @Override
>> 765: protected int batchSize(int lastBatchFrameCount) {
>> 766: // this method is only called when the caller class is not
>> found in
>
> Minor nit: start sentence with an uppercase letter.
I'll leave it as is.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15642#discussion_r1323277241
Re: RFR: JDK-8314194: Reusing CyclicBarrier, it should be possible to change the barrierCommand
On Fri, 11 Aug 2023 02:33:05 GMT, chenggwang wrote: > Sorry, my description in Issue JDK-8314194(which I submitted) is ambiguous > and makes you think of Phaser. My intention is that each generation of > CyclicBarrier barrierCommand can change. Let me give you a scenario > For example, the U.S. Army 'Gordon Sullivan Cup'. > Five tanks competing. > 1. The first round is for artillery strikes against targets. > 2. Second round of anti-aircraft machine gun targets. > 3. The third round is minefield racing. > The scoring criteria are different for each round, so the CyclicBarrier's > barrierCommand should be different for each round. But in the current code, > `private final Runnable barrierCommand`, constructing the CyclicBarrier > instance is already determined to be unchangeable. We should try to retain designed immutability in concurrency classes. Having had the experience of having fixed many bugs with knobs tunable at runtime. If you make a field mutable, you need to think not just about "set", but also about "compare and set", "freeze" and security implications of adding a new form of attack. So let's leave CyclicBarrier unchanged. - PR Comment: https://git.openjdk.org/jdk/pull/15239#issuecomment-1716024120
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v19]
On Tue, 12 Sep 2023 13:27:29 GMT, 温绍锦 wrote: >> java.util.DecimalDigits::DIGITS and java.lang.StringLatin1.PACKED_DIGITS are >> duplicates, We need to move >> java.util.Digits/OctalDigits/DecimalDigits/HexDigits to the >> jdk.internal.util package, and modify these classes to public class, so that >> classes in other packages can also access them. >> >> DecimalDigits::DIGITS provides a new digitPair static method, used to >> replace StringLatin1.PACKED_DIGITS access. >> >> In order to be consistent with the original StringLatin1.PACKED_DIGITS, >> OctalDigits::DIGITS and DecimalDigits::DIGITS are modified to little-endian. >> HexDigits::DIGITS will also be modified after PR #14745 is merged. >> >> These changes will be used by PR #15658 #1 > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > little-endian Thanks for the updates. - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15651#pullrequestreview-1622614379
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v19]
On Tue, 12 Sep 2023 13:27:29 GMT, 温绍锦 wrote: >> java.util.DecimalDigits::DIGITS and java.lang.StringLatin1.PACKED_DIGITS are >> duplicates, We need to move >> java.util.Digits/OctalDigits/DecimalDigits/HexDigits to the >> jdk.internal.util package, and modify these classes to public class, so that >> classes in other packages can also access them. >> >> DecimalDigits::DIGITS provides a new digitPair static method, used to >> replace StringLatin1.PACKED_DIGITS access. >> >> In order to be consistent with the original StringLatin1.PACKED_DIGITS, >> OctalDigits::DIGITS and DecimalDigits::DIGITS are modified to little-endian. >> HexDigits::DIGITS will also be modified after PR #14745 is merged. >> >> These changes will be used by PR #15658 #1 > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > little-endian Marked as reviewed by redestad (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15651#pullrequestreview-1622597772
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v18]
On Tue, 12 Sep 2023 13:24:56 GMT, 温绍锦 wrote: > I prefer to use little endian, most environments are little endian. Changes > to HexDigit will have to wait until #PR 14745 is merged. I don't have a very strong opinion except that we should be consistent across these related implementations. If you could either include `OctalDigits` changes in #14745 or prepare a separate PR for it I'm OK with this. - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715954790
Re: RFR: 8312498: Thread::getState and JVM TI GetThreadState should return TIMED_WAITING virtual thread is timed parked [v2]
> Thread::getState is an API for monitoring and management purposes to report > the thread state. If a virtual thread is parked with LockSupport.parkNanos, > its state is reported as WAITING when it should be TIMED_WAITING. JVM TI > GetThreadState has the same issue in that it returns > JVMTI_THREAD_STATE_WAITING_INDEFINITELY instead of the > JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT bit set. Not a very visible issue > because JDWP maps both states to "WAIT" but it may be noticed by tools using > other JVM TI agents. > > The change is straight-forward, it's just additional bit to indicate that the > parking/parked/pinned states are timed. The existing virtual/ThreadAPI.java > test is expanded to this scenario. A new test is added for JVM TI > GetThreadState to test waiting/timed-waited cases (including pinned) as test > coverage seems patchy here. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision: - Merge - Remove unecessary RF from test - Merge - Merge - Remove tab - Cleanup comments - Merge - Spurious tab - Test improvements - Initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/14978/files - new: https://git.openjdk.org/jdk/pull/14978/files/872ab6a7..7b9e0d5a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14978=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14978=00-01 Stats: 80726 lines in 2543 files changed: 46386 ins; 17285 del; 17055 mod Patch: https://git.openjdk.org/jdk/pull/14978.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14978/head:pull/14978 PR: https://git.openjdk.org/jdk/pull/14978
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v19]
On Tue, 12 Sep 2023 14:16:27 GMT, Roger Riggs wrote: > > The title has been updated, please help update the title of JIRA > > The description needs an update too. The description has also been updated - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715880514
Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass [v4]
On Mon, 11 Sep 2023 17:52:56 GMT, Mandy Chung wrote:
>> Typically it will find the caller class at the second stack frame from the
>> frame of executing `StackWalker::getCallerClass`. The initial size of the
>> buffer can be changed from 8 to 4 (the top 2 elements are reserved for
>> implementation use). If it fetches another batch, `getCallerClass` may be
>> invoked via core reflection, so subsequent batches can be increased to a
>> larger size. This PR also moves the benchmark for `getCallerClass` in its
>> own file because it does not need to test with different depth and can be
>> separated from StackWalkBench.
>
> Mandy Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> cleanup
Looks good. Thanks for doing the proposed changes.
src/java.base/share/classes/java/lang/StackStreamFactory.java line 766:
> 764: @Override
> 765: protected int batchSize(int lastBatchFrameCount) {
> 766: // this method is only called when the caller class is not
> found in
Minor nit: start sentence with an uppercase letter.
-
Marked as reviewed by simonis (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15642#pullrequestreview-1622420644
PR Review Comment: https://git.openjdk.org/jdk/pull/15642#discussion_r1323147343
Re: RFR: 8315999: Improve Date toString performance [v7]
On Tue, 12 Sep 2023 13:54:34 GMT, Roger Riggs wrote: > You haven't make the case for these changes, please describe the use cases > when performance is a significant constraint on application performance. The > changes largely just add more code to maintain without otherwise adding > sufficient value. Many scenarios require this optimization, such as: 1. The open source project [datax](https://github.com/alibaba/datax) is a data integration engine we maintain. When the input is a Date but the target type is String, the Date must be converted to String. In the flame graph below, commons-lang's FastDateFormat::format method consumes most of the time. It will be similar if replaced by DateTimeFormatter::format.  2. JSON libraries such as gson/jackson/fastjson need to convert Date/Instant into String, and format/toString usually takes up a relatively large proportion of time. - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1715842265
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v19]
On Tue, 12 Sep 2023 13:27:29 GMT, 温绍锦 wrote: >> Some codes in core libs are duplicated, including: >> java.util.DecimalDigits::DIGITS -> java.lang.StringLatin1.PACKED_DIGITS >> java.util.DecimalDigits::size -> java.lang.Long.stringSize >> >> We can reduce duplication through JavaLangAccess, which is also needed in >> other places, such as: >> https://github.com/openjdk/jdk/pull/1 > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > little-endian I have updated the JBS bug title and explicitly stated the actions in this patch: 1. Move Digits to jdk.internal.util 2. Keep the tables in DecimalDigits and access via a new digitPair static method 3. Change the DecimalDigits' table to be LE (instead of BE) and update implementation - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715829791
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v19]
On Tue, 12 Sep 2023 14:13:06 GMT, 温绍锦 wrote: > The title has been updated, please help update the title of JIRA The description needs an update too. - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715814638
Re: RFR: 8315968: Move java.util.Digits to jdk.internal.util and refactor to reduce duplication [v19]
On Tue, 12 Sep 2023 13:49:09 GMT, Roger Riggs wrote: > Please update this PR title and description to indicate this refactoring to > move to jdk.internal.util. I can update the Jira title and description to > match after that. The title has been updated, please help update the title of JIRA - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715808794
Re: RFR: 8315999: Improve Date toString performance [v7]
On Tue, 12 Sep 2023 13:05:19 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 two additional commits > since the last revision: > > - improved DateTimeFormatter.format > - improved ISO_INSTANT format You haven't make the case for these changes, please describe the use cases when performance is a significant constraint on application performance. The changes largely just add more code to maintain without otherwise adding sufficient value. - PR Comment: https://git.openjdk.org/jdk/pull/15658#issuecomment-1715774423
Re: RFR: 8315968: Consolidate java.util.Digits and StringLatin1::PACKED_DIGITS [v19]
On Tue, 12 Sep 2023 13:27:29 GMT, 温绍锦 wrote: >> Some codes in core libs are duplicated, including: >> java.util.DecimalDigits::DIGITS -> java.lang.StringLatin1.PACKED_DIGITS >> java.util.DecimalDigits::size -> java.lang.Long.stringSize >> >> We can reduce duplication through JavaLangAccess, which is also needed in >> other places, such as: >> https://github.com/openjdk/jdk/pull/1 > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > little-endian Please update this PR title and description to indicate this refactoring to move to jdk.internal.util. I can update the Jira title and description to match after that. - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715764494
Re: RFR: 8315968: Consolidate java.util.Digits and StringLatin1::PACKED_DIGITS [v19]
> Some codes in core libs are duplicated, including: > java.util.DecimalDigits::DIGITS -> java.lang.StringLatin1.PACKED_DIGITS > java.util.DecimalDigits::size -> java.lang.Long.stringSize > > We can reduce duplication through JavaLangAccess, which is also needed in > other places, such as: > https://github.com/openjdk/jdk/pull/1 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: little-endian - Changes: - all: https://git.openjdk.org/jdk/pull/15651/files - new: https://git.openjdk.org/jdk/pull/15651/files/93ed81da..c68c4e81 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15651=18 - incr: https://webrevs.openjdk.org/?repo=jdk=15651=17-18 Stats: 6 lines in 1 file changed: 1 ins; 1 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15651.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15651/head:pull/15651 PR: https://git.openjdk.org/jdk/pull/15651
Re: RFR: 8315968: Consolidate java.util.Digits and StringLatin1::PACKED_DIGITS [v18]
On Tue, 12 Sep 2023 10:11:53 GMT, Claes Redestad wrote: > Running some additional testing. This mostly looks fine. > > One issue is that you're swapping the byte-order in `DecimalDigits::DIGITS` > but not in `OctalDigits` and `HexDigits`. I think we need to keep these > internally consistent to avoid surprises. > > I also would like to see performance numbers of the byte order swap evaluated > in isolation. I suspect the real effect is small and might be due to JIT > noise rather than a real effect, and that this swap got rushed in without > solid evidence that it helps. > > If there's no significant performance difference I would prefer if we kept > `DecimalDigits::DIGITS` big-endian encoded - which is more intuitive to most > - and adjust code depending on `DecimalDigits::digitPair` to use `ByteArray` > rather than `ByteArrayLittleEndian`. I prefer to use little endian, most environments are little endian. Changes to HexDigit will have to wait until #PR 14745 is merged. - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715721750
Re: RFR: 8315999: Improve Date toString performance [v7]
On Mon, 11 Sep 2023 14:59:25 GMT, 温绍锦 wrote: >> The performance of optimizing DateTimeFormatter cannot be as fast as using >> ixed-length buffer directly. > > Of course, the optimization of DateTimeFormatter is more general, and we can > spend time doing it later. The format of toString is fixed, we can not use > DateTimeFormatter. > Have you considered potentially more generalizable optimizations to > `DateTimeFormatter.ISO_INSTANT.format(this)` here? > > Hand-rolling a fixed-length buffer, skipping the `StringBuilder` .. > understandably this can have a performance edge, but perhaps a > `DateTimeFormatter` like `ISO_INSTANT` can be optimized to get closer to > whatever speed-up this gets you - with broader implications. I submitted an optimization for the commonly used format of DateTimeFormatter::format - PR Review Comment: https://git.openjdk.org/jdk/pull/15658#discussion_r1323021409
Re: RFR: 8315999: Improve Date toString performance [v7]
> 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 two additional commits since the last revision: - improved DateTimeFormatter.format - improved ISO_INSTANT format - Changes: - all: https://git.openjdk.org/jdk/pull/15658/files - new: https://git.openjdk.org/jdk/pull/15658/files/26d7c7c0..8b3f0637 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15658=06 - incr: https://webrevs.openjdk.org/?repo=jdk=15658=05-06 Stats: 459 lines in 6 files changed: 393 ins; 44 del; 22 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: 8312522: Implementation of Foreign Function & Memory API [v21]
> 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 > `UnsupportedOperationException` on ... 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> - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/0e702f06..e68b95c1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=20 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=19-20 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8315968: Consolidate java.util.Digits and StringLatin1::PACKED_DIGITS [v18]
On Tue, 12 Sep 2023 10:11:53 GMT, Claes Redestad wrote: > If there's no significant performance difference I would prefer if we kept > `DecimalDigits::DIGITS` big-endian encoded - which is more intuitive to most > - and adjust code depending on `DecimalDigits::digitPair` to use `ByteArray` > rather than `ByteArrayLittleEndian`. Just a datapoint but when I test implementing `HexFormat::toHexDigits` using either `ByteArray` or `ByteArrayLittleEndian` then the difference is in the noise: Name Cnt Base Error Test Error Unit Diff% HexFormatBench.toHexDigitsLong 15 1,950 ± 0,012 1,941 ± 0,016 us/op0,5% (p = 0,081 ) - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715482633
Re: RFR: 8315968: Consolidate java.util.Digits and StringLatin1::PACKED_DIGITS [v18]
On Tue, 12 Sep 2023 01:00:39 GMT, 温绍锦 wrote: >> Some codes in core libs are duplicated, including: >> java.util.DecimalDigits::DIGITS -> java.lang.StringLatin1.PACKED_DIGITS >> java.util.DecimalDigits::size -> java.lang.Long.stringSize >> >> We can reduce duplication through JavaLangAccess, which is also needed in >> other places, such as: >> https://github.com/openjdk/jdk/pull/1 > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > little-endian Running some additional testing. This mostly looks fine. One issue is that you're swapping the byte-order in `DecimalDigits::DIGITS` but not in `OctalDigits` and `HexDigits`. I think we need to keep these internally consistent to avoid surprises. I also would like to see performance numbers of the byte order swap evaluated in isolation. I suspect the real effect is small and might be due to JIT noise rather than a real effect, and that this swap got rushed in without solid evidence that it helps. If there's no significant performance difference I would prefer if we kept `DecimalDigits::DIGITS` big-endian encoded - which is more intuitive to most - and adjust code depending on `DecimalDigits::digitPair` to use `ByteArray` rather than `ByteArrayLittleEndian`. - PR Comment: https://git.openjdk.org/jdk/pull/15651#issuecomment-1715419335
Re: RFR: 8314891: Additional Zip64 extra header validation
On Sat, 9 Sep 2023 14:33:53 GMT, Lance Andersen wrote: > Please review this PR which improves the Zip64 extra header validation: > > - Throw a ZipException If the extra len field is 0 and : > -- size, csize, or loc offset are set to 0x > -- disk starting number is set to 0x > > - We have a valid size for the Zip64 extra header but we are missing the > csize or loc fields if they are expected to be part of the header > > Mach5 tiers 1-3 are clean test/jdk/java/util/zip/ZipFile/MissingZIP64EntriesTest.java line 198: > 196: * actual value is stored in the Zip64 Extra Header > 197: */ > 198: private static final int ZIP64_MAGICCOUNT = 0x; Suggestion: private static final int ZIP64_MAGICCOUNT = 0x; - PR Review Comment: https://git.openjdk.org/jdk/pull/15650#discussion_r1322670868
Re: Java 21 clinit deadlock
David, On Mon, Sep 11, 2023 at 9:29 AM Simone Bordet wrote: > > Hi, > > On Mon, Sep 11, 2023 at 7:22 AM David Holmes wrote: > > I've looked at the dump and there is no sign that the MutableHttpFields > > is actively in use. It suggests to me that class initialization has > > failed but the class state has not been correctly updated and the > > waiters released. There were some changes in JDK 21 about how failures > > in this area were handled, so it is possible I/we got something wrong. > > Is it possible to try running this with additional logging enabled e.g. > > > > -Xlog:class+init=debug -Xlog:exceptions=debug > > We will try this, than you! Here is another failed run: JVM threads dump: https://gist.github.com/olamy/b3e20a76f0fc77232882b9be95db47e1 JVM output (gist is truncated, download full file): https://gist.githubusercontent.com/olamy/86f0a1215c722e5e9acf96cae597422e/raw/4404693c39fd767122d34a7a3dde1d797afa6f25/gistfile1.txt The JVM stops outputting at 2.360s, then we have a Jetty idle timeout after 30 seconds, then we see: [51.472s][debug][exceptions] Thread::clear_pending_exception: cleared exception:java.lang.VirtualMachineError '. And then more log lines follow after another 5 minutes. Let us know your findings. Thanks! -- 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
Integrated: 8316038: Fix doc typos in java.io.Console and java.util.Scanner
On Mon, 11 Sep 2023 17:58:20 GMT, Pavel Rappo wrote: > Please review this trivial PR. This pull request has now been integrated. Changeset: f55e7994 Author:Pavel Rappo URL: https://git.openjdk.org/jdk/commit/f55e799491c39dcaf7b3935b6d560ee0a3239191 Stats: 7 lines in 2 files changed: 0 ins; 0 del; 7 mod 8316038: Fix doc typos in java.io.Console and java.util.Scanner Reviewed-by: bpb, naoto, iris - PR: https://git.openjdk.org/jdk/pull/15667
Integrated: 8315938: Deprecate for removal Unsafe methods that have standard APIs for many releases
On Fri, 8 Sep 2023 15:54:05 GMT, Alan Bateman wrote: > There are several methods defined by sun.misc.Unsafe that have standard API > equivalents for many years and releases. The change proposed here is to > deprecate, for removal, the park, unpark, getLoadAverage, loadFence, > storeFence, and fullFence methods. Code using these methods should move to > LockSupport.park/unpark (Java 5), OperatingSystemMXBean.getSystemLoadAverage > (Java 6), and VarHandles xxxFence methods (Java 9). > > The following is a summary of a search of 175973022 classes in 484751 > artifacts to get a sense of the usage of these methods. > > - 1290 usages of Unsafe.park. 1195 are libraries that have re-packaged some > version of ForkJoinPool, maybe from the jsr166 repo, maybe an older JDK > release. In the remaining, only Ehcache stands out with 29 matches. It seems > to be one usage but the library seems to copied/shaded into other artifacts. > > - 1322 usages of Unsafe.unpark. 1243 are re-packaged ForkJoinPool. 29 > occurrences are Encache, again one usage but the library seems to > copied/shaded into other artifacts. > > - 22 usages of Unsafe.getLoadAverage. Most of these are one usage in many > published versions of Apache HBase. > > - 339 usages of Unsafe.loadFence, 1057 usages of Unsafe.storeFence, 517 > usages of Unsafe.fullFence. A handful of these are libraries with copies of > j.u.concurrent, the rest seem to be high performance libraries that support > older JDK releases or just haven't been updated to use VarHandles yet. This pull request has now been integrated. Changeset: d08258f7 Author:Alan Bateman URL: https://git.openjdk.org/jdk/commit/d08258f735053142e43896c16cf7c6155cd9c35f Stats: 22 lines in 1 file changed: 20 ins; 1 del; 1 mod 8315938: Deprecate for removal Unsafe methods that have standard APIs for many releases Reviewed-by: mchung, psandoz, iris - PR: https://git.openjdk.org/jdk/pull/15641
Re: RFR: 8316000: File.setExecutable silently fails if file does not exist
On Tue, 12 Sep 2023 01:36:34 GMT, Brian Burkhalter wrote:
>> On Windows, do not return `true` from the `java.io.File` methods
>> `setReadable(boolean, boolean)` and `setExecutable(boolean, boolean)` if the
>> file does not exist.
>
> src/java.base/windows/native/libjava/WinNTFileSystem_md.c line 479:
>
>> 477: if (access == java_io_FileSystem_ACCESS_READ ||
>> 478: access == java_io_FileSystem_ACCESS_EXECUTE) {
>> 479: return _waccess(pathbuf, 0) == 0 ? enable : JNI_FALSE;
>
> Here `enable` is returned for backward compatibility, but per the
> specification it seems that `JNI_TRUE` should be returned instead.
I don't think this is right, at least it doesn't work with ACLs and file system
security so it can't test if the file is executable. Also I have a concern
about mixing win32 and C runtime functions here. The main issue with these
setXXX methods is that don't map to DOS file attributes or ACL based security
so they will need to fail for some cases.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15673#discussion_r1322486689
