Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v4]
On Thu, 6 May 2021 08:05:12 GMT, Jason Zaugg wrote:
>> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1235:
>>
>>> 1233: synchronized(ch) {
>>> 1234: return ch.position(pos).read(bb);
>>> 1235: }
>>
>> I think it's okay to include the update to EntryInputStream, that part looks
>> fine, as does the directly use of the FileChannel positional read.
>>
>> I'm still mulling over the case where ch is not a FileChannel as I would
>> expected it to capture the existing position and restore it after the read.
>> I think this is the degenerative case when the zip file is located in a
>> custom file system that doesn't support FileChannel. In that case,
>> positional read has to be implemented on the most basic SeekableByteChannel.
>> It would only be observed when mixing positional read ops with other ops
>> that depend on the current position.
>
> Here are all the references to `ch`.
>
>
> this.ch = Files.newByteChannel(zfpath, READ);
> ...
> this.ch.close();
> ...
> ch.close(); // close the ch just in case no update
> ...
> if (ch instanceof FileChannel fch) {
> return fch.read(bb, pos);
> } else {
> synchronized(ch) {
> return ch.position(pos).read(bb);
> }
> }
> ...
> long ziplen = ch.size();
> ...
> ch.close();
>
>
> It appears the only position-dependent operation called `read(ByteBuffer)`.
> This is performed together with the `pos` call within the `synchronized(ch)`
> lock.
I have confirmed that the non-`FileChannel` code path is exercised by existing
tests.
test/jdk/jdk/nio/zipfs/ZipFSTester.java includes a test that forms a file
system based on a JAR that is itself an entry within another `ZipFileSystem`.
Sample stacks:
java.lang.Throwable: readFullyAt. ch.getClass=class
jdk.nio.zipfs.ByteArrayChannel
at
jdk.zipfs/jdk.nio.zipfs.ZipFileSystem.readFullyAt(ZipFileSystem.java:1234)
at
jdk.zipfs/jdk.nio.zipfs.ZipFileSystem.readFullyAt(ZipFileSystem.java:1226)
at
jdk.zipfs/jdk.nio.zipfs.ZipFileSystem$EntryInputStream.initDataPos(ZipFileSystem.java:2259)
at
jdk.zipfs/jdk.nio.zipfs.ZipFileSystem$EntryInputStream.read(ZipFileSystem.java:2201)
at jdk.zipfs/jdk.nio.zipfs.ZipFileSystem$2.fill(ZipFileSystem.java:2151)
at
java.base/java.util.zip.InflaterInputStream.read(InflaterInputStream.java:158)
at ZipFSTester.checkEqual(ZipFSTester.java:858)
at ZipFSTester.test1(ZipFSTester.java:259)
java.lang.Throwable: readFullyAt. ch.getClass=class
jdk.nio.zipfs.ByteArrayChannel
at
jdk.zipfs/jdk.nio.zipfs.ZipFileSystem.readFullyAt(ZipFileSystem.java:1234)
at
jdk.zipfs/jdk.nio.zipfs.ZipFileSystem$EntryInputStream.read(ZipFileSystem.java:2214)
at jdk.zipfs/jdk.nio.zipfs.ZipFileSystem$2.fill(ZipFileSystem.java:2151)
at
java.base/java.util.zip.InflaterInputStream.read(InflaterInputStream.java:158)
at ZipFSTester.checkEqual(ZipFSTester.java:858)
at ZipFSTester.test1(ZipFSTester.java:259)
This use case is not covered by the `ZipFSTester.test2`, a multi-threaded test.
While looking at the test I noticed false warnings in the output:
`read()/position() failed`. This did not actually fail the test. I investigated
this and a) fixed the condition to deal with the edge case of zero-length
entries and b) throw an "check failed" exception when the assertion fails.
-
PR: https://git.openjdk.java.net/jdk/pull/3853
Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v5]
> If the given Path represents a file, use the overload of read defined > in FileChannel that accepts an explicit position and avoid serializing > reads. > > Note: The underlying NIO implementation is not required to implement > FileChannel.read(ByteBuffer, long) concurrently; Windows still appears > to lock, as it returns true for NativeDispatcher.needsPositionLock. > > > On MacOS X, the enclosed benchmark improves from: > > > BenchmarkMode Cnt Score Error Units > ZipFileSystemBenchmark.read avgt 10 75.311 ? 3.301 ms/op > > > To: > > > BenchmarkMode Cnt Score Error Units > ZipFileSystemBenchmark.read avgt 10 12.520 ? 0.875 ms/op Jason Zaugg has updated the pull request incrementally with one additional commit since the last revision: [zipfs] Add missing check-failed exception to position/read test This appears to have been omitted when this test was added. To avoid false error reports, the condition must deal with the edge case of zero-length entries, for which read will return -1. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3853/files - new: https://git.openjdk.java.net/jdk/pull/3853/files/c9758c58..9106096e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3853=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3853=03-04 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3853.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3853/head:pull/3853 PR: https://git.openjdk.java.net/jdk/pull/3853
RFR: 8266456: Replace direct TKit.run() calls with jdk.jpackage.test.Annotations.Test annotation
- Replaced direct TKit.run() calls with Test annotation. - Increased timeout for SigningPackageTest from default to 360 due to timeout. This is regression from JDK-8248904 due to changes done in signing and --remove-signature adds additional time since it is run per file. - Fixed issue with jtreg.SkippedException which caused test to fail instead of being skipped, since it was wrapped in ExceptionBox. - Commit messages: - 8266456: Replace direct TKit.run() calls with jdk.jpackage.test.Annotations.Test annotation Changes: https://git.openjdk.java.net/jdk/pull/3911/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3911=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266456 Stats: 214 lines in 12 files changed: 23 ins; 16 del; 175 mod Patch: https://git.openjdk.java.net/jdk/pull/3911.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3911/head:pull/3911 PR: https://git.openjdk.java.net/jdk/pull/3911
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
On Thu, 6 May 2021 21:59:32 GMT, Ioi Lam wrote: >> Yumin Qi has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove tab space > > The latest version LGTM @iklam @calvinccheung Thanks for review! - PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
On Thu, 6 May 2021 21:37:35 GMT, Yumin Qi wrote: >> Hi, Please review >> When using jcmd to dump shared archive, if the archive name exists, it >> will be deleted first. If exception happens during dumping process, there is >> no new archive created. This PR changes to first dump the archive with a >> temporary file name. With successful dump, the temporary file will be rename >> to its given name. This way the old archive will not be touched on exception. >> The newly added test case skipped testing on Windows since File operation >> result is not same as on Linux. >> >>Tests: tier1,tier2,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Remove tab space Marked as reviewed by ccheung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
On Fri, 7 May 2021 00:02:56 GMT, Yumin Qi wrote: >> test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 78: >> >>> 76: print2ln(test_count++ + " Set target dir not writable, do >>> dynamic dump"); >>> 77: setKeepArchive(true); >>> 78: outputDirFile.setWritable(true); >> >> Should the comment be `// Set target dir writable ...` ? (since you're >> setting the dir to writable at line 78) > > The comment is for the testing item --- that is consistent with println > contents. > The first set 'true' is for get the archive and keep the archive, the real > test is after set it to 'false', the test will fail and we check the previous > archive still available. I see. - PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
On Thu, 6 May 2021 23:59:21 GMT, Calvin Cheung wrote: >> Yumin Qi has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove tab space > > test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 78: > >> 76: print2ln(test_count++ + " Set target dir not writable, do >> dynamic dump"); >> 77: setKeepArchive(true); >> 78: outputDirFile.setWritable(true); > > Should the comment be `// Set target dir writable ...` ? (since you're > setting the dir to writable at line 78) The comment is for the testing item --- that is consistent with println contents. The first set 'true' is for get the archive and keep the archive, the real test is after set it to 'false', the test will fail and we check the previous archive still available. > test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 89: > >> 87: outputDirFile.setWritable(false); >> 88: test(localFileName, pid, noBoot, EXPECT_FAIL); >> 89: outputDirFile.setWritable(true); > > Would the test fail without setting the dir not writable at line 87? See reply above. - PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
On Thu, 6 May 2021 21:37:35 GMT, Yumin Qi wrote: >> Hi, Please review >> When using jcmd to dump shared archive, if the archive name exists, it >> will be deleted first. If exception happens during dumping process, there is >> no new archive created. This PR changes to first dump the archive with a >> temporary file name. With successful dump, the temporary file will be rename >> to its given name. This way the old archive will not be touched on exception. >> The newly added test case skipped testing on Windows since File operation >> result is not same as on Linux. >> >>Tests: tier1,tier2,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Remove tab space I have 2 questions on the test. test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 78: > 76: print2ln(test_count++ + " Set target dir not writable, do dynamic > dump"); > 77: setKeepArchive(true); > 78: outputDirFile.setWritable(true); Should the comment be `// Set target dir writable ...` ? (since you're setting the dir to writable at line 78) test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 89: > 87: outputDirFile.setWritable(false); > 88: test(localFileName, pid, noBoot, EXPECT_FAIL); > 89: outputDirFile.setWritable(true); Would the test fail without setting the dir not writable at line 87? - PR: https://git.openjdk.java.net/jdk/pull/3886
Proposal for new interface: TimeSource
This is a proposal to add a new interface to java.time.*
public interface TimeSource {
public static TimeSource system() { ... }
public abstract Instant instant();
public default long millis() {
return instant().toEpochMilli();
}
public default Clock withZone(ZoneId zoneId) {
return Clock.of(this, zoneId);
}
}
The existing `Clock` abstract class would be altered to implement the interface.
A new static method `Clock.of(TimeSource, ZoneId)` would be added.
No changes are needed to any other part of the API.
(Could add `Instant.now(TimeSource)`, but it isn't entirely necessary)
Callers would pass around and use `TimeSource` directly, for example
by dependency injection.
Why add this interface?
I've seen various indications that there is a desire for an interface
representing a supplier of `Instant`. Specifically this desire is
driven by the "unnecessary" time zone that `java.time.Clock` contains.
Put simply, if the only thing you want is an `Instant`, then `Clock`
isn't the right API because it forces you to think about time zones.
A key thing that this interface allows is the separation of the OS
Clock from the time-zone (which should generally be linked to user
localization). A good architecture would pass `TimeSource` around the
system and combine it with a time-zone held in a `UserContext` to get
a `Clock`. The current design of java.time.* does not enable that
because the `TimeSource` concept does not exist. Developers either
have to write their own interface, or use `Clock` and ignore the time
zone.
A `Supplier` obviously performs similar functionality, but it
lacks discoverability and understandability. Plus, injecting
generified interfaces tends to be painful.
Downsides?
None really, other than a new type in the JDK that probably should
have been in Java 8.
See this ThreeTen-Extra discussion
- https://github.com/ThreeTen/threeten-extra/issues/150
Joda-Time has a `MillisProvider` that is similar:
-
https://www.joda.org/joda-time/apidocs/org/joda/time/DateTimeUtils.MillisProvider.html
Time4J has a `TimeSource`:
-
https://github.com/MenoData/Time4J/blob/master/base/src/main/java/net/time4j/base/TimeSource.java
Spring has a `DateTimeContext` that separates the user localization as
per the `UserContext` described above:
-
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/format/datetime/standard/DateTimeContext.html
There is a similar concept `TimeSource` in `sun.net.httpserver`
There may be a case to name the interface `InstantSource`, however
`TimeSource` is a fairly widely understood name for this concept.
There is the potential to extend the interface with something similar
to Kotlin's `TimeMark` that would allow access to the monotonic clock,
suitable for measurement of durations without any leap second effects:
- https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-time-mark/
Feedback?
Stephen
New candidate JEP: 415: Context-Specific Deserialization Filters
https://openjdk.java.net/jeps/415 Summary: Allow applications to configure context-specific and dynamically-selected deserialization filters via a JVM-wide filter factory that is invoked to select a filter for each individual deserialization operation. - Mark
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
On Thu, 6 May 2021 21:37:35 GMT, Yumin Qi wrote: >> Hi, Please review >> When using jcmd to dump shared archive, if the archive name exists, it >> will be deleted first. If exception happens during dumping process, there is >> no new archive created. This PR changes to first dump the archive with a >> temporary file name. With successful dump, the temporary file will be rename >> to its given name. This way the old archive will not be touched on exception. >> The newly added test case skipped testing on Windows since File operation >> result is not same as on Linux. >> >>Tests: tier1,tier2,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Remove tab space The latest version LGTM - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]
> Hi, Please review > When using jcmd to dump shared archive, if the archive name exists, it > will be deleted first. If exception happens during dumping process, there is > no new archive created. This PR changes to first dump the archive with a > temporary file name. With successful dump, the temporary file will be rename > to its given name. This way the old archive will not be touched on exception. > The newly added test case skipped testing on Windows since File operation > result is not same as on Linux. > >Tests: tier1,tier2,tier3,tier4 > > Thanks > Yumin Yumin Qi has updated the pull request incrementally with one additional commit since the last revision: Remove tab space - Changes: - all: https://git.openjdk.java.net/jdk/pull/3886/files - new: https://git.openjdk.java.net/jdk/pull/3886/files/e3363915..ab43e478 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3886=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3886=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3886.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3886/head:pull/3886 PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v4]
> Hi, Please review > When using jcmd to dump shared archive, if the archive name exists, it > will be deleted first. If exception happens during dumping process, there is > no new archive created. This PR changes to first dump the archive with a > temporary file name. With successful dump, the temporary file will be rename > to its given name. This way the old archive will not be touched on exception. > The newly added test case skipped testing on Windows since File operation > result is not same as on Linux. > >Tests: tier1,tier2,tier3,tier4 > > Thanks > Yumin Yumin Qi has updated the pull request incrementally with one additional commit since the last revision: Removed using temporary file in test case - Changes: - all: https://git.openjdk.java.net/jdk/pull/3886/files - new: https://git.openjdk.java.net/jdk/pull/3886/files/83478dd1..e3363915 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3886=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3886=02-03 Stats: 17 lines in 1 file changed: 2 ins; 10 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/3886.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3886/head:pull/3886 PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v7]
> Please review this PR that intents to make sealed classes a final feature in > Java. This PR contains compiler and VM changes. In line with similar PRs, > which has made preview features final, this one is mostly removing preview > related comments from APIs plus options in test cases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero 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 11 additional commits since the last revision: - Merge branch 'master' into JDK-8260517 - restoring jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES, it is needed by the build - Merge branch 'master' into JDK-8260517 - Merge branch 'master' into JDK-8260517 - updating comment after review feedback - removing javax.lang.model changes - Merge branch 'master' into JDK-8260517 - Merge branch 'master' into JDK-8260517 - fixing failing regression tests - JVM related changes - ... and 1 more: https://git.openjdk.java.net/jdk/compare/10336a26...0208dcf0 - Changes: - all: https://git.openjdk.java.net/jdk/pull/3526/files - new: https://git.openjdk.java.net/jdk/pull/3526/files/304fd76a..0208dcf0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3526=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3526=05-06 Stats: 18908 lines in 391 files changed: 9887 ins; 4814 del; 4207 mod Patch: https://git.openjdk.java.net/jdk/pull/3526.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3526/head:pull/3526 PR: https://git.openjdk.java.net/jdk/pull/3526
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Thu, 6 May 2021 11:52:09 GMT, Ignasi Marimon-Clos wrote: >> @ignasi35 I have created JDK-8266578. Please change the name of this PR to: >> >> `8266578: Disambiguate BigDecimal description of scale` >> >> Please resolve any conflicts and commit the updated file. Once the update is >> available, I will file a CSR and mark this PR accordingly. > > Thanks @bplb for doing the heavy lifting. I've updated the PR title and > solved the conflict. @ignasi35 This PR has been approved, so once you have issued the [integrate](https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/integrate) command I can sponsor it. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Fri, 9 Oct 2020 16:14:59 GMT, Ignasi Marimon-Clos wrote: > The API Docs for `BigDecimal` introduce the meaning of `scale`. The current > writeup can be missleading when presenting the meaning of a `scale` value > that's negative. Hopefully, with this PR, the sentence is more clear. > > The ambiguity is on this sentence: > >> If negative, the unscaled value of the number is ... > > Instead, I suggest the more verbose: > >> If the scale is negative, the unscaled value of the number is ... > > To keep symmetry, I've also reviewed the positive case from: > >> If zero or positive, the scale is the number of digits ... > > to: > >> If the scale is zero or positive, the scale is the number of digits ... Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Fri, 9 Oct 2020 16:14:59 GMT, Ignasi Marimon-Clos wrote: > The API Docs for `BigDecimal` introduce the meaning of `scale`. The current > writeup can be missleading when presenting the meaning of a `scale` value > that's negative. Hopefully, with this PR, the sentence is more clear. > > The ambiguity is on this sentence: > >> If negative, the unscaled value of the number is ... > > Instead, I suggest the more verbose: > >> If the scale is negative, the unscaled value of the number is ... > > To keep symmetry, I've also reviewed the positive case from: > >> If zero or positive, the scale is the number of digits ... > > to: > >> If the scale is zero or positive, the scale is the number of digits ... Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v3]
On Thu, 6 May 2021 16:57:12 GMT, Daniel Fuchs wrote: >> Hi, please find here a trivial test change that adds some diagnostic (time >> stamps) to the LoggerFinder/modules subprocess test logs. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > removed unused import statement Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v3]
On Thu, 6 May 2021 16:57:12 GMT, Daniel Fuchs wrote: >> Hi, please find here a trivial test change that adds some diagnostic (time >> stamps) to the LoggerFinder/modules subprocess test logs. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > removed unused import statement Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v3]
On Thu, 6 May 2021 16:57:12 GMT, Daniel Fuchs wrote: >> Hi, please find here a trivial test change that adds some diagnostic (time >> stamps) to the LoggerFinder/modules subprocess test logs. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > removed unused import statement Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v3]
On Thu, 6 May 2021 16:57:12 GMT, Daniel Fuchs wrote: >> Hi, please find here a trivial test change that adds some diagnostic (time >> stamps) to the LoggerFinder/modules subprocess test logs. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > removed unused import statement Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v3]
> Hi, please find here a trivial test change that adds some diagnostic (time > stamps) to the LoggerFinder/modules subprocess test logs. Daniel Fuchs has updated the pull request incrementally with one additional commit since the last revision: removed unused import statement - Changes: - all: https://git.openjdk.java.net/jdk/pull/3904/files - new: https://git.openjdk.java.net/jdk/pull/3904/files/3e6b436c..c101347a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3904=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3904=01-02 Stats: 4 lines in 4 files changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3904.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3904/head:pull/3904 PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v2]
On Thu, 6 May 2021 16:42:16 GMT, Daniel Fuchs wrote: >> Hi, please find here a trivial test change that adds some diagnostic (time >> stamps) to the LoggerFinder/modules subprocess test logs. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > Use + Instant.now(); Instant.toString() uses the same formatter as > ISO_INSTANT. Doh. Done. - PR: https://git.openjdk.java.net/jdk/pull/3904
RFR: 8266225: jarsigner is using incorrect security property to show weakness of certs
Please review the change to jarsigner so it uses certpath security property in order to properly display the weakness of the certificate algorithms. - Commit messages: - 8266225:jarsigner is using incorrect security property to show weakness of certs Changes: https://git.openjdk.java.net/jdk/pull/3905/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3905=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266225 Stats: 39 lines in 2 files changed: 31 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/3905.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3905/head:pull/3905 PR: https://git.openjdk.java.net/jdk/pull/3905
Re: RFR: 8266317: Vector API enhancements
On Thu, 29 Apr 2021 21:13:38 GMT, Paul Sandoz wrote: > This PR contains API and implementation changes for [JEP-414 Vector API > (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for > when targeted. > > Enhancements are made to the API for the support of operations on characters, > such as for UTF-8 character decoding. Specifically, methods for > loading/storing a `short` vector from/to a `char[]` array, and new vector > comparison operators for unsigned comparisons with integral vectors. The x64 > implementation is enhanced to supported unsigned comparisons. > > Enhancements are made to the API for loading/storing a `byte` vector from/to > a `boolean[]` array. > > The testing of loads/stores can be expanded for scatter/gather, but before > doing that i think some refactoring of the tests is required to reposition > tests in the right classes. I would like to do that work after integration of > the PR. Reviewed X86 backend changes for unsigned comparison. - Marked as reviewed by jbhateja (Committer). PR: https://git.openjdk.java.net/jdk/pull/3803
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v2]
On Thu, 6 May 2021 16:42:16 GMT, Daniel Fuchs wrote: >> Hi, please find here a trivial test change that adds some diagnostic (time >> stamps) to the LoggerFinder/modules subprocess test logs. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > Use + Instant.now(); Instant.toString() uses the same formatter as > ISO_INSTANT. Looks good. I guess you could also remove `DateTimeFormatter` import statement now. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules [v2]
> Hi, please find here a trivial test change that adds some diagnostic (time > stamps) to the LoggerFinder/modules subprocess test logs. Daniel Fuchs has updated the pull request incrementally with one additional commit since the last revision: Use + Instant.now(); Instant.toString() uses the same formatter as ISO_INSTANT. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3904/files - new: https://git.openjdk.java.net/jdk/pull/3904/files/f027f8e0..3e6b436c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3904=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3904=00-01 Stats: 8 lines in 4 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/3904.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3904/head:pull/3904 PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules
On Thu, 6 May 2021 15:25:10 GMT, Daniel Fuchs wrote: > Hi, please find here a trivial test change that adds some diagnostic (time > stamps) to the LoggerFinder/modules subprocess test logs. Oh - good point Naoto! Thanks for the suggestion. Done. - PR: https://git.openjdk.java.net/jdk/pull/3904
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v6]
> Please consider this request to override the `java.io.InputStream` methods > `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more > performant implementations. The method overrides attempt to read all > requested bytes into a single array of the required size rather than > composing the result from a sequence of smaller arrays. An example of the > performance improvements is as follows. > > **readAllBytes** > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 > ± 7.309 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 > ± 0.369 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 > ± 0.048 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 > ± 0.183 ops/s > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 > ± 43.425 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 > ± 1.273 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 > ± 0.084 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 > ± 0.003 ops/s > > > **readNBytes** > > `N = file_size / 2` > > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 > ± 153.353 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 > ± 1.104 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 > ± 0.141 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 > ± 0.211 ops/s > > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 > ± 66.045 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 > ± 0.951 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 > ± 0.102 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 > ± 0.035 ops/s Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8264777: Make length and position consistent with RAF; add path to OOME message - Changes: - all: https://git.openjdk.java.net/jdk/pull/3845/files - new: https://git.openjdk.java.net/jdk/pull/3845/files/0e476822..5272d5ef Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3845=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3845=04-05 Stats: 13 lines in 2 files changed: 7 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3845.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3845/head:pull/3845 PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8266622: Optimize Class.descriptorString() and Class.getCanonicalName0()
On Thu, 6 May 2021 15:20:23 GMT, Сергей Цыпанов
wrote:
> Hello, from discussion in https://github.com/openjdk/jdk/pull/3464 and
> https://github.com/openjdk/jdk/pull/2212 it appears, that in `j.l.Class`
> expressions like
>
> String str = baseName.replace('.', '/') + '/' + name;
>
> are not compiled into invokedynamic-based code, but into one using
> `StringBuilder`.
>
> This happens due to some bootstraping issues. Currently the bytecode for the
> last (most often used) branch of `Class.descriptorString()` looks like
>
> public sb()Ljava/lang/String;
>L0
> LINENUMBER 21 L0
> NEW java/lang/StringBuilder
> DUP
> INVOKESPECIAL java/lang/StringBuilder. ()V
> ASTORE 1
>L1
> LINENUMBER 23 L1
> ALOAD 1
> LDC "a"
> INVOKEVIRTUAL java/lang/StringBuilder.append
> (Ljava/lang/String;)Ljava/lang/StringBuilder;
> POP
>L2
> LINENUMBER 24 L2
> ALOAD 1
> LDC "b"
> INVOKEVIRTUAL java/lang/StringBuilder.append
> (Ljava/lang/String;)Ljava/lang/StringBuilder;
> POP
>L3
> LINENUMBER 26 L3
> ALOAD 1
> INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String;
> ARETURN
>
> Here the `StringBuilder` is created with default constructor and then expands
> if necessary while appending.
>
> This can be improved by manually allocating `StringBuilder` of exact size.
> The benchmark demonstrates measurable improvement:
>
> @State(Scope.Benchmark)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ClassDescriptorStringBenchmark {
>
> private final Class clazzWithShortDescriptor = Object.class;
> private final Class clazzWithLongDescriptor = getClass();
>
> @Benchmark
> public String descriptorString_short() {
> return clazzWithShortDescriptor.descriptorString();
> }
>
> @Benchmark
> public String descriptorString_long() {
> return clazzWithLongDescriptor.descriptorString();
> }
> }
>
>
>
> original
> -Xint
>Mode Score Error
> Units
> descriptorString_long avgt 6326.478 ± 107.251
> ns/op
> descriptorString_short avgt 5220.729 ± 103.545
> ns/op
> descriptorString_long:·gc.alloc.rate.norm avgt 528.089 ± 0.021
> B/op
> descriptorString_short:·gc.alloc.rate.norm avgt 232.036 ± 0.015
> B/op
>
> -XX:TieredStopAtLevel=1
>Mode ScoreError
> Units
> descriptorString_long avgt230.223 ± 1.254
> ns/op
> descriptorString_short avgt164.255 ± 0.755
> ns/op
> descriptorString_long:·gc.alloc.rate.norm avgt528.046 ± 0.002
> B/op
> descriptorString_short:·gc.alloc.rate.norm avgt232.022 ± 0.001
> B/op
>
> full
>Mode Score Error
> Units
> descriptorString_long avgt 74.835 ± 0.262
> ns/op
> descriptorString_short avgt 43.822 ± 0.788
> ns/op
> descriptorString_long:·gc.alloc.rate.norm avgt504.010 ± 0.001
> B/op
> descriptorString_short:·gc.alloc.rate.norm avgt208.004 ± 0.001
> B/op
>
>
> patched
> -Xint
>Mode Score Error
> Units
> descriptorString_long avgt 4485.994 ± 60.173
> ns/op
> descriptorString_short avgt 3949.965 ± 278.143
> ns/op
> descriptorString_long:·gc.alloc.rate.norm avgt336.051 ± 0.004
> B/op
> descriptorString_short:·gc.alloc.rate.norm avgt184.027 ± 0.010
> B/op
>
> -XX:TieredStopAtLevel=1
>ModeScoreError
> Units
> descriptorString_long avgt 185.774 ± 1.100
> ns/op
> descriptorString_short avgt 135.338 ± 1.066
> ns/op
> descriptorString_long:·gc.alloc.rate.norm avgt 336.030 ± 0.001
> B/op
> descriptorString_short:·gc.alloc.rate.norm avgt 184.019 ± 0.001
> B/op
>
> full
>Mode Score Error
> Units
> descriptorString_long avgt 42.864 ± 0.160
> ns/op
> descriptorString_short avgt 27.255 ± 0.381
> ns/op
> descriptorString_long:·gc.alloc.rate.norm avgt224.005 ± 0.001
> B/op
> descriptorString_short:·gc.alloc.rate.norm avgt120.002 ± 0.001
> B/op
>
> Same can be done also for Class.isHidden() branch in Class.descriptorString()
> and for Class.getCanonicalName0()
Hi Sergei,
You are right that what javac generates is sub-optimal as it doesn't take into
account the
Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v3]
> Hi, Please review > When using jcmd to dump shared archive, if the archive name exists, it > will be deleted first. If exception happens during dumping process, there is > no new archive created. This PR changes to first dump the archive with a > temporary file name. With successful dump, the temporary file will be rename > to its given name. This way the old archive will not be touched on exception. > The newly added test case skipped testing on Windows since File operation > result is not same as on Linux. > >Tests: tier1,tier2,tier3,tier4 > > Thanks > Yumin Yumin Qi has updated the pull request incrementally with one additional commit since the last revision: Fixed temp file operation, archive and temp file name consistency - Changes: - all: https://git.openjdk.java.net/jdk/pull/3886/files - new: https://git.openjdk.java.net/jdk/pull/3886/files/b420da7b..83478dd1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3886=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3886=01-02 Stats: 289 lines in 5 files changed: 120 ins; 138 del; 31 mod Patch: https://git.openjdk.java.net/jdk/pull/3886.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3886/head:pull/3886 PR: https://git.openjdk.java.net/jdk/pull/3886
Re: RFR: 8265768 [aarch64] Use glibc libm impl for dlog, dlog10, dexp iff 2.29 or greater on AArch64.
On 5/6/2021 5:21 AM, Carlos O'Donell wrote: On Thu, 15 Apr 2021 08:33:47 GMT, gregcawthorne wrote: Glibc 2.29 onwards provides optimised versions of log,log10,exp. These functions have an accuracy of 0.9ulp or better in glibc 2.29. Therefore this patch adds code to parse, store and check the runtime glibcs version in os_linux.cpp/hpp. This is then used to select the glibcs implementation of log, log10, exp at runtime for c1 and c2, iff we have glibc 2.29 or greater. This will ensure OpenJDK can benefit from future improvements to glibc. Glibc adheres to the ieee754 standard, unless stated otherwise in its spec. As there are no stated exceptions in the current glibc spec for dlog, dlog10 and dexp, we can assume they currently follow ieee754 (which testing confirms). As such, future version of glibc are unlikely to lose this compliance with ieee754 in future. W.r.t performance this patch sees ~15-30% performance improvements for log and log10, with ~50-80% performance improvements for exp for the common input ranged (which output real numbers). However for the NaN and inf output ranges we see a slow down of up to a factor of 2 for some functions and architectures. Due to this being the uncommon case we assert that this is a worthwhile tradeoff. Where does the requirement for monotonicity come from? From the specifications: "The computed result [of log] must be within 1 ulp of the exact result. Results must be semi-monotonic." https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/Math.html#log(double) and similarly for the other method. -Joe
Re: RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules
On Thu, 6 May 2021 15:25:10 GMT, Daniel Fuchs wrote: > Hi, please find here a trivial test change that adds some diagnostic (time > stamps) to the LoggerFinder/modules subprocess test logs. Could those be replaced with just `Instant.now()`? `Instant.toString()` uses the same formatter as `ISO_INSTANT`. - PR: https://git.openjdk.java.net/jdk/pull/3904
RFR: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules
Hi, please find here a trivial test change that adds some diagnostic (time stamps) to the LoggerFinder/modules subprocess test logs. - Commit messages: - 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules Changes: https://git.openjdk.java.net/jdk/pull/3904/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3904=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266646 Stats: 38 lines in 4 files changed: 27 ins; 2 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/3904.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3904/head:pull/3904 PR: https://git.openjdk.java.net/jdk/pull/3904
RFR: 8266622: Optimize Class.descriptorString() and Class.getCanonicalName0()
Hello, from discussion in https://github.com/openjdk/jdk/pull/3464 it appears,
that in `j.l.Class` expressions like
String str = baseName.replace('.', '/') + '/' + name;
are not compiled into invokedynamic-based code, but into one using
`StringBuilder`.
This happens due to some bootstraping issues. Currently the bytecode for the
last (most often used) branch of `Class.descriptorString()` looks like
public sb()Ljava/lang/String;
L0
LINENUMBER 21 L0
NEW java/lang/StringBuilder
DUP
INVOKESPECIAL java/lang/StringBuilder. ()V
ASTORE 1
L1
LINENUMBER 23 L1
ALOAD 1
LDC "a"
INVOKEVIRTUAL java/lang/StringBuilder.append
(Ljava/lang/String;)Ljava/lang/StringBuilder;
POP
L2
LINENUMBER 24 L2
ALOAD 1
LDC "b"
INVOKEVIRTUAL java/lang/StringBuilder.append
(Ljava/lang/String;)Ljava/lang/StringBuilder;
POP
L3
LINENUMBER 26 L3
ALOAD 1
INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String;
ARETURN
Here the `StringBuilder` is created with default constructor and then expands
if necessary while appending.
This can be improved by manually allocating `StringBuilder` of exact size. The
benchmark demonstrates measurable improvement:
@State(Scope.Benchmark)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class ClassDescriptorStringBenchmark {
private final Class clazzWithShortDescriptor = Object.class;
private final Class clazzWithLongDescriptor = getClass();
@Benchmark
public String descriptorString_short() {
return clazzWithShortDescriptor.descriptorString();
}
@Benchmark
public String descriptorString_long() {
return clazzWithLongDescriptor.descriptorString();
}
}
original
-Xint
Mode Score Error Units
descriptorString_long avgt 6326.478 ± 107.251 ns/op
descriptorString_short avgt 5220.729 ± 103.545 ns/op
descriptorString_long:·gc.alloc.rate.norm avgt 528.089 ± 0.021B/op
descriptorString_short:·gc.alloc.rate.norm avgt 232.036 ± 0.015B/op
-XX:TieredStopAtLevel=1
Mode ScoreError Units
descriptorString_long avgt230.223 ± 1.254 ns/op
descriptorString_short avgt164.255 ± 0.755 ns/op
descriptorString_long:·gc.alloc.rate.norm avgt528.046 ± 0.002B/op
descriptorString_short:·gc.alloc.rate.norm avgt232.022 ± 0.001B/op
full
Mode Score Error Units
descriptorString_long avgt 74.835 ± 0.262 ns/op
descriptorString_short avgt 43.822 ± 0.788 ns/op
descriptorString_long:·gc.alloc.rate.norm avgt504.010 ± 0.001B/op
descriptorString_short:·gc.alloc.rate.norm avgt208.004 ± 0.001B/op
patched
-Xint
Mode Score Error Units
descriptorString_long avgt 4485.994 ± 60.173 ns/op
descriptorString_short avgt 3949.965 ± 278.143 ns/op
descriptorString_long:·gc.alloc.rate.norm avgt336.051 ± 0.004B/op
descriptorString_short:·gc.alloc.rate.norm avgt184.027 ± 0.010B/op
-XX:TieredStopAtLevel=1
ModeScoreError
Units
descriptorString_long avgt 185.774 ± 1.100
ns/op
descriptorString_short avgt 135.338 ± 1.066
ns/op
descriptorString_long:·gc.alloc.rate.norm avgt 336.030 ± 0.001
B/op
descriptorString_short:·gc.alloc.rate.norm avgt 184.019 ± 0.001
B/op
full
Mode Score Error Units
descriptorString_long avgt 42.864 ± 0.160 ns/op
descriptorString_short avgt 27.255 ± 0.381 ns/op
descriptorString_long:·gc.alloc.rate.norm avgt224.005 ± 0.001B/op
descriptorString_short:·gc.alloc.rate.norm avgt120.002 ± 0.001B/op
Same can be done also for Class.isHidden() branch in Class.descriptorString()
and for Class.getCanonicalName0()
-
Commit messages:
- 8266622: Optimize Class.descriptorString() and Class.getCanonicalName0()
Changes: https://git.openjdk.java.net/jdk/pull/3903/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3903=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8266622
Stats: 19 lines in 1 file changed: 15 ins; 0 del; 4 mod
Patch: https://git.openjdk.java.net/jdk/pull/3903.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3903/head:pull/3903
PR: https://git.openjdk.java.net/jdk/pull/3903
Re: RFR: 8254598: StringDedupTable should use OopStorage
On Fri, 23 Apr 2021 19:48:47 GMT, Kim Barrett wrote:
> Please review this change to the String Deduplication facility. It is being
> changed to use OopStorage to hold weak references to relevant objects,
> rather than bespoke data structures requiring dedicated processing phases by
> supporting GCs.
>
> (The Shenandoah update was contributed by Zhengyu Gu.)
>
> This change significantly simplifies the interface between a given GC and
> the String Deduplication facility, which should make it much easier for
> other GCs to opt in. However, this change does not alter the set of GCs
> providing support; currently only G1 and Shenandoah support string
> deduplication. Adding support by other GCs will be in followup RFEs.
>
> Reviewing via the diffs might not be all that useful for some parts, as
> several files have been essentially completely replaced, and a number of
> files have been added or eliminated. The full webrev might be a better
> place to look.
>
> The major changes are in gc/shared/stringdedup/* and in the supporting
> collectors, but there are also some smaller changes in other places, most
> notably classfile/{stringTable,javaClasses}.
>
> This change is additionally labeled for review by core-libs, although there
> are no source changes there. This change injects a byte field of bits into
> java.lang.String, using one of the previously unused padding bytes in that
> class. (There were two unused bytes, now only one.)
>
> Testing:
> mach5 tier1-2 with and without -XX:+UseStringDeduplication
>
> Locally (linux-x64) ran all of the existing tests that use string
> deduplication with both G1 and Shenandoah. Note that
> TestStringDeduplicationInterned.java is disabled for shenandoah, as it
> currently fails, for reasons I haven't figured out but suspect are test
> related.
>
> - gc/stringdedup/ -- these used to be in gc/g1
> - runtime/cds/SharedStringsDedup.java
> - runtime/cds/appcds/cacheObject/DifferentHeapSizes.java
> - runtime/cds/appcds/sharedStrings/*
>
> shenandoah-only:
> - gc/shenandoah/jvmti/TestHeapDump.java
> - gc/shenandoah/TestStringDedup.java
> - gc/shenandoah/TestStringDedupStress.java
>
> Performance tested baseline, baseline + stringdedup, new with stringdedup,
> finding no significant differences.
src/hotspot/share/classfile/vmSymbols.hpp line 490:
> 488: template(data_cache_line_flush_size_name,
> "DATA_CACHE_LINE_FLUSH_SIZE") \
> 489: template(during_unsafe_access_name,
> "during_unsafe_access") \
> 490: template(no_deduplication_name,
> "no_deduplication") \
This addition was left over from an earlier version of the change, where I'd
added an ordinary private boolean member of this name to String. That was
before I learned about injected members and before the idea of adding
"duplication requested" came up. This name is no longer needed, and I'll be
removing it.
-
PR: https://git.openjdk.java.net/jdk/pull/3662
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v12]
> This PR contains the API and implementation changes for JEP-412 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/412 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Replace uses of -Djdk.foreign.restricted (useless now) with --enable-native-access - Changes: - all: https://git.openjdk.java.net/jdk/pull/3699/files - new: https://git.openjdk.java.net/jdk/pull/3699/files/72eb9bbc..926229ed Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3699=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3699=10-11 Stats: 45 lines in 8 files changed: 0 ins; 0 del; 45 mod Patch: https://git.openjdk.java.net/jdk/pull/3699.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699 PR: https://git.openjdk.java.net/jdk/pull/3699
Integrated: 8266460: java.io tests fail on null stream with upgraded jtreg/TestNG
On Tue, 4 May 2021 20:45:43 GMT, Lance Andersen wrote: > Hi all, > > This fix addresses a change in TestNG behavior for the Before/AfterGroups > annotations that was introduced via > https://github.com/cbeust/testng/pull/2167. The tests have been verified > against the latest TestNG release and continue to run with the current TestNG > release used by jtreg This pull request has now been integrated. Changeset: e8405970 Author:Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/e8405970b9998ff8f77bcf196f1456713a98c47f Stats: 122 lines in 4 files changed: 12 ins; 24 del; 86 mod 8266460: java.io tests fail on null stream with upgraded jtreg/TestNG Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/3866
Integrated: 8266579: Update test/jdk/java/lang/ProcessHandle/PermissionTest.java & test/jdk/java/sql/testng/util/TestPolicy.java
On Wed, 5 May 2021 19:10:21 GMT, Lance Andersen wrote:
> Hi all,
>
> Please review this patch which updates
> test/jdk/java/lang/ProcessHandle/PermissionTest.java &
> test/jdk/java/sql/testng/util/TestPolicy.java to include :
>
> `new PropertyPermission("testng.memory.friendly", "read");
> `
> Which will be needed by TestNG 7.4
>
> Best,
> Lance
This pull request has now been integrated.
Changeset: fcedfc8a
Author:Lance Andersen
URL:
https://git.openjdk.java.net/jdk/commit/fcedfc8a3b4299372f195cae036129dcd7b740ea
Stats: 2 lines in 2 files changed: 2 ins; 0 del; 0 mod
8266579: Update test/jdk/java/lang/ProcessHandle/PermissionTest.java &
test/jdk/java/sql/testng/util/TestPolicy.java
Reviewed-by: joehw, naoto, bpb
-
PR: https://git.openjdk.java.net/jdk/pull/3888
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v5]
On Tue, 4 May 2021 20:13:20 GMT, Brian Burkhalter wrote: >> On `/proc/cpuinfo` for example, `fstat()` succeeds but `st_size` in `struct >> stat` is zero. The correct position is however returned by `lseek()`. >> Apparently this proposal needs to be reworked to expect size zero when the >> size is in fact non-zero. > > Updated to handle `length() == 0` case. Not sure however whether for this > case it might not be better just to fall back to `super.readXBytes()` instead. Can you rename the native length and position methods to length0 and position0 as we will need to wrap these methods later. Also it will keep them consistent with RAF. - PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8265768 [aarch64] Use glibc libm impl for dlog, dlog10, dexp iff 2.29 or greater on AArch64.
On Thu, 15 Apr 2021 08:33:47 GMT, gregcawthorne wrote: > Glibc 2.29 onwards provides optimised versions of log,log10,exp. > These functions have an accuracy of 0.9ulp or better in glibc > 2.29. > > Therefore this patch adds code to parse, store and check > the runtime glibcs version in os_linux.cpp/hpp. > This is then used to select the glibcs implementation of > log, log10, exp at runtime for c1 and c2, iff we have > glibc 2.29 or greater. > > This will ensure OpenJDK can benefit from future improvements > to glibc. > > Glibc adheres to the ieee754 standard, unless stated otherwise > in its spec. > > As there are no stated exceptions in the current glibc spec > for dlog, dlog10 and dexp, we can assume they currently follow > ieee754 (which testing confirms). As such, future version of > glibc are unlikely to lose this compliance with ieee754 in > future. > > W.r.t performance this patch sees ~15-30% performance improvements for > log and log10, with ~50-80% performance improvements for exp for the > common input ranged (which output real numbers). However for the NaN > and inf output ranges we see a slow down of up to a factor of 2 for > some functions and architectures. > > Due to this being the uncommon case we assert that this is a > worthwhile tradeoff. Where does the requirement for monotonicity come from? - PR: https://git.openjdk.java.net/jdk/pull/3510
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Wed, 5 May 2021 20:29:59 GMT, Brian Burkhalter wrote: >>> @ignasi35 Do you wish to continue this PR? If so I will file an issue. >>> Thanks. >> >> Hi @bplb, this completely fell through the cracks. If you could file the >> issue for me I'd really appreciate it. Thanks. > > @ignasi35 I have created JDK-8266578. Please change the name of this PR to: > > `8266578: Disambiguate BigDecimal description of scale` > > Please resolve any conflicts and commit the updated file. Once the update is > available, I will file a CSR and mark this PR accordingly. Thanks @bplb for doing the heavy lifting. I've updated the PR title and solved the conflict. - PR: https://git.openjdk.java.net/jdk/pull/582
Integrated: 8266168: -Wmaybe-uninitialized happens in check_code.c
On Thu, 29 Apr 2021 06:54:33 GMT, Yasumasa Suenaga wrote: > We can see following compiler warning in check_code.c on GCC 11. This pull request has now been integrated. Changeset: 0f9852c6 Author:Yasumasa Suenaga URL: https://git.openjdk.java.net/jdk/commit/0f9852c63b12c43b52615ea003a4fc1d69ad3ada Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8266168: -Wmaybe-uninitialized happens in check_code.c Reviewed-by: stuefe - PR: https://git.openjdk.java.net/jdk/pull/3787
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Tue, 12 Jan 2021 14:07:23 GMT, Ignasi Marimon-Clos wrote: >> The `jcheck` test fails because "The commit message does not reference any >> issue. To add an issue reference to this PR, edit the title to be of the >> format `issue number: message`." > > Thanks @bplb. I'm afraid I'm not an > [author](http://openjdk.java.net/bylaws#author) or have access to create or > read the issues in > [JIRA](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300=1). > > I am a first-time crontributor and I'm unfamiliar with the specifics on > contributing. > @ignasi35 Do you wish to continue this PR? If so I will file an issue. Thanks. Hi @bplb, this completely fell through the cracks. If you could file the issue for me I'd really appreciate it. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Mon, 3 May 2021 16:45:22 GMT, Ignasi Marimon-Clos wrote: >> Thanks @bplb. I'm afraid I'm not an >> [author](http://openjdk.java.net/bylaws#author) or have access to create or >> read the issues in >> [JIRA](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300=1). >> >> I am a first-time crontributor and I'm unfamiliar with the specifics on >> contributing. > >> @ignasi35 Do you wish to continue this PR? If so I will file an issue. >> Thanks. > > Hi @bplb, this completely fell through the cracks. If you could file the > issue for me I'd really appreciate it. Thanks. @ignasi35 I have created JDK-8266578. Please change the name of this PR to: `8266578: Disambiguate BigDecimal description of scale` Please resolve any conflicts and commit the updated file. Once the update is available, I will file a CSR and mark this PR accordingly. - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Tue, 12 Jan 2021 14:07:23 GMT, Ignasi Marimon-Clos wrote: >> The `jcheck` test fails because "The commit message does not reference any >> issue. To add an issue reference to this PR, edit the title to be of the >> format `issue number: message`." > > Thanks @bplb. I'm afraid I'm not an > [author](http://openjdk.java.net/bylaws#author) or have access to create or > read the issues in > [JIRA](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300=1). > > I am a first-time crontributor and I'm unfamiliar with the specifics on > contributing. @ignasi35 Do you wish to continue this PR? If so I will file an issue. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Mon, 4 Jan 2021 19:31:40 GMT, Brian Burkhalter wrote: >> The API Docs for `BigDecimal` introduce the meaning of `scale`. The current >> writeup can be missleading when presenting the meaning of a `scale` value >> that's negative. Hopefully, with this PR, the sentence is more clear. >> >> The ambiguity is on this sentence: >> >>> If negative, the unscaled value of the number is ... >> >> Instead, I suggest the more verbose: >> >>> If the scale is negative, the unscaled value of the number is ... >> >> To keep symmetry, I've also reviewed the positive case from: >> >>> If zero or positive, the scale is the number of digits ... >> >> to: >> >>> If the scale is zero or positive, the scale is the number of digits ... > > The `jcheck` test fails because "The commit message does not reference any > issue. To add an issue reference to this PR, edit the title to be of the > format `issue number: message`." Thanks @bplb. I'm afraid I'm not an [author](http://openjdk.java.net/bylaws#author) or have access to create or read the issues in [JIRA](https://bugs.openjdk.java.net/secure/CreateIssue.jspa?pid=11300=1). I am a first-time crontributor and I'm unfamiliar with the specifics on contributing. - PR: https://git.openjdk.java.net/jdk/pull/582
RFR: 8266578: Disambiguate BigDecimal description of scale
The API Docs for `BigDecimal` introduce the meaning of `scale`. The current writeup can be missleading when presenting the meaning of a `scale` value that's negative. Hopefully, with this PR, the sentence is more clear. The ambiguity is on this sentence: > If negative, the unscaled value of the number is ... Instead, I suggest the more verbose: > If the scale is negative, the unscaled value of the number is ... To keep symmetry, I've also reviewed the positive case from: > If zero or positive, the scale is the number of digits ... to: > If the scale is zero or positive, the scale is the number of digits ... - Commit messages: - Merge branch 'master' into minor-disambiguation - Disambiguate the BigDecimal docs Changes: https://git.openjdk.java.net/jdk/pull/582/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=582=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266578 Stats: 6 lines in 1 file changed: 1 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/582.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/582/head:pull/582 PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Fri, 9 Oct 2020 16:14:59 GMT, Ignasi Marimon-Clos wrote: > The API Docs for `BigDecimal` introduce the meaning of `scale`. The current > writeup can be missleading when presenting the meaning of a `scale` value > that's negative. Hopefully, with this PR, the sentence is more clear. > > The ambiguity is on this sentence: > >> If negative, the unscaled value of the number is ... > > Instead, I suggest the more verbose: > >> If the scale is negative, the unscaled value of the number is ... > > To keep symmetry, I've also reviewed the positive case from: > >> If zero or positive, the scale is the number of digits ... > > to: > >> If the scale is zero or positive, the scale is the number of digits ... I'm trying to see what's the cause for the Check failure in `jcheck` but clicking `Details` opens http://openjdk.java.net/ without too much information to point me to the actual issue causing the check failure. - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8266578: Disambiguate BigDecimal description of scale
On Fri, 9 Oct 2020 16:14:59 GMT, Ignasi Marimon-Clos wrote: > The API Docs for `BigDecimal` introduce the meaning of `scale`. The current > writeup can be missleading when presenting the meaning of a `scale` value > that's negative. Hopefully, with this PR, the sentence is more clear. > > The ambiguity is on this sentence: > >> If negative, the unscaled value of the number is ... > > Instead, I suggest the more verbose: > >> If the scale is negative, the unscaled value of the number is ... > > To keep symmetry, I've also reviewed the positive case from: > >> If zero or positive, the scale is the number of digits ... > > to: > >> If the scale is zero or positive, the scale is the number of digits ... The `jcheck` test fails because "The commit message does not reference any issue. To add an issue reference to this PR, edit the title to be of the format `issue number: message`." - PR: https://git.openjdk.java.net/jdk/pull/582
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v8]
> The JDK codebase re-created many variants of checkIndex(`grep -I -r > 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which > annotated with @IntrinsicCandidate and it only has a corresponding C1 > intrinsic version. > > In fact, there is an utility method > `jdk.internal.util.Preconditions.checkIndex`(wrapped by > java.lang.Objects.checkIndex) that behaves the same as these variants of > checkIndex, we can replace these re-created variants of checkIndex by > Objects.checkIndex, it would significantly reduce duplicated code and enjoys > performance improvement because Preconditions.checkIndex is > @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of > Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we > can firstly implement its C1 counterpart. There are also a few kinds of stuff > we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK > codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request incrementally with one additional commit since the last revision: build failed - Changes: - all: https://git.openjdk.java.net/jdk/pull/3615/files - new: https://git.openjdk.java.net/jdk/pull/3615/files/e4959148..f996c99f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3615=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3615=06-07 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v7]
> The JDK codebase re-created many variants of checkIndex(`grep -I -r > 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which > annotated with @IntrinsicCandidate and it only has a corresponding C1 > intrinsic version. > > In fact, there is an utility method > `jdk.internal.util.Preconditions.checkIndex`(wrapped by > java.lang.Objects.checkIndex) that behaves the same as these variants of > checkIndex, we can replace these re-created variants of checkIndex by > Objects.checkIndex, it would significantly reduce duplicated code and enjoys > performance improvement because Preconditions.checkIndex is > @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of > Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we > can firstly implement its C1 counterpart. There are also a few kinds of stuff > we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK > codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - cmp clobbers its left argument on x86_32 - Merge branch 'master' into consolidate_checkindex - better check1-4 - AssertionError when expected exception was not thrown - remove extra newline - remove InlineNIOCheckIndex flag - remove java_nio_Buffer in javaClasses.hpp - consolidate - Changes: https://git.openjdk.java.net/jdk/pull/3615/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3615=06 Stats: 331 lines in 11 files changed: 235 ins; 78 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
Re: RFR: 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native
On Mon, 3 May 2021 15:37:44 GMT, Jorn Vernee wrote: >> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/macos/StackVaList.java >> line 131: >> >>> 129: MemorySegment struct = allocator.allocate(layout); >>> 130: struct.copyFrom(segment.asSlice(0L, >>> layout.byteSize())); >>> 131: segment = segment.asSlice(VA_SLOT_SIZE_BYTES); >> >> Since arguments are packed according to alignment, I guess the offset could >> be larger or smaller than 8 bytes as well? > > This is using `alignUp(arg.layout.byteSize(), VA_SLOT_SIZE_BYTES)` in the > writing code, so I think it should be the same here? Yes this is a mistake (`skip()` has the same problem). I'll add an extra case to VaListTest to catch it. - PR: https://git.openjdk.java.net/jdk/pull/3617
Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v4]
On Thu, 6 May 2021 07:41:00 GMT, Alan Bateman wrote:
>> Jason Zaugg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove redundant cast from previous commit.
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1235:
>
>> 1233: synchronized(ch) {
>> 1234: return ch.position(pos).read(bb);
>> 1235: }
>
> I think it's okay to include the update to EntryInputStream, that part looks
> fine, as does the directly use of the FileChannel positional read.
>
> I'm still mulling over the case where ch is not a FileChannel as I would
> expected it to capture the existing position and restore it after the read. I
> think this is the degenerative case when the zip file is located in a custom
> file system that doesn't support FileChannel. In that case, positional read
> has to be implemented on the most basic SeekableByteChannel. It would only be
> observed when mixing positional read ops with other ops that depend on the
> current position.
Here are all the references to `ch`.
this.ch = Files.newByteChannel(zfpath, READ);
...
this.ch.close();
...
ch.close(); // close the ch just in case no update
...
if (ch instanceof FileChannel fch) {
return fch.read(bb, pos);
} else {
synchronized(ch) {
return ch.position(pos).read(bb);
}
}
...
long ziplen = ch.size();
...
ch.close();
It appears the only position-dependent operation called `read(ByteBuffer)`.
This is performed together with the `pos` call within the `synchronized(ch)`
lock.
-
PR: https://git.openjdk.java.net/jdk/pull/3853
Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v4]
On Thu, 6 May 2021 00:03:20 GMT, Jason Zaugg wrote:
>> If the given Path represents a file, use the overload of read defined
>> in FileChannel that accepts an explicit position and avoid serializing
>> reads.
>>
>> Note: The underlying NIO implementation is not required to implement
>> FileChannel.read(ByteBuffer, long) concurrently; Windows still appears
>> to lock, as it returns true for NativeDispatcher.needsPositionLock.
>>
>>
>> On MacOS X, the enclosed benchmark improves from:
>>
>>
>> BenchmarkMode Cnt Score Error Units
>> ZipFileSystemBenchmark.read avgt 10 75.311 ? 3.301 ms/op
>>
>>
>> To:
>>
>>
>> BenchmarkMode Cnt Score Error Units
>> ZipFileSystemBenchmark.read avgt 10 12.520 ? 0.875 ms/op
>
> Jason Zaugg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove redundant cast from previous commit.
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1235:
> 1233: synchronized(ch) {
> 1234: return ch.position(pos).read(bb);
> 1235: }
I think it's okay to include the update to EntryInputStream, that part looks
fine, as does the directly use of the FileChannel positional read.
I'm still mulling over the case where ch is not a FileChannel as I would
expected it to capture the existing position and restore it after the read. I
think this is the degenerative case when the zip file is located in a custom
file system that doesn't support FileChannel. In that case, positional read has
to be implemented on the most basic SeekableByteChannel. It would only be
observed when mixing positional read ops with other ops that depend on the
current position.
-
PR: https://git.openjdk.java.net/jdk/pull/3853
Re: RFR: 8266168: -Wmaybe-uninitialized happens in check_code.c
On Thu, 29 Apr 2021 06:54:33 GMT, Yasumasa Suenaga wrote: > We can see following compiler warning in check_code.c on GCC 11. LGTM. I am not aware of any platform where sizeof(char) is not 1, but good to be prepared :) - Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3787
Re: [11u] RFR 8266352: Add parallel heap iteration for jmap –histo
On 06/05/2021 04:47, buddyliao(廖彬) wrote: Dear all, Would you help me to review the following backport from jdk-master? I think you are looking for jdk-updates-dev, the folks there will guide you through how to get approval for these serviceability and GC changes. -Alan
Re: RFR: 8266168: -Wmaybe-uninitialized happens in check_code.c
On Thu, 29 Apr 2021 06:54:33 GMT, Yasumasa Suenaga wrote: > We can see following compiler warning in check_code.c on GCC 11. Ping: Could you review this PR? We still see this warning with GCC 11.1. - PR: https://git.openjdk.java.net/jdk/pull/3787
Re: RFR: 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native
On Mon, 26 Apr 2021 12:52:54 GMT, Jorn Vernee wrote: >> macOS on Apple silicon uses slightly different ABI conventions to the >> standard AArch64 ABI. The differences are outlined in [1]. In >> particular in the standard (AAPCS) ABI, variadic arguments may be passed >> in either registers or on the stack following the normal calling >> convention. To handle this, va_list is a struct containing separate >> pointers for arguments located in integer registers, floating point >> registers, and on the stack. Apple's ABI simplifies this by passing all >> variadic arguments on the stack and the va_list type becomes a simple >> char* pointer. >> >> This patch adds a new MacOsAArch64 CABI type and MacOsAArch64Linker to >> represent the new ABI variant on macOS. StackVaList is based on >> WinVaList lightly modified to handle the different TypeClasses on >> AArch64. The original AArch64Linker is renamed to AapcsLinker and is >> currently used for all non-Mac platforms. I think we also need to add a >> WinAArch64 CABI but I haven't yet been able to test on a Windows system >> so will do that later. >> >> The macOS ABI also uses a different method of spilling arguments to the >> stack (the standard ABI pads each argument to a multiple of 8 byte stack >> slots, but the Mac ABI packs arguments according to their natural >> alignment). None of the existing tests exercise this so I'll open a new >> JBS issue and work on that separately. >> >> Tested jdk_foreign on macOS AArch64, Linux AArch64, and Linux X86_64. >> >> [1] >> https://developer.apple.com/documentation/xcode/writing_arm64_code_for_apple_platforms > > Hi Nick. Sorry for the late reply, I've been out sick. I'll hopefully be > taking a thorough look at this soon (still catching up on things). > > I'm pretty impressed that such a large amount of code can just be shared > between the two platforms :) @JornVernee thanks for the review. I'll park this until the JEP is integrated and then fix it up afterwards. - PR: https://git.openjdk.java.net/jdk/pull/3617
Re: RFR: 8266598: Exception values for AnnotationTypeMismatchException are not always informative [v2]
> This improves the messages that are provided by > `AnnotationTypeMismatchException`s. The message provided by > AnnotationTypeMismatchExceptions is deliberately undefined such that this > should not break any contract. Rafael Winterhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8266598: Exception values for AnnotationTypeMismatchException are not always informative - Changes: - all: https://git.openjdk.java.net/jdk/pull/3892/files - new: https://git.openjdk.java.net/jdk/pull/3892/files/5e61d239..57ab3721 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3892=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3892=00-01 Stats: 2 lines in 2 files changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3892.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3892/head:pull/3892 PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v2]
On Wed, 5 May 2021 13:20:13 GMT, Vladimir Sitnikov wrote: >> We usually don't do that for OutOfMemoryError because concatenation implies >> allocation > > Well, here `OutOfMemory` is more like "I can't read all the data since the > array won't fit". > It would definitely help to have actual file size there, especially in case > filesystem returns weird values (e.g. negative file length). > > This branch will almost never be taken, and even if taken, it won't be "low > memory" condition. So the allocation does not hurt, however, it would > simplify the analysis should the case trigger. What about adding the filename into exception message? - PR: https://git.openjdk.java.net/jdk/pull/3845
