Re: RFR: 8254574: PrintWriter handling of InterruptedIOException is not documented
On Wed, 16 Feb 2022 22:32:21 GMT, Brian Burkhalter wrote: > Remove reference to `java.io.InterruptedIOException` from > `java.io.PrintStream`, and make the specifications of `checkError()`, > `setError()`, and `clearError()` consistent between `java.io.PrintStream` and > `java.io.PrintWriter`. I think looks okay, we should have removed that text from checkError's javadoc a long time ago. I've added the "csr" label to the PR as this is a spec change that should be tracked. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7507
Re: RFR: 8279995: jpackage --add-launcher option should allow overriding description [v4]
> Added ability to override description for additional launchers via > "description" property. Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision: 8279995: jpackage --add-launcher option should allow overriding description [v4] - Changes: - all: https://git.openjdk.java.net/jdk/pull/7399/files - new: https://git.openjdk.java.net/jdk/pull/7399/files/c5fea599..6d4b4d3f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7399=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7399=02-03 Stats: 5 lines in 2 files changed: 3 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7399.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7399/head:pull/7399 PR: https://git.openjdk.java.net/jdk/pull/7399
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v14]
On Wed, 16 Feb 2022 19:11:53 GMT, XenoAmess wrote: >> 8281631: HashMap copy constructor and putAll can over-allocate table > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > fix test OK, good progress. Yes, leaving ConcurrentHashMap to another PR is fine. Do the changes to Class.java and the enum optimal capacity test need to be part of this PR? They seem unrelated. It's not clear to me that test is actually testing anything useful; it's just testing the same computation a couple different ways. The test seems reasonable enough and is a good start. There are a number of things that could be improved about it though. 1) It should probably be a TestNG test. This will allow you to use a DataProvider and also use TestNG assertions. 2) There are 12 test cases here, which seems amenable to using a DataProvider. You could try to make a little "combo" test that combines the three classes with the four ways of creating them, but it might be difficult to do that without using reflection. It might be easier to write a table with 12 cases, even if there is a bit of repetition. 3) Instead of writing reflective code to create the maps, it's probably easier to use suppliers that create the maps into the desired state. Each of the 12 test cases should have a lambda that does the creation. The test method then invokes the supplier and makes its assertions over the resulting map instance. 4) The `fill12` method can be used in an expression if it returns its argument. 5) Create a map with 12 entries as part of the test setup, and then just use it as the copy constructor argument. Note, I'll be on vacation next week, so there will be a gap in my responses during that time. test/jdk/java/util/Map/HashMapsPutAllOverAllocateTableTest.java line 2: > 1: /* > 2: * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved. Fix copyright year. - PR: https://git.openjdk.java.net/jdk/pull/7431
Integrated: 8282007: Assorted enhancements to jpackage testing framework
On Wed, 16 Feb 2022 20:06:09 GMT, Alexey Semenyuk wrote: > 8282007: Assorted enhancements to jpackage testing framework This pull request has now been integrated. Changeset: cd234f5d Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/cd234f5dbebd18ebf0c78dfdf533318cdc627971 Stats: 742 lines in 17 files changed: 416 ins; 180 del; 146 mod 8282007: Assorted enhancements to jpackage testing framework Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/7502
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v4]
On Thu, 17 Feb 2022 05:11:40 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8282007: Assorted enhancements to jpackage testing framework [v3]
On Thu, 17 Feb 2022 03:50:39 GMT, Alexey Semenyuk wrote: >> 8282007: Assorted enhancements to jpackage testing framework > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > Copyright years fixed Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7502
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
On Thu, 17 Feb 2022 03:05:36 GMT, Athijegannathan Sundararajan wrote: >> Joe Darcy has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains six additional commits >> since the last revision: >> >> - Revisit and update fix to address review feedback. >> - Merge branch 'master' into 8268250 >> - Add missing ending newline. >> - Add test file. >> - Merge branch 'master' into 8268250 >> - 8268250: Class.arrayType() for a 255-d array throws undocumented >> IllegalArgumentException > > test/jdk/java/lang/Class/ArrayType.java line 2: > >> 1: /* >> 2: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. > > 2022? Copyright updated to cover 2022 (filed created in 2021); thanks,. - PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v4]
> Make explicit illegal argument cases of Class.arrayType. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8268300 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4382/files - new: https://git.openjdk.java.net/jdk/pull/4382/files/226b2260..b2a7d900 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4382=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4382=02-03 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4382.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4382/head:pull/4382 PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
On Thu, 17 Feb 2022 02:56:53 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Revisit and update fix to address review feedback. > - Merge branch 'master' into 8268250 > - Add missing ending newline. > - Add test file. > - Merge branch 'master' into 8268250 > - 8268250: Class.arrayType() for a 255-d array throws undocumented > IllegalArgumentException Reviving this request, I looked over the exception types defined in java.lang and if IllegalArgumentException is not used, UnsupportedOperationException seems to me to be the most appropriate alternative. This is a small behavioral change from the current (undocumented) behavior of throwing IllegalArgumentException, but I don't estimate the cases in question are common in practice. - PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8282007: Assorted enhancements to jpackage testing framework [v2]
On Wed, 16 Feb 2022 23:53:44 GMT, Alexander Matveev wrote: >> Alexey Semenyuk has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Don't fail if requested to uninstall not installed Debian package > > test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CfgFile.java line 2: > >> 1: /* >> 2: * Copyright (c) 2019, 2022 Oracle and/or its affiliates. All rights >> reserved. > > Missing ",". Fixed! > test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java line 2: > >> 1: /* >> 2: * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. > > Do you know why 2021 is removed? By accident. Restored and changed to 2022 - PR: https://git.openjdk.java.net/jdk/pull/7502
Re: RFR: 8282007: Assorted enhancements to jpackage testing framework [v3]
> 8282007: Assorted enhancements to jpackage testing framework Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: Copyright years fixed - Changes: - all: https://git.openjdk.java.net/jdk/pull/7502/files - new: https://git.openjdk.java.net/jdk/pull/7502/files/8ce156d9..c890c8a4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7502=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7502=01-02 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7502.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7502/head:pull/7502 PR: https://git.openjdk.java.net/jdk/pull/7502
Re: RFR: 8279508: Auto-vectorize Math.round API [v5]
On Wed, 16 Feb 2022 12:30:27 GMT, Jatin Bhateja wrote: >> Summary of changes: >> - Intrinsify Math.round(float) and Math.round(double) APIs. >> - Extend auto-vectorizer to infer vector operations on encountering scalar >> IR nodes for above intrinsics. >> - Test creation using new IR testing framework. >> >> Following are the performance number of a JMH micro included with the patch >> >> Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server) >> >> >> Benchmark | TESTSIZE | Baseline AVX3 (ops/ms) | Withopt AVX3 (ops/ms) | Gain >> ratio | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain ratio >> -- | -- | -- | -- | -- | -- | -- | -- >> FpRoundingBenchmark.test_round_double | 1024.00 | 584.99 | 1870.70 | 3.20 | >> 510.35 | 548.60 | 1.07 >> FpRoundingBenchmark.test_round_double | 2048.00 | 257.17 | 965.33 | 3.75 | >> 293.60 | 273.15 | 0.93 >> FpRoundingBenchmark.test_round_float | 1024.00 | 825.69 | 3592.54 | 4.35 | >> 825.32 | 1836.42 | 2.23 >> FpRoundingBenchmark.test_round_float | 2048.00 | 388.55 | 1895.77 | 4.88 | >> 412.31 | 945.82 | 2.29 >> >> >> Kindly review and share your feedback. >> >> Best Regards, >> Jatin > > Jatin Bhateja has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains seven commits: > > - 8279508: Adding few descriptive comments. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8279508 > - 8279508: Replacing by efficient instruction sequence based on MXCSR.RC > mode. > - 8279508: Adding vectorized algorithms to match the semantics of rounding > operations. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8279508 > - 8279508: Adding a test for scalar intrinsification. > - 8279508: Auto-vectorize Math.round API > _Mailing list message from [Joseph D. Darcy](mailto:joe.da...@oracle.com) on > [hotspot-dev](mailto:hotspot-...@mail.openjdk.java.net):_ > > On 2/12/2022 6:55 PM, Jatin Bhateja wrote: > > > On Fri, 21 Jan 2022 00:49:04 GMT, Sandhya Viswanathan > openjdk.org> wrote: > > > The JVM currently initializes the x86 mxcsr to round to nearest even, see > > > below in stubGenerator_x86_64.cpp: // Round to nearest (even), 64-bit > > > mode, exceptions masked StubRoutines::x86::_mxcsr_std = 0x1F80; The above > > > works for Math.rint which is specified to be round to nearest even. > > > Please see: > > > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sdm.html > > > : section 4.8.4 > > > The rounding mode needed for Math.round is round to positive infinity > > > which needs a different x86 mxcsr initialization(0x5F80). > > > Hi @sviswa7 , > > > As per JLS 17 section 15.4 Java follows round to nearest rounding policy > > > for all floating point operations except conversion to integer and > > > remainder where it uses round toward zero. > > That is a true background condition, but I will note that the Math.round > method does independently define the semantics of its operation and rounding > behavior, which has changed (slightly) over the lifetime of the platform. > > -Joe Hi @jddarcy , Thanks for your comments, patch has been updated to follow the prescribed semantics of Math.round API. - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
On Thu, 17 Feb 2022 02:56:53 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Revisit and update fix to address review feedback. > - Merge branch 'master' into 8268250 > - Add missing ending newline. > - Add test file. > - Merge branch 'master' into 8268250 > - 8268250: Class.arrayType() for a 255-d array throws undocumented > IllegalArgumentException LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
On Thu, 17 Feb 2022 02:56:53 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Revisit and update fix to address review feedback. > - Merge branch 'master' into 8268250 > - Add missing ending newline. > - Add test file. > - Merge branch 'master' into 8268250 > - 8268250: Class.arrayType() for a 255-d array throws undocumented > IllegalArgumentException test/jdk/java/lang/Class/ArrayType.java line 2: > 1: /* > 2: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. 2022? - PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
> Make explicit illegal argument cases of Class.arrayType. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8268300 Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision: - Revisit and update fix to address review feedback. - Merge branch 'master' into 8268250 - Add missing ending newline. - Add test file. - Merge branch 'master' into 8268250 - 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException - Changes: - all: https://git.openjdk.java.net/jdk/pull/4382/files - new: https://git.openjdk.java.net/jdk/pull/4382/files/088e025f..226b2260 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4382=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4382=01-02 Stats: 1299276 lines in 8092 files changed: 742875 ins; 505966 del; 50435 mod Patch: https://git.openjdk.java.net/jdk/pull/4382.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4382/head:pull/4382 PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v6]
> This is an early review of changes to better model JVM access flags, that is > "modifiers" like public, protected, etc. but explicitly at a VM level. > > Language level modifiers and JVM level access flags are closely related, but > distinct. There are concepts that overlap in the two domains (public, > private, etc.), others that only have a language-level modifier (sealed), and > still others that only have an access flag (synthetic). > > The existing java.lang.reflect.Modifier class is inadequate to model these > subtleties. For example, the bit positions used by access flags on different > kinds of elements overlap (such as "volatile" for fields and "bridge" for > methods. Just having a raw integer does not provide sufficient context to > decode the corresponding language-level string. Methods like > Modifier.methodModifiers() were introduced to cope with this situation. > > With additional modifiers and flags on the horizon with projects like > Valhalla, addressing the existent modeling deficiency now ahead of time is > reasonable before further strain is introduced. > > This PR in its current form is meant to give the overall shape of the API. It > is missing implementations to map from, say, method modifiers to access > flags, taking into account overlaps in bit positions. > > The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in > once the API is further along. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision: - Update JVMS references. - Merge branch 'master' into JDK-8266670 - Reorder constants by mask value per review feedback. - Merge branch 'master' into JDK-8266670 - Respond to more review feedback. - Respond to review feedback explicitly stating returned sets are immutable. - Fix typo from review feedback. - Merge branch 'master' into JDK-8266670 - JDK-8266670: Better modeling of modifiers in core reflection - Changes: - all: https://git.openjdk.java.net/jdk/pull/7445/files - new: https://git.openjdk.java.net/jdk/pull/7445/files/2f9b168d..02228108 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=04-05 Stats: 4119 lines in 128 files changed: 3549 ins; 218 del; 352 mod Patch: https://git.openjdk.java.net/jdk/pull/7445.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7445/head:pull/7445 PR: https://git.openjdk.java.net/jdk/pull/7445
Integrated: Merge jdk18
On Thu, 17 Feb 2022 00:16:02 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 18 -> JDK 19 This pull request has now been integrated. Changeset: b6e48e67 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/b6e48e678244481dd45d38bc3ddc325fccda2acc Stats: 423 lines in 9 files changed: 0 ins; 412 del; 11 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/7509
Re: RFR: 8282007: Assorted enhancements to jpackage testing framework [v2]
On Wed, 16 Feb 2022 23:51:30 GMT, Alexey Semenyuk wrote: >> 8282007: Assorted enhancements to jpackage testing framework > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > Don't fail if requested to uninstall not installed Debian package test/jdk/tools/jpackage/helpers/jdk/jpackage/test/CfgFile.java line 2: > 1: /* > 2: * Copyright (c) 2019, 2022 Oracle and/or its affiliates. All rights > reserved. Missing ",". test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java line 2: > 1: /* > 2: * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. Do you know why 2021 is removed? - PR: https://git.openjdk.java.net/jdk/pull/7502
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v6]
On Tue, 15 Feb 2022 03:45:19 GMT, Stuart Marks wrote: >>> The changes to j.l.Class and the EnumConstantDirectory test don't belong >>> here -- these are _uses_ of HashMap. This bug and fix should focus on >>> HashMap itself, to ensure that the cases in question allocate a table of >>> the right size. >> >> A test will fail if not change codes there. Every pr should pass ci, so I >> have no choice. >> >>> Are there any other maps that have this computation besides HashMap and >>> WeakHashMap? >> >> Good question. Can I get a list of classes where I should check？（I guesd I >> shall start at LinkedHashMap and hash sets， but have no further ideas） >> >>> There should be a regression test for this. It's probably sufficient to >>> base this on your original test program, which puts 12 entries into a >>> HashMap using a variety of techniques. It should assert that the table size >>> is 16 in all cases. Also, should there be a test case for WeakHashMap? >> >> OK I will have a try... a hard part is how to read private field in class >> but I think I can find some clue in the existed tests. >> >>> Also, I changed the summary of the bug report to be more precise. The PR >>> title will need to be changed to correspond to it. Thanks. >> >> OK. > >> A test will fail if not change codes there. Every pr should pass ci, so I >> have no choice. > > Hm, yes I recall in the preliminary email that there was some mention of a > test. However, the test seemed to use the same (incorrect) calculation, so > maybe the test needs to be fixed instead. > >> Good question. Can I get a list of classes where I should check？（I guesd I >> shall start at LinkedHashMap and hash sets， but have no further ideas） > > Offhand, the HashMap/LinkedHashMap and the corresponding Set classes, and > WeakHashMap, are the main places to look. IdentityHashMap and the Map.of() > implementations use a different organization so are probably unrelated. > ConcurrentHashMap is another obvious place; you might want to investigate > there, but depending on the fix (if any) we might want to handle it > separately. I'd search for "loadFactor" or "LOAD_FACTOR" and see if anything > else turns up. > >> OK I will have a try... a hard part is how to read private field in class >> but I think I can find some clue in the existed tests. > > There is an incantation in the test header to ensure that the java.base > module is opened for reflection. Let me know if you have trouble with it. @stuart-marks done. please review again. thanks. - PR: https://git.openjdk.java.net/jdk/pull/7431
RFR: Merge jdk18
Forwardport JDK 18 -> JDK 19 - Commit messages: - Merge - 8280415: Remove EA from JDK 18 version string starting with Initial RC promotion B35 on February 10, 2022 - 8281713: [BACKOUT] AArch64: Implement string_compare intrinsic in SVE The webrevs contain the adjustments done while merging with regards to each parent branch: - master: https://webrevs.openjdk.java.net/?repo=jdk=7509=00.0 - jdk18: https://webrevs.openjdk.java.net/?repo=jdk=7509=00.1 Changes: https://git.openjdk.java.net/jdk/pull/7509/files Stats: 423 lines in 9 files changed: 0 ins; 412 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/7509.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7509/head:pull/7509 PR: https://git.openjdk.java.net/jdk/pull/7509
Re: RFR: 8282007: Assorted enhancements to jpackage testing framework [v2]
> 8282007: Assorted enhancements to jpackage testing framework Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: Don't fail if requested to uninstall not installed Debian package - Changes: - all: https://git.openjdk.java.net/jdk/pull/7502/files - new: https://git.openjdk.java.net/jdk/pull/7502/files/895997ac..8ce156d9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7502=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7502=00-01 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7502.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7502/head:pull/7502 PR: https://git.openjdk.java.net/jdk/pull/7502
Integrated: 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if light.exe is not in %PATH%
On Wed, 16 Feb 2022 17:52:43 GMT, Alexey Semenyuk wrote: > 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if > light.exe is not in %PATH% This pull request has now been integrated. Changeset: 0b00ce17 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/0b00ce17cd6b530d9394e79ac8b07208cd4b92f5 Stats: 6 lines in 1 file changed: 3 ins; 0 del; 3 mod 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if light.exe is not in %PATH% Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/7500
Re: RFR: 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if light.exe is not in %PATH%
On Wed, 16 Feb 2022 17:52:43 GMT, Alexey Semenyuk wrote: > 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if > light.exe is not in %PATH% Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7500
Re: RFR: 8281315: Unicode, (?i) flag and backreference throwing IndexOutOfBounds Exception [v2]
On Wed, 16 Feb 2022 22:41:40 GMT, Ian Graves wrote: >> This is a fix in the buggy way CIBackRef traverses unicode characters that >> could be variable-length. Originally it followed the approach that BackRef >> does, but failed to account for unicode characters that could be 2 >> chars-long. The upper bound (groupSize) for the traversing loop is set by >> the difference between group start and stop indexes. This works for single >> char characters and it also works for case-sensitive comparisons because >> byte-by-byte comparisons are acceptable, but it doesn't work for a >> comparison where some kind of normalization (i.e. case) is required. This >> fix adjusts the upper bound for the loop that traverses the character when a >> two-char character is encountered. >> >> An alternative was to check the length of the group size by scanning the >> group in advance and converting to code points, but this could potentially >> result in multiple scans and codepoint conversions of the same matcher group >> which could be long. The solution that adjusts the loop bounds on the fly >> avoids this case. > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Removing increment variable and some other tweaks Looks fine. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7501
Re: RFR: 8254574: PrintWriter handling of InterruptedIOException is not documented
On Wed, 16 Feb 2022 22:32:21 GMT, Brian Burkhalter wrote:
> Remove reference to `java.io.InterruptedIOException` from
> `java.io.PrintStream`, and make the specifications of `checkError()`,
> `setError()`, and `clearError()` consistent between `java.io.PrintStream` and
> `java.io.PrintWriter`.
In the various methods which print there is still this sort of construct
try {
// print operation which can throw IOException
}
catch (InterruptedIOException x) {
Thread.currentThread().interrupt();
}
catch (IOException x) {
trouble = true;
}
where an `InterruptedIOException` causes the current thread to be interrupted.
Should this PR also excise these interrupts (as vestigial)? There is no longer
any description of this in the specifications of the two print stream classes
although in theory an, e.g., `OutputStream` which throws
`InterruptedIOException` could be passed in.
-
PR: https://git.openjdk.java.net/jdk/pull/7507
Re: RFR: 8281315: Unicode, (?i) flag and backreference throwing IndexOutOfBounds Exception [v2]
> This is a fix in the buggy way CIBackRef traverses unicode characters that > could be variable-length. Originally it followed the approach that BackRef > does, but failed to account for unicode characters that could be 2 > chars-long. The upper bound (groupSize) for the traversing loop is set by the > difference between group start and stop indexes. This works for single char > characters and it also works for case-sensitive comparisons because > byte-by-byte comparisons are acceptable, but it doesn't work for a comparison > where some kind of normalization (i.e. case) is required. This fix adjusts > the upper bound for the loop that traverses the character when a two-char > character is encountered. > > An alternative was to check the length of the group size by scanning the > group in advance and converting to code points, but this could potentially > result in multiple scans and codepoint conversions of the same matcher group > which could be long. The solution that adjusts the loop bounds on the fly > avoids this case. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Removing increment variable and some other tweaks - Changes: - all: https://git.openjdk.java.net/jdk/pull/7501/files - new: https://git.openjdk.java.net/jdk/pull/7501/files/28d80c80..c4e5343e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7501=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7501=00-01 Stats: 9 lines in 1 file changed: 0 ins; 1 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/7501.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7501/head:pull/7501 PR: https://git.openjdk.java.net/jdk/pull/7501
RFR: 8254574: PrintWriter handling of InterruptedIOException is not documented
Remove reference to `java.io.InterruptedIOException` from `java.io.PrintStream`, and make the specifications of `checkError()`, `setError()`, and `clearError()` consistent between `java.io.PrintStream` and `java.io.PrintWriter`. - Commit messages: - 8254574: PrintWriter handling of InterruptedIOException is not documented Changes: https://git.openjdk.java.net/jdk/pull/7507/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7507=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8254574 Stats: 21 lines in 2 files changed: 0 ins; 11 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/7507.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7507/head:pull/7507 PR: https://git.openjdk.java.net/jdk/pull/7507
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]
On Wed, 16 Feb 2022 21:39:20 GMT, Tim Prinzing wrote: >> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is >> null > > Tim Prinzing has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - Merge master > - fix test breakage from rename > - reformat test summary > - reformat test summary > - improve test name and remove experimental test code > - fix copyright date > - More changes from feedback. > >The javadoc is improved to reflect InvalidCallerException is thrown with >a caller that can't be assigned to a ClassLoader as well as a null >caller frame. Added a test IsParallelCapableBadCaller that uses >reflection hackery to create a case where called with an invalid caller >on the call stack. > - Changes from feedback. > >- Copyright dates fixed >- IllegalCallerException thrown for no caller frame, and associated >javadoc changes >- test changed to look for IllegalCallerException thrown. > - Merge branch 'master' into JDK-8281000 > - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Looks fine. - Marked as reviewed by bchristi (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7448
Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder
On Wed, 16 Feb 2022 21:19:04 GMT, Olga Mikhaltsova
wrote:
> This fix made equal processing of strings such as ""C:\\Program
> Files\\Git\\"" before and after JDK-8250568.
>
> For example, it's needed to execute the following command on Windows:
> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
> it's equal to:
> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs",
> ""C:\\Program Files\\Git\\"", "Test").start();`
>
> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as
> unquoted due to the condition added in JDK-8250568.
>
> private static String unQuote(String str) {
> ..
>if (str.endsWith("\\"")) {
> return str;// not properly quoted, treat as unquoted
> }
> ..
> }
>
>
> that leads to the additional surrounding by quotes in
> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true due
> to the space inside the string argument.
> As a result the native function CreateProcessW
> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly
> quoted argument:
>
> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
> (jdk.lang.Process.allowAmbiguousCommands = true)
> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\""
> Test
> (jdk.lang.Process.allowAmbiguousCommands = false)
>
>
> Obviously, a string ending with `"\\""` must not be started with `"""` to
> treat as unquoted overwise it’s should be treated as properly quoted.
The fix looks right.
The PR should include a test. Can you write a standalone test in
jdk/test/java/lang/ProcessBuilder/... to confirm the fix.
Possibly it could be added to Basic.java but that file is pretty large and
doesn't seem like the correct place to add.
It may be sufficient to invoke echo with that 3rd argument and verify the
output.
-
PR: https://git.openjdk.java.net/jdk/pull/7504
Re: RFR: 8279508: Auto-vectorize Math.round API [v2]
On 2/12/2022 6:55 PM, Jatin Bhateja wrote: On Fri, 21 Jan 2022 00:49:04 GMT, Sandhya Viswanathan wrote: The JVM currently initializes the x86 mxcsr to round to nearest even, see below in stubGenerator_x86_64.cpp: // Round to nearest (even), 64-bit mode, exceptions masked StubRoutines::x86::_mxcsr_std = 0x1F80; The above works for Math.rint which is specified to be round to nearest even. Please see: https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sdm.html : section 4.8.4 The rounding mode needed for Math.round is round to positive infinity which needs a different x86 mxcsr initialization(0x5F80). Hi @sviswa7 , As per JLS 17 section 15.4 Java follows round to nearest rounding policy for all floating point operations except conversion to integer and remainder where it uses round toward zero. That is a true background condition, but I will note that the Math.round method does independently define the semantics of its operation and rounding behavior, which has changed (slightly) over the lifetime of the platform. -Joe
Re: RFR: 8282019: Unused static fields DEGREES_TO_RADIANS, RADIANS_TO_DEGREES in StrictMath
On Wed, 16 Feb 2022 14:48:04 GMT, Andrey Turbanov wrote: > Couple of fields in StrictMath are unused since > [JDK-8244146](https://bugs.openjdk.java.net/browse/JDK-8244146). > Also I fixed incorrect javadoc for private method. Looks like typo in > [JDK-6282196](https://bugs.openjdk.java.net/browse/JDK-6282196) Marked as reviewed by darcy (Reviewer). Looks fine; thanks for the cleanup. - PR: https://git.openjdk.java.net/jdk/pull/7495
Integrated: JDK-8281671: Class.getCanonicalName spec should explicitly cover array classes
On Tue, 15 Feb 2022 22:23:36 GMT, Joe Darcy wrote: > Add explicit discussion of array types to Class.getCanonicalName, no change > in behavior. > > I didn't see any existing regression tests for the various "getFooName" > methods of Class so I created a new test file for a handful of test cases. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8281922 This pull request has now been integrated. Changeset: 5ec7898d Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/5ec7898dbf1ebe261e5e25939cad42134611ff12 Stats: 111 lines in 2 files changed: 111 ins; 0 del; 0 mod 8281671: Class.getCanonicalName spec should explicitly cover array classes Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk/pull/7483
Re: RFR: JDK-8281671: Class.getCanonicalName spec should explicitly cover array classes [v2]
On Wed, 16 Feb 2022 20:17:31 GMT, Mandy Chung wrote: >> Joe Darcy has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Augment test. > > test/jdk/java/lang/Class/NameTest.java line 27: > >> 25: * @test >> 26: * @bug 8281671 >> 27: * @summary Checks on various "getFooName" methods of java.lang.Class > > This tests only `getCanonicalName` method. Added analogous test cases for getSimpleName name. > test/jdk/java/lang/Class/NameTest.java line 56: > >> 54: expectedCanonicalName.put(int.class, "int"); >> 55: expectedCanonicalName.put(Object.class, >> "java.lang.Object"); >> 56: expectedCanonicalName.put(objectArray.getClass(), >> "java.lang.Object[]"); > > Just an observation: TestNG is a good option to clearly show the data > provider and the test body. Up to you. Noted; I'll stick with the hand-written code for now. - PR: https://git.openjdk.java.net/jdk/pull/7483
Re: RFR: 8281315: Unicode, (?i) flag and backreference throwing IndexOutOfBounds Exception
On Wed, 16 Feb 2022 21:00:00 GMT, Naoto Sato wrote:
>> This is a fix in the buggy way CIBackRef traverses unicode characters that
>> could be variable-length. Originally it followed the approach that BackRef
>> does, but failed to account for unicode characters that could be 2
>> chars-long. The upper bound (groupSize) for the traversing loop is set by
>> the difference between group start and stop indexes. This works for single
>> char characters and it also works for case-sensitive comparisons because
>> byte-by-byte comparisons are acceptable, but it doesn't work for a
>> comparison where some kind of normalization (i.e. case) is required. This
>> fix adjusts the upper bound for the loop that traverses the character when a
>> two-char character is encountered.
>>
>> An alternative was to check the length of the group size by scanning the
>> group in advance and converting to code points, but this could potentially
>> result in multiple scans and codepoint conversions of the same matcher group
>> which could be long. The solution that adjusts the loop bounds on the fly
>> avoids this case.
>
> src/java.base/share/classes/java/util/regex/Pattern.java line 5104:
>
>> 5102: j += Character.charCount(c2);
>> 5103:
>> 5104: if(xIncr > 1) {
>
> You can eliminate `xIncr` by comparing `c1 >=
> Character.MIN_SUPPLEMENTARY_CODE_POINT` here.
Nice! Thanks will do.
-
PR: https://git.openjdk.java.net/jdk/pull/7501
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]
On Wed, 16 Feb 2022 21:39:20 GMT, Tim Prinzing wrote: >> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is >> null > > Tim Prinzing has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - Merge master > - fix test breakage from rename > - reformat test summary > - reformat test summary > - improve test name and remove experimental test code > - fix copyright date > - More changes from feedback. > >The javadoc is improved to reflect InvalidCallerException is thrown with >a caller that can't be assigned to a ClassLoader as well as a null >caller frame. Added a test IsParallelCapableBadCaller that uses >reflection hackery to create a case where called with an invalid caller >on the call stack. > - Changes from feedback. > >- Copyright dates fixed >- IllegalCallerException thrown for no caller frame, and associated >javadoc changes >- test changed to look for IllegalCallerException thrown. > - Merge branch 'master' into JDK-8281000 > - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Looks good. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7448
Re: RFR: 8282019: Unused static fields DEGREES_TO_RADIANS, RADIANS_TO_DEGREES in StrictMath
On Wed, 16 Feb 2022 14:48:04 GMT, Andrey Turbanov wrote: > Couple of fields in StrictMath are unused since > [JDK-8244146](https://bugs.openjdk.java.net/browse/JDK-8244146). > Also I fixed incorrect javadoc for private method. Looks like typo in > [JDK-6282196](https://bugs.openjdk.java.net/browse/JDK-6282196) Approved assuming that there are no build errors. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7495
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Tim Prinzing has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits: - Merge master - fix test breakage from rename - reformat test summary - reformat test summary - improve test name and remove experimental test code - fix copyright date - More changes from feedback. The javadoc is improved to reflect InvalidCallerException is thrown with a caller that can't be assigned to a ClassLoader as well as a null caller frame. Added a test IsParallelCapableBadCaller that uses reflection hackery to create a case where called with an invalid caller on the call stack. - Changes from feedback. - Copyright dates fixed - IllegalCallerException thrown for no caller frame, and associated javadoc changes - test changed to look for IllegalCallerException thrown. - Merge branch 'master' into JDK-8281000 - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null - Changes: https://git.openjdk.java.net/jdk/pull/7448/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7448=06 Stats: 216 lines in 5 files changed: 216 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7448.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448 PR: https://git.openjdk.java.net/jdk/pull/7448
Re: Use copy_file_range system call for copying on Linux systems
On Feb 16, 2022, at 12:10 PM, Alan Bateman mailto:alan.bate...@oracle.com>> wrote: I suspect Ilya meant using syscall(2) with the NR. We had code in libnio that used approach for the *at functions before they were added to the glibc header files. It's a bit fragile as they are architecture specific. Oh, I see. It looks like the only place that is still used is in UnixNativeDispatcher.c for fstatat() on Linux. Brian
RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder
This fix made equal processing of strings such as ""C:\\Program Files\\Git\\""
before and after JDK-8250568.
For example, it's needed to execute the following command on Windows:
`C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
it's equal to:
`new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs",
""C:\\Program Files\\Git\\"", "Test").start();`
While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as
unquoted due to the condition added in JDK-8250568.
private static String unQuote(String str) {
..
if (str.endsWith("\\"")) {
return str;// not properly quoted, treat as unquoted
}
..
}
that leads to the additional surrounding by quotes in
ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true due
to the space inside the string argument.
As a result the native function CreateProcessW
(src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly
quoted argument:
pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
(jdk.lang.Process.allowAmbiguousCommands = true)
pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\""
Test
(jdk.lang.Process.allowAmbiguousCommands = false)
Obviously, a string ending with "\\"" must not be started with """ to treat as
unquoted overwise it’s should be treated as properly quoted.
-
Commit messages:
- 8282008: Incorrect handling of quoted arguments in ProcessBuilder
Changes: https://git.openjdk.java.net/jdk/pull/7504/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7504=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8282008
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/7504.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7504/head:pull/7504
PR: https://git.openjdk.java.net/jdk/pull/7504
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v6]
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: fix test breakage from rename - Changes: - all: https://git.openjdk.java.net/jdk/pull/7448/files - new: https://git.openjdk.java.net/jdk/pull/7448/files/a449acdc..977162db Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7448.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448 PR: https://git.openjdk.java.net/jdk/pull/7448
Re: RFR: JDK-8281671: Class.getCanonicalName spec should explicitly cover array classes [v2]
> Add explicit discussion of array types to Class.getCanonicalName, no change > in behavior. > > I didn't see any existing regression tests for the various "getFooName" > methods of Class so I created a new test file for a handful of test cases. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8281922 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Augment test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7483/files - new: https://git.openjdk.java.net/jdk/pull/7483/files/2b0729ad..b414e3ef Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7483=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7483=00-01 Stats: 31 lines in 1 file changed: 31 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7483.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7483/head:pull/7483 PR: https://git.openjdk.java.net/jdk/pull/7483
Re: RFR: 8281315: Unicode, (?i) flag and backreference throwing IndexOutOfBounds Exception
On Wed, 16 Feb 2022 18:45:29 GMT, Ian Graves wrote:
> This is a fix in the buggy way CIBackRef traverses unicode characters that
> could be variable-length. Originally it followed the approach that BackRef
> does, but failed to account for unicode characters that could be 2
> chars-long. The upper bound (groupSize) for the traversing loop is set by the
> difference between group start and stop indexes. This works for single char
> characters and it also works for case-sensitive comparisons because
> byte-by-byte comparisons are acceptable, but it doesn't work for a comparison
> where some kind of normalization (i.e. case) is required. This fix adjusts
> the upper bound for the loop that traverses the character when a two-char
> character is encountered.
>
> An alternative was to check the length of the group size by scanning the
> group in advance and converting to code points, but this could potentially
> result in multiple scans and codepoint conversions of the same matcher group
> which could be long. The solution that adjusts the loop bounds on the fly
> avoids this case.
Looks good with a minor suggestion.
src/java.base/share/classes/java/util/regex/Pattern.java line 5104:
> 5102: j += Character.charCount(c2);
> 5103:
> 5104: if(xIncr > 1) {
You can eliminate `xIncr` by comparing `c1 >=
Character.MIN_SUPPLEMENTARY_CODE_POINT` here.
-
PR: https://git.openjdk.java.net/jdk/pull/7501
RFR: 8282019: Unused static fields DEGREES_TO_RADIANS, RADIANS_TO_DEGREES in StrictMath
Couple of fields in StrictMath are unused since [JDK-8244146](https://bugs.openjdk.java.net/browse/JDK-8244146). Also I fixed incorrect javadoc for private method. Looks like typo in [JDK-6282196](https://bugs.openjdk.java.net/browse/JDK-6282196) - Commit messages: - [PATCH] Remove unused static fields 'DEGREES_TO_RADIANS', RADIANS_TO_DEGREES in StrictMath Changes: https://git.openjdk.java.net/jdk/pull/7495/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7495=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282019 Stats: 17 lines in 1 file changed: 0 ins; 13 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7495.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7495/head:pull/7495 PR: https://git.openjdk.java.net/jdk/pull/7495
RFR: 8282007: Assorted enhancements to jpackage testing framework
8282007: Assorted enhancements to jpackage testing framework - Commit messages: - Trailing whitespace removed - 8282007: Assorted enhancements to jpackage testing framework - macOS bugix - Don't use TKit.assertZZZ to validate test setup correctness. Throw exception instead. - Assorted fixes and enhancements to jpackage testing framework Changes: https://git.openjdk.java.net/jdk/pull/7502/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7502=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282007 Stats: 738 lines in 17 files changed: 413 ins; 180 del; 145 mod Patch: https://git.openjdk.java.net/jdk/pull/7502.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7502/head:pull/7502 PR: https://git.openjdk.java.net/jdk/pull/7502
Re: RFR: JDK-8281671: Class.getCanonicalName spec should explicitly cover array classes
On Tue, 15 Feb 2022 22:23:36 GMT, Joe Darcy wrote: > Add explicit discussion of array types to Class.getCanonicalName, no change > in behavior. > > I didn't see any existing regression tests for the various "getFooName" > methods of Class so I created a new test file for a handful of test cases. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8281922 Marked as reviewed by mchung (Reviewer). test/jdk/java/lang/Class/NameTest.java line 27: > 25: * @test > 26: * @bug 8281671 > 27: * @summary Checks on various "getFooName" methods of java.lang.Class This tests only `getCanonicalName` method. test/jdk/java/lang/Class/NameTest.java line 56: > 54: expectedCanonicalName.put(int.class, "int"); > 55: expectedCanonicalName.put(Object.class, > "java.lang.Object"); > 56: expectedCanonicalName.put(objectArray.getClass(), > "java.lang.Object[]"); Just an observation: TestNG is a good option to clearly show the data provider and the test body. Up to you. - PR: https://git.openjdk.java.net/jdk/pull/7483
Integrated: JDK-8281003 - MethodHandles::lookup throws NPE if caller is null
On Fri, 11 Feb 2022 20:32:46 GMT, Tim Prinzing wrote: > JDK-8281003 - MethodHandles::lookup throws NPE if caller is null This pull request has now been integrated. Changeset: 67763df4 Author:Tim Prinzing Committer: Mandy Chung URL: https://git.openjdk.java.net/jdk/commit/67763df4dce387da33da6d93d0f5d80e54cf8e5b Stats: 160 lines in 4 files changed: 158 ins; 0 del; 2 mod 8281003: MethodHandles::lookup throws NPE if caller is null Reviewed-by: ihse, mchung, jrose, alanb - PR: https://git.openjdk.java.net/jdk/pull/7447
Re: Use copy_file_range system call for copying on Linux systems
On 16/02/2022 20:03, Brian Burkhalter wrote: On Feb 16, 2022, at 9:51 AM, Ilya Starchenko wrote: I think we can just check if syscall return ENOSYS and if it’s true fail back to more generic code. The code would not even build without the syscall in the headers so I don’t think so. I suspect Ilya meant using syscall(2) with the NR. We had code in libnio that used approach for the *at functions before they were added to the glibc header files. It's a bit fragile as they are architecture specific. -Alan
Re: Use copy_file_range system call for copying on Linux systems
On Feb 16, 2022, at 9:51 AM, Ilya Starchenko mailto:redux1234...@mail.ru>> wrote: I think we can just check if syscall return ENOSYS and if it’s true fail back to more generic code. The code would not even build without the syscall in the headers so I don’t think so. Brian
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v14]
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with one additional commit since the last revision: fix test - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/3e8f2d7e..d0a4e6fa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=12-13 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v5]
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Tim Prinzing has updated the pull request incrementally with three additional commits since the last revision: - reformat test summary - reformat test summary - improve test name and remove experimental test code - Changes: - all: https://git.openjdk.java.net/jdk/pull/7448/files - new: https://git.openjdk.java.net/jdk/pull/7448/files/533a0660..a449acdc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=03-04 Stats: 129 lines in 3 files changed: 58 ins; 70 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7448.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448 PR: https://git.openjdk.java.net/jdk/pull/7448
RFR: 8281315: Unicode, (?i) flag and backreference throwing IndexOutOfBounds Exception
This is a fix in the buggy way CIBackRef traverses unicode characters that could be variable-length. Originally it followed the approach that BackRef does, but failed to account for unicode characters that could be 2 chars-long. The upper bound (groupSize) for the traversing loop is set by the difference between group start and stop indexes. This works for single char characters and it also works for case-sensitive comparisons because byte-by-byte comparisons are acceptable, but it doesn't work for a comparison where some kind of normalization (i.e. case) is required. This fix adjusts the upper bound for the loop that traverses the character when a two-char character is encountered. An alternative was to check the length of the group size by scanning the group in advance and converting to code points, but this could potentially result in multiple scans and codepoint conversions of the same matcher group which could be long. The solution that adjusts the loop bounds on the fly avoids this case. - Commit messages: - Adding test - Initial fix for IOOBE in CIBackRef Changes: https://git.openjdk.java.net/jdk/pull/7501/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7501=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281315 Stats: 26 lines in 2 files changed: 22 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7501.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7501/head:pull/7501 PR: https://git.openjdk.java.net/jdk/pull/7501
Re: RFR: JDK-8281003 - MethodHandles::lookup throws NPE if caller is null [v5]
On Wed, 16 Feb 2022 17:55:55 GMT, Tim Prinzing wrote: >> JDK-8281003 - MethodHandles::lookup throws NPE if caller is null > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > fix copyright date Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7447
Integrated: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library
On Thu, 10 Feb 2022 23:27:49 GMT, Mandy Chung wrote: > This patch removes the restriction in the raw library loading mechanism that > does not allow mix-n-match of loading a library as a JNI library and as a raw > library. > > The raw library loading mechanism is designed for panama to load native > library essentially equivalent to dlopen/dlclose calls independent of JNI > library loading. If a native library is loaded as a JNI library and a raw > library, it will get different NativeLibrary instances. When a class loader > is being unloaded, JNI_Unload will be invoked but the native library may not > be unloaded until NativeLibrary::unload is explicitly called for the raw > library. This pull request has now been integrated. Changeset: 980d1878 Author:Mandy Chung URL: https://git.openjdk.java.net/jdk/commit/980d18789139295c95ec6045539b68d1ae57bc31 Stats: 281 lines in 7 files changed: 183 ins; 66 del; 32 mod 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library Reviewed-by: sundar, mcimadamore - PR: https://git.openjdk.java.net/jdk/pull/7435
RFR: 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if light.exe is not in %PATH%
8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails if light.exe is not in %PATH% - Commit messages: - 8282011: test/jdk/tools/jpackage/windows/WinL10nTest.java test fails locally Changes: https://git.openjdk.java.net/jdk/pull/7500/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7500=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282011 Stats: 6 lines in 1 file changed: 3 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7500.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7500/head:pull/7500 PR: https://git.openjdk.java.net/jdk/pull/7500
Re: RFR: JDK-8281003 - MethodHandles::lookup throws NPE if caller is null [v5]
> JDK-8281003 - MethodHandles::lookup throws NPE if caller is null Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: fix copyright date - Changes: - all: https://git.openjdk.java.net/jdk/pull/7447/files - new: https://git.openjdk.java.net/jdk/pull/7447/files/7d1b97fe..ecfd125f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7447=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7447=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7447.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7447/head:pull/7447 PR: https://git.openjdk.java.net/jdk/pull/7447
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v4]
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: fix copyright date - Changes: - all: https://git.openjdk.java.net/jdk/pull/7448/files - new: https://git.openjdk.java.net/jdk/pull/7448/files/d67bbb16..533a0660 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7448.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448 PR: https://git.openjdk.java.net/jdk/pull/7448
Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v3]
On Wed, 19 Jan 2022 05:50:50 GMT, Ioi Lam wrote:
>> I don't really know this code well enough to do a good code review. I had
>> some comments though.
>
>> I don't really know this code well enough to do a good code review. I had
>> some comments though.
>
> Hi Coleen, thanks for taking a look.
>
> This PR has two major parts:
>
> 1. Check for inappropriate reference to static fields. This is mainly done in
> cdsHeapVerifier.cpp. These checks don't affect the contents of the CDS
> archive. They just print out warnings if problems are found.
> 2. Special initialization of enum classes. Essentially if any instance of an
> enum class `X` is archived, then `X::` will not be executed, and
> we'll take this path instead (in instanceKlass.cpp):
>
>
> // This is needed to ensure the consistency of the archived heap objects.
> if (has_archived_enum_objs()) {
> assert(is_shared(), "must be");
> bool initialized = HeapShared::initialize_enum_klass(this, CHECK);
> if (initialized) {
> return;
> }
> }
>
> Could you check if (2) is correct?
> @iklam This pull request has been inactive for more than 4 weeks and will be
> automatically closed if another 4 weeks passes without any activity. To avoid
> this, simply add a new comment to the pull request. Feel free to ask for
> assistance if you need help with progressing this pull request towards
> integration!
keepalive
-
PR: https://git.openjdk.java.net/jdk/pull/6653
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v3]
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is > null Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: More changes from feedback. The javadoc is improved to reflect InvalidCallerException is thrown with a caller that can't be assigned to a ClassLoader as well as a null caller frame. Added a test IsParallelCapableBadCaller that uses reflection hackery to create a case where called with an invalid caller on the call stack. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7448/files - new: https://git.openjdk.java.net/jdk/pull/7448/files/fa66af15..d67bbb16 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=01-02 Stats: 85 lines in 3 files changed: 74 ins; 5 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/7448.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448 PR: https://git.openjdk.java.net/jdk/pull/7448
Integrated: 8281874: Can't unpack msi installers from test/jdk/tools/jpackage/windows/test/jdk/tools/jpackage/windows/WinShortcutPromptTest.java test
On Tue, 15 Feb 2022 22:56:35 GMT, Alexey Semenyuk wrote: > 8281874: Can't unpack msi installers from > test/jdk/tools/jpackage/windows/test/jdk/tools/jpackage/windows/WinShortcutPromptTest.java > test This pull request has now been integrated. Changeset: 81645521 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/81645521c81c7363d199e5051d51043146058a91 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod 8281874: Can't unpack msi installers from test/jdk/tools/jpackage/windows/test/jdk/tools/jpackage/windows/WinShortcutPromptTest.java test Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/7484
Integrated: 8281170: Test jdk/tools/jpackage/windows/WinInstallerIconTest always fails on Windows 11
On Tue, 15 Feb 2022 17:44:24 GMT, Alexey Semenyuk wrote: > Code clean up. Remove obsolete comments, unused imports, and unneeded jtreg > test parameter. > Fix the code to handle the case when installers are not created and there is > nothing to verify in the test. This pull request has now been integrated. Changeset: bb4dece2 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/bb4dece246a56f2b225089c331e9f3d092dfbfa1 Stats: 46 lines in 1 file changed: 12 ins; 19 del; 15 mod 8281170: Test jdk/tools/jpackage/windows/WinInstallerIconTest always fails on Windows 11 Reviewed-by: almatvee - PR: https://git.openjdk.java.net/jdk/pull/7481
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v13]
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with one additional commit since the last revision: fix test - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/b191bd6a..3e8f2d7e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=11-12 Stats: 5 lines in 1 file changed: 2 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v12]
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with one additional commit since the last revision: fix test - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/f37f8d2a..b191bd6a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=10-11 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Integrated: 8176706: Additional Date-Time Formats
On Thu, 3 Feb 2022 23:29:54 GMT, Naoto Sato wrote: > Following the prior discussion [1], here is the PR for the subject > enhancement. CSR has also been updated according to the suggestion. > > [1] > https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html This pull request has now been integrated. Changeset: 9b74c3f2 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/9b74c3f2e74a4efdec1c1488e96ab5939a408df0 Stats: 821 lines in 12 files changed: 725 ins; 12 del; 84 mod 8176706: Additional Date-Time Formats Reviewed-by: joehw, rriggs - PR: https://git.openjdk.java.net/jdk/pull/7340
Re: Use copy_file_range system call for copying on Linux systems
On Feb 16, 2022, at 5:40 AM, Alan Bateman mailto:alan.bate...@oracle.com>> wrote: On 16/02/2022 13:13, Ilya Starchenko wrote: I have suggestion. Copy_file_range() has been introduced in the Linux kernel since version 4.5. system call performs an in-kernel copy between two file descriptors without the additional cost of transferring data from the kernel to user space and then back into the kernel, including NFS(It will have huge performance impact). It copies up to len bytes of data from the source file descriptor fd_in to the target file descriptor fd_out, overwriting any data that exists within the requested range of the target file. Currently we use sendfile, but I think we need to use copy_file_range when possible. What do you think? There was brief discussion about this on nio-dev in April 2021. As I recall, there was an issue with the function prototype not being available (I can't recall if it was system include files or glibc). It might be best to start a new discussion there as it may be time to look at it again. Please refer to [1]. The system call copy_file_range() is newer than the version of the Linux headers used to build the JDK. It might be that we could use dlsym() to be able to leverage it where it is available but that would require further investigation. Brian [1] https://bugs.openjdk.java.net/browse/JDK-8264744
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v11]
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with one additional commit since the last revision: fix test - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/e8bfa526..f37f8d2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=09-10 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v10]
> 8281631: HashMap copy constructor and putAll can over-allocate table XenoAmess has updated the pull request incrementally with two additional commits since the last revision: - revert changes in ConcurrentHashMap - 9072610: HashMap copy constructor and putAll can over-allocate table - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/6d9069da..e8bfa526 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=08-09 Stats: 9 lines in 2 files changed: 0 ins; 2 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v9]
On Tue, 15 Feb 2022 18:23:51 GMT, XenoAmess wrote: >> 8281631: HashMap copy constructor and putAll can over-allocate table > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > 9072610: HashMap copy constructor and putAll can over-allocate table ConcurrentHashMap is a little complicated... Would like to put it into another pr. Let's focus on non-thread-safe Maps in this pr. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: Use copy_file_range system call for copying on Linux systems
On 16/02/2022 13:13, Ilya Starchenko wrote: I have suggestion. Copy_file_range() has been introduced in the Linux kernel since version 4.5. system call performs an in-kernel copy between two file descriptors without the additional cost of transferring data from the kernel to user space and then back into the kernel, including NFS(It will have huge performance impact). It copies up to len bytes of data from the source file descriptor fd_in to the target file descriptor fd_out, overwriting any data that exists within the requested range of the target file. Currently we use sendfile, but I think we need to use copy_file_range when possible. What do you think? There was brief discussion about this on nio-dev in April 2021. As I recall, there was an issue with the function prototype not being available (I can't recall if it was system include files or glibc). It might be best to start a new discussion there as it may be time to look at it again. -Alan
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v5]
On Tue, 15 Feb 2022 21:09:57 GMT, Joe Darcy wrote:
>> This is an early review of changes to better model JVM access flags, that is
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>>
>> Language level modifiers and JVM level access flags are closely related, but
>> distinct. There are concepts that overlap in the two domains (public,
>> private, etc.), others that only have a language-level modifier (sealed),
>> and still others that only have an access flag (synthetic).
>>
>> The existing java.lang.reflect.Modifier class is inadequate to model these
>> subtleties. For example, the bit positions used by access flags on different
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for
>> methods. Just having a raw integer does not provide sufficient context to
>> decode the corresponding language-level string. Methods like
>> Modifier.methodModifiers() were introduced to cope with this situation.
>>
>> With additional modifiers and flags on the horizon with projects like
>> Valhalla, addressing the existent modeling deficiency now ahead of time is
>> reasonable before further strain is introduced.
>>
>> This PR in its current form is meant to give the overall shape of the API.
>> It is missing implementations to map from, say, method modifiers to access
>> flags, taking into account overlaps in bit positions.
>>
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in
>> once the API is further along.
>
> Joe Darcy has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains seven additional commits since
> the last revision:
>
> - Reorder constants by mask value per review feedback.
> - Merge branch 'master' into JDK-8266670
> - Respond to more review feedback.
> - Respond to review feedback explicitly stating returned sets are immutable.
> - Fix typo from review feedback.
> - Merge branch 'master' into JDK-8266670
> - JDK-8266670: Better modeling of modifiers in core reflection
test/jdk/java/lang/reflect/AccessFlag/BasicAccessFlagTest.java line 65:
> 63: if (left.mask() > right.mask()) {
> 64: throw new RuntimeException(left
> 65:+ "has a greater mas than "
Typo:
Suggestion:
+ "has a greater mask than "
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v4]
On Tue, 15 Feb 2022 19:59:43 GMT, Mandy Chung wrote: >> This patch removes the restriction in the raw library loading mechanism that >> does not allow mix-n-match of loading a library as a JNI library and as a >> raw library. >> >> The raw library loading mechanism is designed for panama to load native >> library essentially equivalent to dlopen/dlclose calls independent of JNI >> library loading. If a native library is loaded as a JNI library and a raw >> library, it will get different NativeLibrary instances. When a class loader >> is being unloaded, JNI_Unload will be invoked but the native library may not >> be unloaded until NativeLibrary::unload is explicitly called for the raw >> library. > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > No need for explicit locking for raw loading Looks good! Thanks for the changes! - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7435
Re: RFR: 8279508: Auto-vectorize Math.round API [v3]
On Wed, 16 Feb 2022 12:26:45 GMT, Jatin Bhateja wrote: >>> > Hi, IIRC for evex encoding you can embed the RC control bit directly in >>> > the evex prefix, removing the need to rely on global MXCSR register. >>> > Thanks. >>> >>> Hi @merykitty , You are correct, we can embed RC mode in instruction >>> encoding of round instruction (towards -inf,+inf, zero). But to match the >>> semantics of Math.round API one needs to add 0.5[f] to input value and then >>> perform rounding over resultant value, which is why @sviswa7 suggested to >>> use a global rounding mode driven by MXCSR.RC so that intermediate floating >>> inexact values are resolved as desired, but OOO execution may misplace >>> LDMXCSR and hence may have undesired side effects. >> >> **Just want to correct above statement, LDMXCSR will not be >> re-ordered/re-scheduled early OOO backend.** > >> That pseudocode would make a very useful comment too. This whole patch is >> very thinly commented. > > I have replaced earlier bulky sequence, new sequence is having similar > performance but reduction in code may improve inlining behavior. Added > descriptive comments around the special cases. > There are already `RoundFloat`, `RoundDouble`, and `RoundDoubleMode` nodes > defined. > > Though `RoundFloat` and `RoundDouble` are legacy nodes used only on x86-32, > `RoundDoubleMode` supports multiple rounding modes and is amenable to > auto-vectorization. > > What do you think about the following alternative? > > Reuse `RoundDoubleMode` (with a new rounding mode) and introduce > `RoundFloatMode`. > > Special rounding rules is not the only peculiarity of `Math.round()`. It also > converts the result to an integral type. It can be represented as `ConvF2I > (RoundFloatMode f #rmode)` / `ConvD2L (RoundDoubleMode d #rmode)`. In scalar > case, it can be matched as a single AD instruction. > > Auto-vectorizer can then convert it to `VectorCastF2X (RoundFloatModeV vf > #rmode)` / `VectorCastD2X (RoundDoubleModeV vd #rmode)` and match it in a > similar manner. Adding new rounding mode to RoundDoubleMode may disturb other targets. match_rule_supported routine operates over Opcodes and currently any target supporting RoundDoubleMode generates code for all the rounding modes. Your solution is anyways based on creating new scalar and vector IR node for floating point rounding operation, which is what patch is doing currently. - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: 8279508: Auto-vectorize Math.round API [v5]
> Summary of changes: > - Intrinsify Math.round(float) and Math.round(double) APIs. > - Extend auto-vectorizer to infer vector operations on encountering scalar IR > nodes for above intrinsics. > - Test creation using new IR testing framework. > > Following are the performance number of a JMH micro included with the patch > > Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server) > > > Benchmark | TESTSIZE | Baseline AVX3 (ops/ms) | Withopt AVX3 (ops/ms) | Gain > ratio | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain ratio > -- | -- | -- | -- | -- | -- | -- | -- > FpRoundingBenchmark.test_round_double | 1024.00 | 584.99 | 1870.70 | 3.20 | > 510.35 | 548.60 | 1.07 > FpRoundingBenchmark.test_round_double | 2048.00 | 257.17 | 965.33 | 3.75 | > 293.60 | 273.15 | 0.93 > FpRoundingBenchmark.test_round_float | 1024.00 | 825.69 | 3592.54 | 4.35 | > 825.32 | 1836.42 | 2.23 > FpRoundingBenchmark.test_round_float | 2048.00 | 388.55 | 1895.77 | 4.88 | > 412.31 | 945.82 | 2.29 > > > Kindly review and share your feedback. > > Best Regards, > Jatin Jatin Bhateja has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - 8279508: Adding few descriptive comments. - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8279508 - 8279508: Replacing by efficient instruction sequence based on MXCSR.RC mode. - 8279508: Adding vectorized algorithms to match the semantics of rounding operations. - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8279508 - 8279508: Adding a test for scalar intrinsification. - 8279508: Auto-vectorize Math.round API - Changes: https://git.openjdk.java.net/jdk/pull/7094/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7094=04 Stats: 739 lines in 23 files changed: 648 ins; 29 del; 62 mod Patch: https://git.openjdk.java.net/jdk/pull/7094.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7094/head:pull/7094 PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: 8279508: Auto-vectorize Math.round API [v3]
On Mon, 14 Feb 2022 17:14:10 GMT, Jatin Bhateja wrote: >> That pseudocode would make a very useful comment too. This whole patch is >> very thinly commented. > >> > Hi, IIRC for evex encoding you can embed the RC control bit directly in >> > the evex prefix, removing the need to rely on global MXCSR register. >> > Thanks. >> >> Hi @merykitty , You are correct, we can embed RC mode in instruction >> encoding of round instruction (towards -inf,+inf, zero). But to match the >> semantics of Math.round API one needs to add 0.5[f] to input value and then >> perform rounding over resultant value, which is why @sviswa7 suggested to >> use a global rounding mode driven by MXCSR.RC so that intermediate floating >> inexact values are resolved as desired, but OOO execution may misplace >> LDMXCSR and hence may have undesired side effects. > > **Just want to correct above statement, LDMXCSR will not be > re-ordered/re-scheduled early OOO backend.** > That pseudocode would make a very useful comment too. This whole patch is > very thinly commented. I have replaced earlier bulky sequence, new sequence is having similar performance but reduction in code may improve inlining behavior. Added descriptive comments around the special cases. - PR: https://git.openjdk.java.net/jdk/pull/7094
Re: RFR: JDK-8281462: Annotation toString output for enum not reusable for source input [v3]
On Fri, 11 Feb 2022 20:49:33 GMT, Joe Darcy wrote: >> Thanks, Joe. >> >> Regarding the RFE, do you plan to open an issue to address the documentation >> for `getCanonicalName`? > > Filed https://bugs.openjdk.java.net/browse/JDK-8281671 Thanks for filing that. - PR: https://git.openjdk.java.net/jdk/pull/7418
Re: RFR: 8279508: Auto-vectorize Math.round API [v4]
> Summary of changes: > - Intrinsify Math.round(float) and Math.round(double) APIs. > - Extend auto-vectorizer to infer vector operations on encountering scalar IR > nodes for above intrinsics. > - Test creation using new IR testing framework. > > Following are the performance number of a JMH micro included with the patch > > Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server) > > > Benchmark | TESTSIZE | Baseline AVX3 (ops/ms) | Withopt AVX3 (ops/ms) | Gain > ratio | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain ratio > -- | -- | -- | -- | -- | -- | -- | -- > FpRoundingBenchmark.test_round_double | 1024.00 | 584.99 | 1870.70 | 3.20 | > 510.35 | 548.60 | 1.07 > FpRoundingBenchmark.test_round_double | 2048.00 | 257.17 | 965.33 | 3.75 | > 293.60 | 273.15 | 0.93 > FpRoundingBenchmark.test_round_float | 1024.00 | 825.69 | 3592.54 | 4.35 | > 825.32 | 1836.42 | 2.23 > FpRoundingBenchmark.test_round_float | 2048.00 | 388.55 | 1895.77 | 4.88 | > 412.31 | 945.82 | 2.29 > > > Kindly review and share your feedback. > > Best Regards, > Jatin Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision: 8279508: Replacing by efficient instruction sequence based on MXCSR.RC mode. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7094/files - new: https://git.openjdk.java.net/jdk/pull/7094/files/2dc364fa..1c9ff777 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7094=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7094=02-03 Stats: 143 lines in 4 files changed: 4 ins; 82 del; 57 mod Patch: https://git.openjdk.java.net/jdk/pull/7094.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7094/head:pull/7094 PR: https://git.openjdk.java.net/jdk/pull/7094
RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream
Currently, `InflaterInputStream::read()` first does a native call to the
underlying zlib `inflate()` function and only afterwards checks if the inflater
requires input (i.e. `Inflater::needsInput()`) or has finished inflating
(`Inflater::finished()`). This leads to an unnecessary native call to
`inflate()` when `InflaterInputStream::read()` is invoked for the very first
time because `Inflater::fill()` hasn't been called and another unnecessary
native call to detect the end of the stream. For small streams/files which
completely fit into the output buffer passed to `InflaterInputStream::read()`
we can therefore save two out of three native calls. The attached micro
benchmark shows that this results in a 5%-10% performance improvement for zip
files of sizes between 4096 to 256 bytes (notice that in common jars like
Tomcat, Spring-Boot, Maven, Jackson, etc. about 60-80% of the classes are
smaller than 4096 bytes).
before JDK-8281962
--
Benchmark (size) Mode Cnt Score Error
Units
InflaterInputStreams.inflaterInputStreamRead 256 avgt5 2.571 ± 0.120
us/op
InflaterInputStreams.inflaterInputStreamRead 512 avgt5 2.861 ± 0.064
us/op
InflaterInputStreams.inflaterInputStreamRead4096 avgt5 5.110 ± 0.278
us/op
after JDK-8281962
-
Benchmark (size) Mode Cnt Score Error
Units
InflaterInputStreams.inflaterInputStreamRead 256 avgt5 2.332 ± 0.081
us/op
InflaterInputStreams.inflaterInputStreamRead 512 avgt5 2.691 ± 0.293
us/op
InflaterInputStreams.inflaterInputStreamRead4096 avgt5 4.812 ± 1.038
us/op
Tested with the JTreg zip/jar/zipfs and the JCK zip/jar tests.
As a side effect, this change also fixes an issue with alternative zlib
implementations like zlib-Chromium or zlib-Cloudflare which pad the inflated
bytes with a specif byte pattern at the end of the output for debugging
purpose. This breaks code patterns like the following:
int readcount = 0;
while ((bytesRead = inflaterInputStream.read(data, 0, bufferSize)) != -1) {
outputStream.write(data, 0, bytesRead);
readCount++;
}
if (readCount == 1) {
return data; // < first bytes might be overwritten
}
return outputStream.toByteArray();
Even if the whole data fits into the `data` array during the first call to
`inflaterInputStream.read()`, we still need a second call to
`inflaterInputStream.read()` in order to detect the end of the inflater stream.
If this second call calls into the native `inflate()` function of
Cloudflare/Chromium's zlib version, this will still write some padding bytes at
the beginning of the `data` array and overwrite the previously read data. This
issue has been reported in Spring [1] and ASM [2] when using these libraries
with Cloudflares zlib version (by setting `LD_LIBRARY_PATH` accordingly).
[1] https://github.com/spring-projects/spring-framework/issues/27429
[2] https://gitlab.ow2.org/asm/asm/-/issues/317955
-
Commit messages:
- 8281962: Avoid unnecessary native calls in InflaterInputStream
Changes: https://git.openjdk.java.net/jdk/pull/7492/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7492=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8281962
Stats: 114 lines in 2 files changed: 112 ins; 0 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/7492.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7492/head:pull/7492
PR: https://git.openjdk.java.net/jdk/pull/7492
