Re: RFR: 8285405: add test and check for negative argument to HashMap::newHashMap et al
On Mon, 6 Jun 2022 06:57:23 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which addresses > https://bugs.openjdk.java.net/browse/JDK-8285405? > > I've added the test for `LinkedHashMap.newLinkedHashMap(int)` in the existing > `test/jdk/java/util/LinkedHashMap/Basic.java` since that test has tests for > various APIs of this class. > > For `WeakHashMap.newWeakHashMap` and `HashMap.newHashMap`, I have created new > test classes under relevant locations, since these classes already have test > classes (almost) per API/feature in those locations. Hello Stuart, I'll include tests for those in this PR shortly. - PR: https://git.openjdk.org/jdk/pull/9036
RFR: 8287917: System.loadLibrary does not work on Big Sur if JDK is built with macOS SDK 10.15 and earlier
Please review this PR. SDK 10.15 and earlier reports os.version as 10.16 on Big Sur. This fix will check if dynamic linker support, which is supported from Big Sur, is available or not on the OS even if os.version is reported as 10.16 instead of 11. The os.version 10.16 doesn't include the update version like y of 10.x.y. Hence the change only see if it is 10.16 or not. - Commit messages: - Remove trailing whtiespace - Move print-debug to the top - Print os.version for logging purpose - 8287917: System.loadLibrary does not work on Big Sur if JDK is built with macOS SDK 10.15 and earlier Changes: https://git.openjdk.org/jdk/pull/9077/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9077=00 Issue: https://bugs.openjdk.org/browse/JDK-8287917 Stats: 5 lines in 2 files changed: 3 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/9077.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9077/head:pull/9077 PR: https://git.openjdk.org/jdk/pull/9077
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long
On Wed, 8 Jun 2022 09:39:04 GMT, Quan Anh Mai wrote: >> Hi, >> >> This patch implements intrinsics for `Integer/Long::compareUnsigned` using >> the same approach as the JVM does for long and floating-point comparisons. >> This allows efficient and reliable usage of unsigned comparison in Java, >> which is a basic operation and is important for range checks such as >> discussed in #8620 . >> >> Thank you very much. > > I have added a benchmark for the intrinsic. The result is as follows, thanks > a lot: > > Before After > Benchmark (size) Mode Cnt Score Error Score Error > Units > Integers.compareUnsigned 500 avgt 15 0.527 ± 0.002 0.498 ± 0.011 > us/op > Longs.compareUnsigned500 avgt 15 0.677 ± 0.014 0.561 ± 0.006 > us/op @merykitty Could you please also add the micro benchmark where compareUnsigned result is stored directly in an integer and show the performance of that? - PR: https://git.openjdk.org/jdk/pull/9068
Re: RFR: 8285405: add test and check for negative argument to HashMap::newHashMap et al
On Mon, 6 Jun 2022 06:57:23 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which addresses > https://bugs.openjdk.java.net/browse/JDK-8285405? > > I've added the test for `LinkedHashMap.newLinkedHashMap(int)` in the existing > `test/jdk/java/util/LinkedHashMap/Basic.java` since that test has tests for > various APIs of this class. > > For `WeakHashMap.newWeakHashMap` and `HashMap.newHashMap`, I have created new > test classes under relevant locations, since these classes already have test > classes (almost) per API/feature in those locations. Note that I've integrated [JDK-8284780](https://bugs.openjdk.org/browse/JDK-8284780) (HashSet static factories) so maybe this PR could be updated with tests for those as well. - PR: https://git.openjdk.org/jdk/pull/9036
Re: RFR: 8288140: Avoid redundant Hashtable.get call in Signal.handle
On Thu, 9 Jun 2022 07:35:43 GMT, Andrey Turbanov wrote:
> https://github.com/openjdk/jdk/blob/bc28baeba9360991e9b7575e1fbe178d873ccfc1/src/java.base/share/classes/jdk/internal/misc/Signal.java#L177-L178
>
> Instead of separate Hashtable.get/remove calls we can just use value returned
> by `remove`,
> It results in cleaner and a bit faster code.
src/java.base/share/classes/jdk/internal/misc/Signal.java line 180:
> 178: if (newH == 2) {
> 179: handlers.put(sig, handler);
> 180: }
If you made this change instead:
Suggestion:
Signal.Handler oldHandler;
if (newH == 2) {
oldHandler = handlers.replace(sig, handler);
} else {
oldHandler = handlers.remove(sig);
}
Wouldn't you be able to remove the entire `synchronized` block?
-
PR: https://git.openjdk.org/jdk/pull/9100
RFR: 8288140: Avoid redundant Hashtable.get call in Signal.handle
https://github.com/openjdk/jdk/blob/bc28baeba9360991e9b7575e1fbe178d873ccfc1/src/java.base/share/classes/jdk/internal/misc/Signal.java#L177-L178 Instead of separate Hashtable.get/remove calls we can just use value returned by `remove`, It results in cleaner and a bit faster code. - Commit messages: - [PATCH] Avoid redundant Hashtable.get call in Signal.handle - [PATCH] Avoid redundant Hashtable.get call in Signal.handle Changes: https://git.openjdk.org/jdk/pull/9100/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9100=00 Issue: https://bugs.openjdk.org/browse/JDK-8288140 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/9100.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9100/head:pull/9100 PR: https://git.openjdk.org/jdk/pull/9100
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v5]
> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter 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: - Merge - 6478546: Add break in write loop on ExceptionOccurred - Merge - 6478546: Clean up io_util.c - Merge - 6478546: Decrease malloc'ed buffer maximum size to 64 kB - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available - Changes: - all: https://git.openjdk.org/jdk/pull/8235/files - new: https://git.openjdk.org/jdk/pull/8235/files/10f14bb3..00521485 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8235=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8235=03-04 Stats: 118890 lines in 1877 files changed: 57455 ins; 51098 del; 10337 mod Patch: https://git.openjdk.org/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.org/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.org/jdk/pull/8235
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers [v2]
On Thu, 9 Jun 2022 16:04:43 GMT, Claes Redestad wrote: >> To take optimal advantage of the pre-existing optimization for repeated >> filters we could split the application of different types of stringifiers. >> >> The resulting difference in order of evaluation is not observable by >> conventional means since all reference type share the same object >> stringifier, and the others are filtering primitives (floats and doubles) >> which have been passed by value already. >> >> This change neutral on many concatenation expression shapes, but for any >> complex expressions with interleaving float/double and reference parameters >> it brings a straightforward reduction in rebinds and underlying LFs >> generated. For example on the >> [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) >> test there's a modest 2% reduction in total classes loaded with this change >> (from 16209 to 15872) > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Improve comments based on review feedback Looks good to me. Like Jorne's observation, it'd benefit everyone if this could generate just a single new lambda form multiple filters, not just for repeated filters if feasible. Maybe something to explore more in the future with a tradeoff of complexity. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.org/jdk/pull/9082
Integrated: JDK-8284858: Start of release updates for JDK 20
On Thu, 14 Apr 2022 05:09:14 GMT, Joe Darcy wrote: > Time to start getting ready for JDK 20... This pull request has now been integrated. Changeset: edff51e5 Author:Joe Darcy Committer: Erik Joelsson URL: https://git.openjdk.org/jdk/commit/edff51e5fdb5282830ecfb3792a88c7b28ca6557 Stats: 6453 lines in 69 files changed: 6403 ins; 5 del; 45 mod 8284858: Start of release updates for JDK 20 8286035: Add source 20 and target 20 to javac 8286034: Add SourceVersion.RELEASE_20 Reviewed-by: dholmes, kcr, iris, erikj, jjg, ihse - PR: https://git.openjdk.org/jdk/pull/8236
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers [v2]
> To take optimal advantage of the pre-existing optimization for repeated > filters we could split the application of different types of stringifiers. > > The resulting difference in order of evaluation is not observable by > conventional means since all reference type share the same object > stringifier, and the others are filtering primitives (floats and doubles) > which have been passed by value already. > > This change neutral on many concatenation expression shapes, but for any > complex expressions with interleaving float/double and reference parameters > it brings a straightforward reduction in rebinds and underlying LFs > generated. For example on the > [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) > test there's a modest 2% reduction in total classes loaded with this change > (from 16209 to 15872) Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Improve comments based on review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/9082/files - new: https://git.openjdk.org/jdk/pull/9082/files/51c841e8..4b7c696e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9082=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9082=00-01 Stats: 11 lines in 1 file changed: 7 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/9082.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9082/head:pull/9082 PR: https://git.openjdk.org/jdk/pull/9082
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v7]
On Tue, 7 Jun 2022 14:15:55 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > Gaurav Chaudhari has updated the pull request incrementally with one > additional commit since the last revision: > > 8285838: Simplified Daylight period checks for test. Looks good. Thanks for the contribution. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.org/jdk/pull/8661
Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
On Tue, 7 Jun 2022 12:19:29 GMT, Magnus Ihse Bursie wrote: > The test > `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` > verifies different failure modes of resource bundles. One of the failures is > that the runner class, `MissingResourceCauseTestRun.java`, creates a file > `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the > test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file is > removed. > > This does not work reliably on Windows. On msys2, `chmod` is essentially a > no-op, so the file is not made unreadable, and hence the test fails. In my > personal cygwin test environment, the chmod command does have some effect, > but it is still not enough to make the file unreadable, and so the test fails. > > The test was originally a shell script test that got converted to Java in > [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original > shell script code explicitly excluded Windows from testing. This was changed > in the rewrite, for reasons I cannot determine. > > What suprises me, though, is the "how can this ever has worked???" factor. > Apparently the test passes on the current Cygwin setup on GHA. I have failed > to reproduce the working conditions that makes a file actually unreadable for > the owner on Windows, neither on my GHA test repo, nor locally. I've searched > the web to figure out how to properly set file permissions on Windows to make > the file unreadable, using Windows native tools, to no avail. I've even asked > a [Stack Overflow > question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows); > which as of yet is still unanswered. > > Since I feel I've spent far more time than reasonable trying to get this to > work properly, I suggest we instead skip the unreadable test on Windows. It > is clearly unstable and highly depending on the Windows environment, the test > was never originally supported or intended for Windows, and at the of the > day, testing file unreadability is not an important regression test for > [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216). Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9061
Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
On Wed, 8 Jun 2022 17:43:49 GMT, Naoto Sato wrote: >> The test >> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` >> verifies different failure modes of resource bundles. One of the failures is >> that the runner class, `MissingResourceCauseTestRun.java`, creates a file >> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the >> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file >> is removed. >> >> This does not work reliably on Windows. On msys2, `chmod` is essentially a >> no-op, so the file is not made unreadable, and hence the test fails. In my >> personal cygwin test environment, the chmod command does have some effect, >> but it is still not enough to make the file unreadable, and so the test >> fails. >> >> The test was originally a shell script test that got converted to Java in >> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original >> shell script code explicitly excluded Windows from testing. This was changed >> in the rewrite, for reasons I cannot determine. >> >> What suprises me, though, is the "how can this ever has worked???" factor. >> Apparently the test passes on the current Cygwin setup on GHA. I have failed >> to reproduce the working conditions that makes a file actually unreadable >> for the owner on Windows, neither on my GHA test repo, nor locally. I've >> searched the web to figure out how to properly set file permissions on >> Windows to make the file unreadable, using Windows native tools, to no >> avail. I've even asked a [Stack Overflow >> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows); >> which as of yet is still unanswered. >> >> Since I feel I've spent far more time than reasonable trying to get this to >> work properly, I suggest we instead skip the unreadable test on Windows. It >> is clearly unstable and highly depending on the Windows environment, the >> test was never originally supported or intended for Windows, and at the of >> the day, testing file unreadability is not an important regression test for >> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216). > > Thanks for the investigation, Magnus. I believe it is a bug to include the > Windows environment in refactoring the shell script to Java. Why it's been > working? No idea. Could be `IOException` has been thrown not by `unreadable` > but for some other reason? > @naotoj Yes, that could be a likely possibility. Perhaps the file were never > created, or not in the expected place? I'd guess something like that too. Anyways, eliminating this unreliability in test is a good clean up. Approving the fix assuming Jai's comment is addressed. - PR: https://git.openjdk.org/jdk/pull/9061
Re: Add java.util.Objects.dump to facilitate quick debugging
My main concerns are about security, that many object details, while
useful to viewers, should not be accessible by any code under any
circumstances. Having any code able to print all field content of any
object sounds extremely dangerous.
In my opinion, we can use a MethodHandles.Lookup as a permission
control; the dump may print fields that the lookup context (obtainable
by caller with MethodHandles.lookup()) may access, including ones
accessible through reflection (e.g. open packages).
On Thu, Jun 9, 2022 at 1:07 AM Yi Yang wrote:
>
> Is it helpful to add a new Objects.dump method for quick debugging? Not in
> all cases
> we can open IDEA and happily use jdb to debug our code, sometimes we may just
> want
> to write a simple script, or work without IDE, for these cases, we can use
> Objects. dump
> to output field details of the object and feed back to the developer, so that
> they can quickly
> modify the code and continue to develop. Another useful scenario is working
> within jshell,
> Objects.dump can visualize object details, which is especially useful for
> REPL environments.
>
> It looks like var_dump() in PHP[1] :
> ```
> Person p = new Person(...)
> Objects.dump(p)
> Person = {
> name = "..."
> age = 24
> height = 1.75f
> attr = {
> ...
> }
> }
> ```
> In addition, we can also add Config parameters to allow users to control
> output content and
> format they expect, such as
> ```
> config.pretty = true
> config.maxDepth = 5
> config.dumpParent = true
> config.fieldFilter (Field f) -> ...
> Objects.dump(obj, config)
> ```
> This is just a very rough idea, it's too early to discuss implementation
> details at this point,
> I'm looking forward to hearing more feedback about the idea of adding
> Objects.dump itself.
>
> Thanks.
>
> [1] https://www.php.net/manual/en/function.var-dump.php
Integrated: 8287903: Reduce runtime of java.math microbenchmarks
On Tue, 7 Jun 2022 12:34:25 GMT, Claes Redestad wrote: > - Reduce runtime by running fewer forks, fewer iterations, less warmup. All > micros tested in this group appear to stabilize very quickly. > - Refactor BigIntegers to avoid re-running some (most) micros over and over > with parameter values that don't affect them. > > Expected runtime down from 14 hours to 15 minutes. This pull request has now been integrated. Changeset: 7e948f7c Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/7e948f7ccbb4b9be04f5ecb65cc8dd72e3b495f4 Stats: 104 lines in 4 files changed: 66 ins; 33 del; 5 mod 8287903: Reduce runtime of java.math microbenchmarks Reviewed-by: ecaspole, aph - PR: https://git.openjdk.java.net/jdk/pull/9062
Re: RFR: 8287903: Reduce runtime of java.math microbenchmarks
On Tue, 7 Jun 2022 12:34:25 GMT, Claes Redestad wrote: > - Reduce runtime by running fewer forks, fewer iterations, less warmup. All > micros tested in this group appear to stabilize very quickly. > - Refactor BigIntegers to avoid re-running some (most) micros over and over > with parameter values that don't affect them. > > Expected runtime down from 14 hours to 15 minutes. Thanks for reviews! - PR: https://git.openjdk.java.net/jdk/pull/9062
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v7]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Update author - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/7362da92..8544abd9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=05-06 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
RFR: JDK-8288094: cleanup old _MSC_VER handling
We still handle at a number of places ancient historic _MSC_VER versions of
Visual Studio releases e.g. pre VS2013 (VS2013 has _MSC_VER 1800).
This should be cleaned up, as long as it is not 3rd party code that we don't
want to adjust.
Currently still supported ("valid") VS version are 2017+, see
https://github.com/openjdk/jdk/blob/master/make/autoconf/toolchain_microsoft.m4
.
VALID_VS_VERSIONS="2019 2017 2022"
Even with adjusted toolchain m4 files, something older than VS2013 (also
probably older than VS2015) would not be able to build jdk anymore.
-
Commit messages:
- JDK-8288094
Changes: https://git.openjdk.java.net/jdk/pull/9105/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9105=00
Issue: https://bugs.openjdk.org/browse/JDK-8288094
Stats: 144 lines in 9 files changed: 0 ins; 131 del; 13 mod
Patch: https://git.openjdk.java.net/jdk/pull/9105.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/9105/head:pull/9105
PR: https://git.openjdk.java.net/jdk/pull/9105
Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
On Wed, 8 Jun 2022 17:43:49 GMT, Naoto Sato wrote: >> The test >> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` >> verifies different failure modes of resource bundles. One of the failures is >> that the runner class, `MissingResourceCauseTestRun.java`, creates a file >> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the >> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file >> is removed. >> >> This does not work reliably on Windows. On msys2, `chmod` is essentially a >> no-op, so the file is not made unreadable, and hence the test fails. In my >> personal cygwin test environment, the chmod command does have some effect, >> but it is still not enough to make the file unreadable, and so the test >> fails. >> >> The test was originally a shell script test that got converted to Java in >> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original >> shell script code explicitly excluded Windows from testing. This was changed >> in the rewrite, for reasons I cannot determine. >> >> What suprises me, though, is the "how can this ever has worked???" factor. >> Apparently the test passes on the current Cygwin setup on GHA. I have failed >> to reproduce the working conditions that makes a file actually unreadable >> for the owner on Windows, neither on my GHA test repo, nor locally. I've >> searched the web to figure out how to properly set file permissions on >> Windows to make the file unreadable, using Windows native tools, to no >> avail. I've even asked a [Stack Overflow >> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows); >> which as of yet is still unanswered. >> >> Since I feel I've spent far more time than reasonable trying to get this to >> work properly, I suggest we instead skip the unreadable test on Windows. It >> is clearly unstable and highly depending on the Windows environment, the >> test was never originally supported or intended for Windows, and at the of >> the day, testing file unreadability is not an important regression test for >> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216). > > Thanks for the investigation, Magnus. I believe it is a bug to include the > Windows environment in refactoring the shell script to Java. Why it's been > working? No idea. Could be `IOException` has been thrown not by `unreadable` > but for some other reason? @naotoj Yes, that could be a likely possibility. Perhaps the file were never created, or not in the expected place? - PR: https://git.openjdk.java.net/jdk/pull/9061
