Integrated: Merge jdk17
On Fri, 2 Jul 2021 00:18:55 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 17 -> JDK 18 This pull request has now been integrated. Changeset: b0e18679 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/b0e186792e816be30347dacfd88b8e55476584e7 Stats: 996 lines in 26 files changed: 843 ins; 64 del; 89 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/4661
Re: RFR: Merge jdk17 [v2]
> Forwardport JDK 17 -> JDK 18
Jesper Wilhelmsson has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains 133 commits:
- Merge
- 8225559: assertion error at TransTypes.visitApply
Reviewed-by: sadayapalam, jlahoda
- 8268960: com/sun/net/httpserver/Headers.java: Ensure mutators normalize keys
and disallow null for keys and values
Reviewed-by: chegar, dfuchs, michaelm
- 8267307: Introduce new client property for XAWT: xawt.mwm_decor_title
Reviewed-by: azvegint, serb
- 8133873: Simplify {Register,Unregister}NMethodOopClosure
Reviewed-by: tschatzl, kbarrett
- 8268298: jdk/jfr/api/consumer/log/TestVerbosity.java fails: unexpected log
message
Reviewed-by: egahlin
- 8266746: C1: Replace UnsafeGetRaw with UnsafeGet when setting up OSR entry
block
Replace UnsafeGetRaw with UnsafeGetObject when setting up OSR entry block,
and rename Unsafe{Get,Put}Object to Unsafe{Get,Put}
Reviewed-by: thartmann, dlong, mdoerr
- 8268870: Remove dead code in metaspaceShared
Reviewed-by: tschatzl
- Merge
- 8268637: Update --release 17 symbol information for JDK 17 build 28
Reviewed-by: iris
- ... and 123 more:
https://git.openjdk.java.net/jdk/compare/a4d2a9a7...5515a992
-
Changes: https://git.openjdk.java.net/jdk/pull/4661/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4661=01
Stats: 32483 lines in 677 files changed: 18918 ins; 11377 del; 2188 mod
Patch: https://git.openjdk.java.net/jdk/pull/4661.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4661/head:pull/4661
PR: https://git.openjdk.java.net/jdk/pull/4661
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8269745: [JVMCI] restore original qualified exports to Graal - 8268566: java/foreign/TestResourceScope.java timed out - 8260684: vmTestbase/gc/gctests/PhantomReference/phantom002/TestDescription.java timed out - 8269580: assert(is_valid()) failed: invalid register (-1) - 8269704: Typo in j.t.Normalizer.normalize() - 8269354: javac crashes when processing parenthesized pattern in instanceof - 8269285: Crash/miscompile in CallGenerator::for_method_handle_inline after JDK-8191998 - 8269088: C2 fails with assert(!n->is_Store() && !n->is_LoadStore()) failed: no node with a side effect - 8269230: C2: main loop in micro benchmark never executed - ... and 3 more: https://git.openjdk.java.net/jdk/compare/de61328d...5515a992 The webrevs contain the adjustments done while merging with regards to each parent branch: - master: https://webrevs.openjdk.java.net/?repo=jdk=4661=00.0 - jdk17: https://webrevs.openjdk.java.net/?repo=jdk=4661=00.1 Changes: https://git.openjdk.java.net/jdk/pull/4661/files Stats: 996 lines in 26 files changed: 843 ins; 64 del; 89 mod Patch: https://git.openjdk.java.net/jdk/pull/4661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4661/head:pull/4661 PR: https://git.openjdk.java.net/jdk/pull/4661
Re: RFR: 8188046: java.lang.Math.mutliplyHigh does not run in constant time
On Thu, 1 Jul 2021 23:46:00 GMT, Brian Burkhalter wrote: > Please consider this proposal to remove the fast path in > `java.lang.Math.multiplyHigh()` the small performance improvement of which is > eclipsed by the branch penalty. Please refer to the issue for a benchmark and sample output from it. - PR: https://git.openjdk.java.net/jdk/pull/4660
RFR: 8188046: java.lang.Math.mutliplyHigh does not run in constant time
Please consider this proposal to remove the fast path in `java.lang.Math.multiplyHigh()` the small performance improvement of which is eclipsed by the branch penalty. - Commit messages: - 8188046: java.lang.Math.mutliplyHigh does not run in constant time Changes: https://git.openjdk.java.net/jdk/pull/4660/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4660=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8188046 Stats: 25 lines in 1 file changed: 0 ins; 11 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/4660.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4660/head:pull/4660 PR: https://git.openjdk.java.net/jdk/pull/4660
Re: [jdk17] RFR: 8269281: java/foreign/TestUpcall.java times out
On Thu, 1 Jul 2021 21:11:44 GMT, Maurizio Cimadamore wrote: > The previous fix to this test which used removed the `VerifyDependency` flag > worked well, but we're still observing rare timeouts. Looking at the test > reports, it seems likely that slightly increasing the timeout would do the > trick. Hi Maurizio, Sorry but I don't see why you think increasing the timeout will help here. From what I can see a normal successful run takes less than a minute, or just over. I see a couple of outliers at 4 and 6 minutes, then you have a timeout after 10 minutes - that is 10x slower than expected. I think something unexpected is happening here to cause it to take so long. David - PR: https://git.openjdk.java.net/jdk17/pull/198
Re: RFR: 8188044: We need Math.unsignedMultiplyHigh [v2]
On Thu, 1 Jul 2021 18:08:22 GMT, Brian Burkhalter wrote: >> Please consider this proposal to add a method >> `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and >> `java.lang.StrictMath`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8188044: Add @see links between multiplyHigh() and unsignedMultiplyHigh(). Oh right. Then obviously this is the right place for the new method. Sorry for the noise. - PR: https://git.openjdk.java.net/jdk/pull/4644
Re: RFR: 8188044: We need Math.unsignedMultiplyHigh [v2]
On Thu, 1 Jul 2021 18:08:22 GMT, Brian Burkhalter wrote: >> Please consider this proposal to add a method >> `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and >> `java.lang.StrictMath`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8188044: Add @see links between multiplyHigh() and unsignedMultiplyHigh(). Yes, I am aware of those methods on `Long`. We do however already have `Math.multiplyHigh()` so adding the new method to `Math` would be consistent. - PR: https://git.openjdk.java.net/jdk/pull/4644
Re: RFR: 8188044: We need Math.unsignedMultiplyHigh [v2]
On Thu, 1 Jul 2021 18:08:22 GMT, Brian Burkhalter wrote: >> Please consider this proposal to add a method >> `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and >> `java.lang.StrictMath`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8188044: Add @see links between multiplyHigh() and unsignedMultiplyHigh(). `Long` already has some unsigned arithmetic methods: `divideUnsigned`, `compareUnsigned`, `toUnsignedString`. `Math` doesn't. Would it make more sense to add the new method to `Long`? That would also avoid the quirk of having distinct methods in `Math` and `StringMath` when the numbers in involved are integers. - PR: https://git.openjdk.java.net/jdk/pull/4644
Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v2]
> This PR-*draft* is **work in progress** and an invitation to discuss a > possible solution for issue > [JDK-8265891](https://bugs.openjdk.java.net/browse/JDK-8265891). It is *not > yet* intended for a final review. > > As proposed in JDK-8265891, this PR provides an implementation for > `Channels.newInputStream().transferTo()` which provide superior performance > compared to the current implementation. The changes are: > * Prevents transfers through the JVM heap as much as possibly by offloading > to deeper levels via NIO, hence allowing the operating system to optimize the > transfer. > * Using more JRE heap in the fallback case when no NIO is possible (still > only KiBs, hence mostl ynegligible even on SBCs) to better perform on modern > hardware / fast I/O devides. > > Using JMH I have benchmarked both, the original implementation and this > implementation, and (depending on the used hardware and use case) performance > change was approx. doubled performance. So this PoC proofs that it makes > sense to finalize this work and turn it into an actual OpenJDK contribution. > > I encourage everybody to discuss this draft: > * Are there valid arguments for *not* doing this change? > * Is there a *better* way to improve performance of > `Channels.newInputStream().transferTo()`? > * How to go on from here: What is missing to get this ready for an actual > review? Markus KARG has updated the pull request incrementally with one additional commit since the last revision: Draft: Renaming i and separating code into several methods Signed-off-by: Markus Karg - Changes: - all: https://git.openjdk.java.net/jdk/pull/4263/files - new: https://git.openjdk.java.net/jdk/pull/4263/files/ed49098a..7d18ca62 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4263=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4263=00-01 Stats: 108 lines in 1 file changed: 57 ins; 39 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/4263.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4263/head:pull/4263 PR: https://git.openjdk.java.net/jdk/pull/4263
[jdk17] RFR: 8269281: java/foreign/TestUpcall.java times out
The previous fix to this test which used removed the `VerifyDependency` flag worked well, but we're still observing rare timeouts. Looking at the test reports, it seems likely that slightly increasing the timeout would do the trick. - Commit messages: - Increase timeout for TestUpcall Changes: https://git.openjdk.java.net/jdk17/pull/198/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=198=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269281 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk17/pull/198.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/198/head:pull/198 PR: https://git.openjdk.java.net/jdk17/pull/198
Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v2]
On Thu, 1 Jul 2021 13:05:26 GMT, Alan Bateman wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> implement review suggestion - move isSelfOrParent to ZipPath class > > src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1971: > >> 1969: } >> 1970: return false; >> 1971: } > > It might be a bit clearer if ZipPath define isSelfOrParent(), that would > avoid needing to expose the internal representation. Done. I've updated the PR to implement this suggestion. - PR: https://git.openjdk.java.net/jdk/pull/4604
Re: RFR: 8188044: We need Math.unsignedMultiplyHigh [v2]
> Please consider this proposal to add a method > `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and > `java.lang.StrictMath`. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8188044: Add @see links between multiplyHigh() and unsignedMultiplyHigh(). - Changes: - all: https://git.openjdk.java.net/jdk/pull/4644/files - new: https://git.openjdk.java.net/jdk/pull/4644/files/df884569..0a8fec63 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4644=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4644=00-01 Stats: 5 lines in 2 files changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4644.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4644/head:pull/4644 PR: https://git.openjdk.java.net/jdk/pull/4644
[jdk17] Integrated: 8268566: java/foreign/TestResourceScope.java timed out
On Mon, 14 Jun 2021 15:42:03 GMT, Maurizio Cimadamore wrote: > This patch contains another minor tweak to TestResourceScope as we have seen > this test timing out at least once (on arm64). > > I realized that some of the logic recently introduced in the test could lead > to the test waiting forever: for instance, if all threads are executed right > away before the main thread gets a chance to do anything, we'd be in a state > where the lock count is zero, and no other thread will update it. I've > removed the while loop - as that's not essential to make sure that the test > works (what we want is to trigger a close operation that is concurrent with > respect some acquire/release). > > I've also lowered the number of threads - which was using 10K - that was good > for stress testing, but it's not a good number in more realistic conditions. This pull request has now been integrated. Changeset: e3773977 Author:Maurizio Cimadamore URL: https://git.openjdk.java.net/jdk17/commit/e3773977cfdcd691a5664a4715328f8552e319e7 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod 8268566: java/foreign/TestResourceScope.java timed out Reviewed-by: jvernee - PR: https://git.openjdk.java.net/jdk17/pull/45
Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller [v2]
On Wed, 30 Jun 2021 15:45:25 GMT, Weijun Wang wrote: >> Add a cache to record which sources have called `System::setSecurityManager` >> and only print out warning lines once for each. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > update cache key from String to Class Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/166
Re: RFR: 8188044: We need Math.unsignedMultiplyHigh
On Wed, 30 Jun 2021 23:13:06 GMT, Brian Burkhalter wrote: > Please consider this proposal to add a method > `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and > `java.lang.StrictMath`. Looks good. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4644
Re: [jdk17] RFR: JDK-8225667: Clarify the behavior of System::gc w.r.t. reference processing [v2]
On Wed, 30 Jun 2021 22:52:36 GMT, Mandy Chung wrote: >> This spec clarification is a follow-up to >> [JDK-8224760](https://bugs.openjdk.java.net/browse/JDK-8224760?focusedCommentId=14268320=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14268320) >> w.r.t. reference processing. Since there is no guarantee for any memory >> reclamation by the invocation of `System::gc`, the spec should also clarify >> that there is no guarantee in determining the change of reachability of any >> objects or any particular number of `Reference` objects be enqueued and >> cleared. >> >> CSR: >>https://bugs.openjdk.java.net/browse/JDK-8269690 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Kim's suggestion on the wording Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/183
Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v2]
> Can I please get a review of this proposed fix for > https://bugs.openjdk.java.net/browse/JDK-8251329? > > As noted in that issue, if a zip filesystem created on top of a jar > containing a "./" entry is "walked" using the `Files.walkFileTree`, it leads > to a infinite never ending iteration (which ultimately fails with Java heap > space OOM). > > Alan notes in that issue that: > >> This is more likely an issue with the zipfs DirectoryStream implementation. >> A DirectoryStream is specified to not include elements that for the special >> links to the current or parent directory. It should be rare. > > This indeed turned out to be an issue in the > `jdk.nio.zipfs.ZipDirectoryStream#iterator()` implementation where it calls > the `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` implementation. The > implementation, unlike what the javadoc of `java.nio.file.DirectoryStream` > states, was including the "." and ".." paths in its returned iterator: > >> The elements returned by the iterator are in no specific order. Some file > systems maintain special links to the directory itself and the directory's > parent directory. Entries representing these links are not returned by the > iterator. > > > The proposed fix in this patch checks the paths for "." and "..", similar to > what the `sun.nio.fs.UnixDirectoryStream` does in its `isSelfOrParent` and > skips those paths from being added into the returned iterator. The > `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` (where this change has been > done) is currently only used by `jdk.nio.zipfs.ZipDirectoryStream#iterator()` > and has package-private visibility, so this change shouldn't impact any other > usage/expectations. > > A new jtreg test has been added to reproduce this issue and verify the fix. > Local testing of the `test/jdk/jdk/nio/` (including this new test) went fine > without any issues after this change. I will be triggering a `tier1` test > locally in a while. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: implement review suggestion - move isSelfOrParent to ZipPath class - Changes: - all: https://git.openjdk.java.net/jdk/pull/4604/files - new: https://git.openjdk.java.net/jdk/pull/4604/files/344da6cb..3971ad6f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4604=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4604=00-01 Stats: 20 lines in 2 files changed: 8 ins; 9 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4604.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4604/head:pull/4604 PR: https://git.openjdk.java.net/jdk/pull/4604
Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside
On Sun, 27 Jun 2021 13:13:42 GMT, Jaikiran Pai wrote: > Can I please get a review of this proposed fix for > https://bugs.openjdk.java.net/browse/JDK-8251329? > > As noted in that issue, if a zip filesystem created on top of a jar > containing a "./" entry is "walked" using the `Files.walkFileTree`, it leads > to a infinite never ending iteration (which ultimately fails with Java heap > space OOM). > > Alan notes in that issue that: > >> This is more likely an issue with the zipfs DirectoryStream implementation. >> A DirectoryStream is specified to not include elements that for the special >> links to the current or parent directory. It should be rare. > > This indeed turned out to be an issue in the > `jdk.nio.zipfs.ZipDirectoryStream#iterator()` implementation where it calls > the `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` implementation. The > implementation, unlike what the javadoc of `java.nio.file.DirectoryStream` > states, was including the "." and ".." paths in its returned iterator: > >> The elements returned by the iterator are in no specific order. Some file > systems maintain special links to the directory itself and the directory's > parent directory. Entries representing these links are not returned by the > iterator. > > > The proposed fix in this patch checks the paths for "." and "..", similar to > what the `sun.nio.fs.UnixDirectoryStream` does in its `isSelfOrParent` and > skips those paths from being added into the returned iterator. The > `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` (where this change has been > done) is currently only used by `jdk.nio.zipfs.ZipDirectoryStream#iterator()` > and has package-private visibility, so this change shouldn't impact any other > usage/expectations. > > A new jtreg test has been added to reproduce this issue and verify the fix. > Local testing of the `test/jdk/jdk/nio/` (including this new test) went fine > without any issues after this change. I will be triggering a `tier1` test > locally in a while. src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1971: > 1969: } > 1970: return false; > 1971: } It might be a bit clearer if ZipPath define isSelfOrParent(), that would avoid needing to expose the internal representation. - PR: https://git.openjdk.java.net/jdk/pull/4604
Re: [jdk17] RFR: JDK-8225667: Clarify the behavior of System::gc w.r.t. reference processing [v2]
On Wed, 30 Jun 2021 22:52:36 GMT, Mandy Chung wrote: >> This spec clarification is a follow-up to >> [JDK-8224760](https://bugs.openjdk.java.net/browse/JDK-8224760?focusedCommentId=14268320=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14268320) >> w.r.t. reference processing. Since there is no guarantee for any memory >> reclamation by the invocation of `System::gc`, the spec should also clarify >> that there is no guarantee in determining the change of reachability of any >> objects or any particular number of `Reference` objects be enqueued and >> cleared. >> >> CSR: >>https://bugs.openjdk.java.net/browse/JDK-8269690 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Kim's suggestion on the wording Marked as reviewed by tschatzl (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/183
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v3]
On Thu, 1 Jul 2021 10:38:24 GMT, Сергей Цыпанов
wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8268764: Update copy-right year
Right, done!
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v4]
> In some JDK classes there's still the following hashCode() implementation:
>
> long objNum;
>
> public int hashCode() {
> return (int) objNum;
> }
>
> This outdated expression should be replaced with Long.hashCode(long) as it
>
> - uses all bits of the original value, does not discard any information
> upfront. For example, depending on how you are generating the IDs, the upper
> bits could change more frequently (or the opposite).
>
> - does not introduce any bias towards values with more ones (zeros), as it
> would be the case if the two halves were combined with an OR (AND) operation.
>
> See https://stackoverflow.com/a/4045083
>
> This is related to https://github.com/openjdk/jdk/pull/4309
Сергей Цыпанов has updated the pull request incrementally with one additional
commit since the last revision:
8268764: Update copy-right year
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4491/files
- new: https://git.openjdk.java.net/jdk/pull/4491/files/932c26ad..20ad76be
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4491=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4491=02-03
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/4491.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4491/head:pull/4491
PR: https://git.openjdk.java.net/jdk/pull/4491
[jdk17] Integrated: 8269704: Typo in j.t.Normalizer.normalize()
On Wed, 30 Jun 2021 21:38:43 GMT, Naoto Sato wrote: > A trivial typo fix. This pull request has now been integrated. Changeset: 54dd510b Author:Naoto Sato URL: https://git.openjdk.java.net/jdk17/commit/54dd510bd5211dc440285dd53ca0e41c85e23552 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8269704: Typo in j.t.Normalizer.normalize() Reviewed-by: joehw, prappo, iris - PR: https://git.openjdk.java.net/jdk17/pull/187
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v3]
On Thu, 1 Jul 2021 10:38:24 GMT, Сергей Цыпанов
wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8268764: Update copy-right year
Marked as reviewed by kevinw (Committer).
OK, one more (C) in
src/jdk.jdi/share/classes/com/sun/tools/jdi/ReferenceTypeImpl.java and done. 8-)
Needs a second Review before integrating, thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v3]
> In some JDK classes there's still the following hashCode() implementation:
>
> long objNum;
>
> public int hashCode() {
> return (int) objNum;
> }
>
> This outdated expression should be replaced with Long.hashCode(long) as it
>
> - uses all bits of the original value, does not discard any information
> upfront. For example, depending on how you are generating the IDs, the upper
> bits could change more frequently (or the opposite).
>
> - does not introduce any bias towards values with more ones (zeros), as it
> would be the case if the two halves were combined with an OR (AND) operation.
>
> See https://stackoverflow.com/a/4045083
>
> This is related to https://github.com/openjdk/jdk/pull/4309
Сергей Цыпанов has updated the pull request incrementally with one additional
commit since the last revision:
8268764: Update copy-right year
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4491/files
- new: https://git.openjdk.java.net/jdk/pull/4491/files/12a1d3ac..932c26ad
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4491=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4491=01-02
Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod
Patch: https://git.openjdk.java.net/jdk/pull/4491.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4491/head:pull/4491
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v2]
On Wed, 30 Jun 2021 11:49:51 GMT, Сергей Цыпанов
wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов 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 two additional
> commits since the last revision:
>
> - Merge branch 'master' into 8268764
> - 8268764: Use Long.hashCode() instead of int-cast where applicable
Hi Kevin, thanks for review! I've updated copy-right year
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v2]
Thank you Alan and Lance. I'll update this PR shortly with the proposed approach. -Jaikiran On 01/07/21 2:13 pm, Alan Bateman wrote: On 30/06/2021 17:15, Jaikiran Pai wrote: I understand that Alan's suggestion holds good and we should have some logic in place which switches to using a temp file once we notice that the sizes we are dealing with can exceed some threshold, but I guess that is something we need to do separately outside of this PR? My comment was mostly just to point out that it's only a partial fix and it will eventually fail with an OOME once the deflated size is too big for the BAOS. I don't have a strong opinion on whether the complete fix is done in one, two or many PRs but I think the first step could be to use the "useTempFile" path when the entry size is larger than some (10s of MB?) threshold. -Alan
Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v2]
On 30/06/2021 17:15, Jaikiran Pai wrote: I understand that Alan's suggestion holds good and we should have some logic in place which switches to using a temp file once we notice that the sizes we are dealing with can exceed some threshold, but I guess that is something we need to do separately outside of this PR? My comment was mostly just to point out that it's only a partial fix and it will eventually fail with an OOME once the deflated size is too big for the BAOS. I don't have a strong opinion on whether the complete fix is done in one, two or many PRs but I think the first step could be to use the "useTempFile" path when the entry size is larger than some (10s of MB?) threshold. -Alan
