Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
I don't think it is a perfect solution to combine the close operation
with the terminator operation, because there are similar issues
in the intermediate operation.
Consider this use case (for example only):
try (var stream = Files.list(path)
.flatMap(dir -> {
try {
return Files.list(dir);
} catch (IOException e) {
return Stream.empty();
}
})) {
// ...
}
It looks closed, but it doesn't. Closing the Stream generated by flatMap
will not close all member Stream. Further consideration is needed to
deal with the problem of closing the Stream. Perhaps it is a withCleaner
method that registers the Stream with the cleaner, or other solutions.
Tagir F.Valeev 于2021年10月4日周一 下午2:52写道：
> Currently, when the stream holds a resource, it's necessary to wrap it
> with try-with-resources. This undermines the compact and fluent style of
> stream API calls. For example, if we want to get the `List` of files inside
> the directory and timely close the underlying filehandle, we should use
> something like this:
>
>
> List paths;
> try (Stream stream = Files.list(Path.of("/etc"))) {
> paths = stream.toList();
> }
> // use paths
>
>
> I suggest to add a new default method to Stream interface named
> `consumeAndClose`, which allows performing terminal stream operation and
> closing the stream at the same time. It may look like this:
>
>
> default R consumeAndClose(Function, ? extends R>
> function) {
> Objects.requireNonNull(function);
> try(this) {
> return function.apply(this);
> }
> }
>
>
> Now, it will be possible to get the list of the files in the fluent manner:
>
>
> List list =
> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
>
> -
>
> Commit messages:
> - Fix tests
> - 8274412: Add a method to Stream API to consume and close the stream
> without using try-with-resources
>
> Changes: https://git.openjdk.java.net/jdk/pull/5796/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5796=00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8274412
> Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod
> Patch: https://git.openjdk.java.net/jdk/pull/5796.diff
> Fetch: git fetch https://git.openjdk.java.net/jdk
> pull/5796/head:pull/5796
>
> PR: https://git.openjdk.java.net/jdk/pull/5796
>
Withdrawn: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
On Sun, 3 Oct 2021 11:00:25 GMT, Tagir F. Valeev wrote:
> Currently, when the stream holds a resource, it's necessary to wrap it with
> try-with-resources. This undermines the compact and fluent style of stream
> API calls. For example, if we want to get the `List` of files inside the
> directory and timely close the underlying filehandle, we should use something
> like this:
>
>
> List paths;
> try (Stream stream = Files.list(Path.of("/etc"))) {
> paths = stream.toList();
> }
> // use paths
>
>
> I suggest to add a new default method to Stream interface named
> `consumeAndClose`, which allows performing terminal stream operation and
> closing the stream at the same time. It may look like this:
>
>
> default R consumeAndClose(Function, ? extends R>
> function) {
> Objects.requireNonNull(function);
> try(this) {
> return function.apply(this);
> }
> }
>
>
> Now, it will be possible to get the list of the files in the fluent manner:
>
>
> List list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.java.net/jdk/pull/5796
Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
Hello!
> I am not really sure we’ve gotten the right idiom here yet. I’d like to slow
> down a bit before making an API decision.
>
> What id suggest is capturing the proposed api and spec on list, and let’s let
> this sit and bake for a bit longer. My sense is that something better may
> well emerge if we do.
I see, thanks for the discussion. Closing the PR for now.
>
> Sent from my MacBook Wheel
>
> > On Oct 9, 2021, at 5:41 AM, Tagir F.Valeev wrote:
> >
> > On Sun, 3 Oct 2021 11:00:25 GMT, Tagir F. Valeev
> > wrote:
> >
> >> Currently, when the stream holds a resource, it's necessary to wrap it
> >> with try-with-resources. This undermines the compact and fluent style of
> >> stream API calls. For example, if we want to get the `List` of files
> >> inside the directory and timely close the underlying filehandle, we should
> >> use something like this:
> >>
> >>
> >> List paths;
> >> try (Stream stream = Files.list(Path.of("/etc"))) {
> >>paths = stream.toList();
> >> }
> >> // use paths
> >>
> >>
> >> I suggest to add a new default method to Stream interface named
> >> `consumeAndClose`, which allows performing terminal stream operation and
> >> closing the stream at the same time. It may look like this:
> >>
> >>
> >>default R consumeAndClose(Function, ? extends R>
> >> function) {
> >>Objects.requireNonNull(function);
> >>try(this) {
> >>return function.apply(this);
> >>}
> >>}
> >>
> >>
> >> Now, it will be possible to get the list of the files in the fluent manner:
> >>
> >>
> >> List list =
> >> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
> >
> > CSR is [posted](https://bugs.openjdk.java.net/browse/JDK-8274994), please
> > review!
> >
> > -
> >
> > PR: https://git.openjdk.java.net/jdk/pull/5796
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
> This PR improves the performance of vector operations that accept masks on > architectures that support masking in hardware, specifically Intel AVX512 and > ARM SVE. > > On architectures that do not support masking in hardware the same technique > as before is applied to most operations, specifically composition using blend. > > Masked loads/stores are a special form of masked operation that require > additional care to ensure out-of-bounds access throw exceptions. The range > checking has not been fully optimized and will require further work. > > No API enhancements were required and only a few additional tests were needed. Paul Sandoz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - Merge branch 'master' into JDK-8271515-vector-api - Apply patch from https://github.com/openjdk/panama-vector/pull/152 - Apply patch from https://github.com/openjdk/panama-vector/pull/142 - Apply patch from https://github.com/openjdk/panama-vector/pull/139 - Apply patch from https://github.com/openjdk/panama-vector/pull/151 - Add new files. - 8271515: Integration of JEP 417: Vector API (Third Incubator) - Changes: https://git.openjdk.java.net/jdk/pull/5873/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5873=02 Stats: 21998 lines in 106 files changed: 16227 ins; 2077 del; 3694 mod Patch: https://git.openjdk.java.net/jdk/pull/5873.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5873/head:pull/5873 PR: https://git.openjdk.java.net/jdk/pull/5873
Re: SourceVersion::feature
PS See https://github.com/openjdk/jdk/pull/5973
-Joe
On 10/14/2021 1:53 PM, Joseph D. Darcy wrote:
On 10/14/2021 10:23 AM, Michael Bien wrote:
is this the right mailing list for javax.lang.model.* discussions?
The compiler-dev list would be appropriate as well, but core-libs will
work.
First, I understand the desire for a method like this. One of the
potential issues is SourceVersion models language versions and while,
to date, there has been a simple linear progression, that is not
guaranteed to be case arbitrarily far into the future, even though it
is the most likely outcome.
Since java.lang.Runtime.Version is in the platform now, I think this
request would be better satisfied via a
public static SourceVersion valueOf(Runtime.Version version)
method on SourceVersion. That way the modeling of SourceVersion can
absorb any non-linearity in versioning and presumably provide
sufficient information for a Runtime.Version -> SourceVersion mapping
to query.
I've filed the RFE
JDK-8275308: Add valueOf(Runtime.Version) factory to SourceVersion
Cheers,
-Joe
if yes:
instead of adding
/**
* Returns the version as int representing the feature release.
* @see Runtime.Version#feature()
*/
public int feature() {
return this.ordinal();
}
to SourceVersion.
a note could be added to the doc that the ordinal can be used as
feature version. Since this statement would apply to the past too,
the note could be backported to all maintained JDKs. This comes with
the usual downside of not being able to add enums for in-between
versions in future.
(doing both would be an option too of course)
To not use the ordinal, client code has to go through some enum init
rituals to be able to do version comparisons:
final static SOURCE_VERSION_RELEASE_18;
static {
SourceVersion tmp18;
try {
tmp18 = SourceVersion.valueOf("RELEASE_18");
} catch (IllegalArgumentException ex) {
tmp18 = null;
}
SOURCE_VERSION_RELEASE_18 = tmp18;
//... more versions
}
just to be able to
if (SOURCE_VERSION_RELEASE_18 != null &&
version.compareTo(SOURCE_VERSION_RELEASE_18) >= 0) {}
or
if
(Integer.parseInt(version.name().substring(version.name().indexOf('_')+1))
>= 18) {}
which is shorter but not a good solution either.
what the client code actually wants is:
if (SourceVersion.latest().feature() >= 18) {}
it was a wise decision for java.lang.Runtime.Version to not use enums :)
best regards,
michael
On 09.10.21 20:58, Michael Bien wrote:
Hello,
could javax.lang.model.SourceVersion receive a feature() method
returning the version as an int, analog to java.lang.Runtime.Version?
if (SourceVersion.latest().feature() >= 18) {}
is simpler than comparing enums which may or may not exist dependent
on the deployed java version and cleaner than ordinal() tricks.
best regards,
michael
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Fri, 15 Oct 2021 20:19:31 GMT, Vladimir Kozlov wrote: > > > How you verified correctness of results? I suggest to extend > > > `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover > > > unsigned method. > > > > > > Tests for unsignedMultiplyHigh were already added in > > test/jdk//java/lang/Math/MultiplicationTests.java (in July 2021 by Brian > > Burkhalter). Used that test to verify the correctness of the results. > > Good. It seems I have old version of the test. Did you run it with -Xcomp? > How you verified that intrinsic is used? The tests were not run with -Xcomp. Looked at the generated x86 code using the disassembler (hsdis) and verified that that correct instruction (mul) instead of (imul, without the intrinsic) was being generated. - PR: https://git.openjdk.java.net/jdk/pull/5933
Integrated: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
On Wed, 13 Oct 2021 17:43:29 GMT, Lance Andersen wrote: > Hi all, > > Please review the fix to address a javadoc issue for the Deflater::deflate > methods that were added as part of JDK-6341887 that could throw a > ReadOnlyBufferException. > > The` @throws ` clause for `ReadOnlyBufferException` was inadvertently > omitted from the javadoc for these new methods. > > A CSR, JDK-8275164, has also been created for this issue. > > Best > Lance This pull request has now been integrated. Changeset: 831802dd Author:Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/831802ddb103f8f9747a9fb139af8365924da801 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException Reviewed-by: bpb, iris, naoto - PR: https://git.openjdk.java.net/jdk/pull/5931
Integrated: 8275145: file.encoding system property has an incorrect value on Windows
On Thu, 14 Oct 2021 15:50:28 GMT, Naoto Sato wrote: > Fixing a regression in which `file.encoding` was initialized by > `sprops->encoding` instead of `sprops->sun_jnu_encoding` on non macOS > platforms. tier1-3 tests passed on all platforms. This pull request has now been integrated. Changeset: ad9e234f Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/ad9e234f5ed61635f926618a40f453fe7b6b491f Stats: 9 lines in 1 file changed: 8 ins; 0 del; 1 mod 8275145: file.encoding system property has an incorrect value on Windows Reviewed-by: mchung, iris, rriggs, alanb - PR: https://git.openjdk.java.net/jdk/pull/5953
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Fri, 15 Oct 2021 19:19:13 GMT, Vamsi Parasa wrote: > > How you verified correctness of results? I suggest to extend > > `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover unsigned > > method. > > Tests for unsignedMultiplyHigh were already added in > test/jdk//java/lang/Math/MultiplicationTests.java (in July 2021 by Brian > Burkhalter). Used that test to verify the correctness of the results. Good. It seems I have old version of the test. Did you run it with -Xcomp? How you verified that intrinsic is used? - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275342: Change nested classes in java.prefs to static nested classes
On Fri, 15 Oct 2021 18:51:44 GMT, Andrey Turbanov wrote:
> Non-static classes hold a link to their parent classes, which can be avoided.
src/java.prefs/macosx/classes/java/util/prefs/MacOSXPreferencesFile.java line
102:
> 100: }
> 101:
> 102: private class SyncTask extends TimerTask {
IDEA suggested to add `static` here too. But this class is unused. And I think
it's better to drop it.
-
PR: https://git.openjdk.java.net/jdk/pull/5971
RFR: 8275342: Change nested classes in java.prefs to static nested classes
Non-static classes hold a link to their parent classes, which can be avoided. - Commit messages: - [PATCH] Change nested classes in java.prefs to static nested classes Changes: https://git.openjdk.java.net/jdk/pull/5971/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5971=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275342 Stats: 20 lines in 3 files changed: 1 ins; 11 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5971.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5971/head:pull/5971 PR: https://git.openjdk.java.net/jdk/pull/5971
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v5]
On Fri, 15 Oct 2021 09:30:18 GMT, Mitsuru Kariya wrote: >> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in >> the following cases: >> >> 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= >> this.length()` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should end successfully. >>(test31) >> >> 2. `pos - 1 + length > this.length()` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should end successfully. *1 >>(test32) >> >> 3. `pos == this.length() + 1` >>The original implementation throws `SerialException` but this case should >> end successfully. *2 >>(test33) >> >> 4. `length < 0` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should throw `SerialException`. >>(test34) >> >> 5. `offset + length > Integer.MAX_VALUE` >>The original implementation throws `ArrayIndexOutOfBoundsException` (or >> `OutOfMemoryError` in most cases) but this case should throw >> `SerialException`. >>(test35) >> >> Additionally, fix `SerialClob.setString(long pos, String str, int offset, >> int length)` in the following cases: >> >> 1. `offset > str.length()` >>The original implementaion throws `StringIndexOutOfBoundsException` but >> this case should throw `SerialException`. >>(test39) >> >> 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= >> this.length()` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should end successfully. >>(test32) >> >> 3. `pos - 1 + length > this.length()` >>The original implementaion throws `SerialException` but this case should >> end successfully. *3 >>(test40) >> >> 4. `pos == this.length() + 1` >>The original implementaion throws `SerialException` but this case should >> end successfully. *4 >>(test41) >> >> 5. `length < 0` >>The original implementation throws `StringIndexOutOfBoundsException` but >> this case should throw `SerialException`. >>(test42) >> >> 6. `offset + length > Integer.MAX_VALUE` >>The original implementation throws `ArrayIndexOutOfBoundsException` (or >> `OutOfMemoryError` in most cases) but this case should throw >> `SerialException`. >>(test43) >> >> >> The javadoc has also been modified according to the above. >> >> *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob >> value is reached while writing the array of bytes, then the length of the >> Blob value will be increased to accommodate the extra bytes." >> >> *2 The documentation of `Blob.setBytes()` says, "If the value specified for >> pos is greater than the length+1 of the BLOB value then the behavior is >> undefined." >>So, it should work correctly when pos == length+1 of the BLOB value. >> >> *3 The documentation of `Clob.setString()` says, "If the end of the Clob >> value is eached while writing the given string, then the length of the Clob >> value will be increased to accommodate the extra characters." >> >> *4 The documentation of `Clob.setString()` says, "If the value specified for >> pos is greater than the length+1 of the CLOB value then the behavior is >> undefined." >>So, it should work correctly when pos == length+1 of the CLOB value. > > Mitsuru Kariya has updated the pull request incrementally with one additional > commit since the last revision: > > Follow the comment Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4001
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Fri, 15 Oct 2021 16:14:25 GMT, Andrew Haley wrote: >> Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. >> This change show 1.87X improvement on a micro benchmark. > > src/hotspot/share/opto/mulnode.cpp line 468: > >> 466: } >> 467: >> 468: >> //= > > MulHiLNode::Value() and UMulHiLNode::Value() seem to be identical. Perhaps > some refactoring would be in order, maybe make a common shared routine. Sure, will do the refactoring to use a shared routine. > test/micro/org/openjdk/bench/java/lang/MathBench.java line 547: > >> 545: return Math.unsignedMultiplyHigh(long747, long13); >> 546: } >> 547: > > As far as I can tell, the JMH overhead dominates when trying to measure the > latency of events in the nanosecond range. `unsignedMultiplyHigh` should have > a latency of maybe 1.5-2ns. Is that what you saw? Yes, the JMH overhead was dominating the measurement of latency. The latency observed for `unsignedMultiplyHigh` was 2.3ns with the intrinsic enabled. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Fri, 15 Oct 2021 18:45:41 GMT, Vladimir Kozlov wrote: > How you verified correctness of results? I suggest to extend > `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover unsigned > method. Tests for unsignedMultiplyHigh were already added in test/jdk//java/lang/Math/MultiplicationTests.java (in July 2021 by Brian Burkhalter). Used that test to verify the correctness of the results. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Wed, 13 Oct 2021 18:55:10 GMT, Vamsi Parasa wrote: > Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. How you verified correctness of results? I suggest to extend `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover unsigned method. - Changes requested by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8266936: Add a finalization JFR event [v17]
On Fri, 15 Oct 2021 09:27:54 GMT, Markus Grönlund wrote: >> Greetings, >> >> Object.finalize() was deprecated in JDK9. There is an ongoing effort to >> replace and mitigate Object.finalize() uses in the JDK libraries; please see >> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. >> >> We also like to assist users in replacing and mitigating uses in non-JDK >> code. >> >> Hence, this changeset adds a periodic JFR event to help identify which >> classes are overriding Object.finalize(). >> >> Thanks >> Markus > > Markus Grönlund has updated the pull request incrementally with two > additional commits since the last revision: > > - renames > - spelling Thanks for doing the renames. Looks good! - Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4731
Integrated: JDK-8275249: Suppress warnings on non-serializable array component types in jdk.jlink
On Wed, 13 Oct 2021 21:08:41 GMT, Joe Darcy wrote: > After a refinement to the checks under development in #5709, the new checks > examine array types of serial fields and warn if the underlying component > type is not serializable. Per the JLS, all array types are serializable, but > if the base component is not serializable, the serialization process can > throw an exception. > > From "Java Object Serialization Specification: 2 - Object Output Classes": > > "If the object is an array, writeObject is called recursively to write the > ObjectStreamClass of the array. The handle for the array is assigned. It is > followed by the length of the array. Each element of the array is then > written to the stream, after which writeObject returns." > > The jdk.jlink module has one instance of this coding pattern that needs > suppression to compile successfully under the future warning. This pull request has now been integrated. Changeset: f1781851 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/f17818516cf80e6e208309200c98b23919c3cddb Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod 8275249: Suppress warnings on non-serializable array component types in jdk.jlink Reviewed-by: iris - PR: https://git.openjdk.java.net/jdk/pull/5936
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov wrote:
>> This change implements a new service provider interface for host name and
>> address resolution, so that java.net.InetAddress API can make use of
>> resolvers other than the platform's built-in resolver.
>>
>> The following API classes are added to `java.net.spi` package to facilitate
>> this:
>> - `InetAddressResolverProvider` - abstract class defining a service, and
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the
>> platform's built-in configuration for resolution operations that could be
>> used to bootstrap a resolver construction, or to implement partial
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the
>> characteristics of one forward lookup operation.
>>
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>>
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Add @since tags to new API classes
> - Add checks and test for empty stream resolver results
test/jdk/java/net/spi/InetAddressResolverProvider/providers/recursive/recursive.init.provider/impl/InetAddressUsageInGetProviderImpl.java
line 38:
> 36: String localHostName;
> 37: try {
> 38: localHostName = InetAddress.getLocalHost().getHostName();
Try to call InetAddress.getLocalHost().getHostName(); twice here.
test/lib/jdk/test/lib/net/IPSupport.java line 88:
> 86: } catch (SocketException se2) {
> 87: return false;
> 88: }
The implementation of this method now conflicts with its name. Maybe we should
pass a `Class` as parameter, and construct the loopback
address from there. IIRC the issue with the original implementation was that it
would say that IPv6 was not supported if IPv6 had only been disabled on the
loopback interface - and that caused trouble because the native implementation
of the built-in resolver had a different view of the situation - and saw that
an IPv6 stack was available on the machine. Maybe this would deserve a comment
too - so that future maintainers can figure out what we are trying to do here.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Wed, 13 Oct 2021 18:55:10 GMT, Vamsi Parasa wrote: > Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. test/micro/org/openjdk/bench/java/lang/MathBench.java line 547: > 545: return Math.unsignedMultiplyHigh(long747, long13); > 546: } > 547: As far as I can tell, the JMH overhead dominates when trying to measure the latency of events in the nanosecond range. `unsignedMultiplyHigh` should have a latency of maybe 1.5-2ns. Is that what you saw? - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v5]
On Fri, 15 Oct 2021 16:54:58 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-419 [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/419 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > * Fix javadoc issue in VaList > * Fix bug in concurrent logic for shared scope acquire src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/SharedScope.java line 138: > 136: ResourceCleanup prev = (ResourceCleanup) > FST.getAcquire(this); > 137: cleanup.next = prev; > 138: ResourceCleanup newSegment = (ResourceCleanup) > FST.compareAndExchangeRelease(this, prev, cleanup); In this place we can actually overwrite CLOSED_LIST (if getAcquired has seen such value). While this particular add will fail later on (since we check witness), another concurrent add might then pass, since it sees a value other than CLOSED_LIST (which is supposed to be a terminal state). Also, after consulting with @DougLea and @shipilev - it seems like using volatile semantics is the way to go here. This should fix a spurious (and very rare) failure on TestResourceScope on arm64. - PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v5]
> This PR contains the API and implementation changes for JEP-419 [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/419 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: * Fix javadoc issue in VaList * Fix bug in concurrent logic for shared scope acquire - Changes: - all: https://git.openjdk.java.net/jdk/pull/5907/files - new: https://git.openjdk.java.net/jdk/pull/5907/files/27e71ee7..5ed94c30 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5907=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5907=03-04 Stats: 10 lines in 2 files changed: 2 ins; 3 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907
Integrated: 8275013: Improve discussion of serialization method declarations in java.io.Object{Input, Output}Stream
On Sun, 10 Oct 2021 20:25:43 GMT, Joe Darcy wrote:
> The java.io.ObjectInputStream and java.io.ObjectOuputStream classes are part
> of the serialization feature. These classes contain brief descriptions of
> some of the methods serializable classes can define to interact with the
> serialization mechanism, readObject, readObjectNoData, and writeObject. These
> descriptions are not entirely consistent with one another and at least one is
> arguably misleading.
>
> This PR makes the brief discussion the same in both classes and addresses the
> misleading point: the throws clauses of the methods will not effect whether
> or not the methods are found by serialization, but throwing unchecked
> exceptions not declared in the standard signatures is ill-advised. (The
> current implementation looks up the methods by name using core reflection;
> the method modifiers are checked to match but the throws information is not.)
>
> Please also review the corresponding CSR :
> https://bugs.openjdk.java.net/browse/JDK-8275014
This pull request has now been integrated.
Changeset: 8c4da9c1
Author:Joe Darcy
URL:
https://git.openjdk.java.net/jdk/commit/8c4da9c15fec7bd27e243e9a6c9ebcad63109506
Stats: 18 lines in 2 files changed: 13 ins; 0 del; 5 mod
8275013: Improve discussion of serialization method declarations in
java.io.Object{Input, Output}Stream
Reviewed-by: smarks, rriggs
-
PR: https://git.openjdk.java.net/jdk/pull/5883
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Wed, 13 Oct 2021 18:55:10 GMT, Vamsi Parasa wrote: > Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. src/hotspot/share/opto/mulnode.cpp line 468: > 466: } > 467: > 468: > //= MulHiLNode::Value() and UMulHiLNode::Value() seem to be identical. Perhaps some refactoring would be in order, maybe make a common shared routine. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Wed, 13 Oct 2021 18:55:10 GMT, Vamsi Parasa wrote: > Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. The `java.lang.Math` change looks good, of course. On the rest I cannot comment but it is good to see something being done here. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov wrote:
>> This change implements a new service provider interface for host name and
>> address resolution, so that java.net.InetAddress API can make use of
>> resolvers other than the platform's built-in resolver.
>>
>> The following API classes are added to `java.net.spi` package to facilitate
>> this:
>> - `InetAddressResolverProvider` - abstract class defining a service, and
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the
>> platform's built-in configuration for resolution operations that could be
>> used to bootstrap a resolver construction, or to implement partial
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the
>> characteristics of one forward lookup operation.
>>
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>>
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Add @since tags to new API classes
> - Add checks and test for empty stream resolver results
src/java.base/share/classes/java/net/InetAddress.java line 231:
> 229: * The first provider found will be used to instantiate the {@link
> InetAddressResolver InetAddressResolver}
> 230: * by invoking the {@link
> InetAddressResolverProvider#get(InetAddressResolverProvider.Configuration)}
> 231: * method. The instantiated {@code InetAddressResolver} will be
> installed as the system-wide
Maybe "... method. The **returned** {@code InetAddressResolver} will be
installed ..."
src/java.base/share/classes/java/net/InetAddress.java line 486:
> 484: return cns;
> 485: } finally {
> 486: bootstrapResolver = null;
This is incorrect. This will set bootstrapResolver to null in the case where
you return it at line 468 - which means that a second call will then find it
null. I believe you want to set it to null in the finally clause only if you
reached line 470. You could achieve this by declaring a local variable - e.g
`InetAddressResolver tempResolver = null;` before entering the try-finally then
set that variable at line 470 `return tempResolver = bootstrapResolver;` and in
finally do `if (tempResolver == null) bootsrapResolver = null;`
To test this you could try to call e.g. `InetAddress.getByName` twice in
succession in your test: I believe it will fail with the code as it stands, but
will succeed with my proposed changes...
src/java.base/share/classes/java/net/InetAddress.java line 860:
> 858: return host;
> 859: }
> 860: } catch (RuntimeException | UnknownHostException e) {
This may deserve a comment: resolver.lookUpHostName and
InetAddress.getAllbyName0 delegate to the system-wide resolver, which could be
custom, and we treat any unexpected RuntimeException thrown by the provider at
that point as we would treat an UnknownHostException or an unmatched host name.
src/java.base/share/classes/java/net/InetAddress.java line 1063:
> 1061: public Stream lookupAddresses(String host,
> LookupPolicy policy)
> 1062: throws UnknownHostException {
> 1063: Objects.requireNonNull(host);
Should we also double check that `policy` is not null? Or maybe assert it?
src/java.base/share/classes/java/net/InetAddress.java line 1203:
> 1201: + " as hosts file " + hostsFile + " not found
> ");
> 1202: }
> 1203: // Check number of found addresses:
I wonder if the logic here could be simplified by first checking whether only
IPv4 addresses are requested, and then if only IPv6 addresses are requested.
That is - evacuate the simple cases first and then only deal with the more
complex case where you have to combine the two lists...
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]
On Thu, 14 Oct 2021 16:05:19 GMT, Mandy Chung wrote: >> src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java >> line 86: >> >>> 84: } >>> 85: >>> 86: private static final int PARAM_COUNT_MASK = 0x00FF; >> >> Is this packing logic required? I get it that it saves footprint - but then >> you have to always unmask bits to get the argument count (see invoke and >> other places). If you keep this, it might be worth documenting what the bit >> layout is supposed to be? > > It's not driven by performance data. It's part of Peter's contribution. I > also prefer it without the packing. I propose to keep the parameter count > as a separate field and examine it when there is footprint issue. The reason for this packing is (was) ORing the value with a non-zero value so that field never held zero value. When for example an individual value (say paramCount) is used in a separate @Stable field, its value set may include the default value (zero) which is then not optimized by JIT as a constant. This is from @Stable docs: * By extension, any variable (either array or field) which has annotated * as stable is called a stable variable, and its non-null or non-zero * value is called a stable value. ...and: * The HotSpot VM relies on this annotation to promote a non-null (resp., * non-zero) component value to a constant, thereby enabling superior * optimizations of code depending on such a value (such as constant folding). * More specifically, the HotSpot VM will process non-null stable fields (final * or otherwise) in a similar manner to static final fields with respect to * promoting the field's value to a constant. Thus, placing aside the * differences for null/non-null values and arrays, a final stable field is * treated as if it is really final from both the Java language and the HotSpot So now that some of these fields are final and not annotated with @Stable any more but are treated as "trusted final" fields, the question is whether JIT is treating the default (zero, null) values differently or not. If they are treated as static final fields where default value makes no difference, then it's ok to split this multi-valued field into individual fields. - PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8275013: Improve discussion of serialization method declarations in java.io.Object{Input, Output}Stream
On Sun, 10 Oct 2021 20:25:43 GMT, Joe Darcy wrote: > The java.io.ObjectInputStream and java.io.ObjectOuputStream classes are part > of the serialization feature. These classes contain brief descriptions of > some of the methods serializable classes can define to interact with the > serialization mechanism, readObject, readObjectNoData, and writeObject. These > descriptions are not entirely consistent with one another and at least one is > arguably misleading. > > This PR makes the brief discussion the same in both classes and addresses the > misleading point: the throws clauses of the methods will not effect whether > or not the methods are found by serialization, but throwing unchecked > exceptions not declared in the standard signatures is ill-advised. (The > current implementation looks up the methods by name using core reflection; > the method modifiers are checked to match but the throws information is not.) > > Please also review the corresponding CSR : > https://bugs.openjdk.java.net/browse/JDK-8275014 Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5883
Re: RFR: 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec
On Fri, 15 Oct 2021 07:17:52 GMT, Сергей Цыпанов wrote: > It looks like we cannot use `Long.hashCode(long)` for > `java.rmi.server.ObjID.hashCode()` due to specification: > https://docs.oracle.com/en/java/javase/17/docs/api/java.rmi/java/rmi/server/ObjID.html#hashCode(). Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5963
RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh
Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. This change show 1.87X improvement on a micro benchmark. - Commit messages: - fix typo in function name - 8275167: x86 intrinsic for unsignedMultiplyHigh Changes: https://git.openjdk.java.net/jdk/pull/5933/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5933=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275167 Stats: 59 lines in 11 files changed: 59 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5933.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5933/head:pull/5933 PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec
On Fri, 15 Oct 2021 11:43:53 GMT, Pavel Rappo wrote: >> It looks like we cannot use `Long.hashCode(long)` for >> `java.rmi.server.ObjID.hashCode()` due to specification: >> https://docs.oracle.com/en/java/javase/17/docs/api/java.rmi/java/rmi/server/ObjID.html#hashCode(). > > It's important to be precise here: it's not "JavaDoc", it's the specification. @pavelrappo you are right. I've reworded the description - PR: https://git.openjdk.java.net/jdk/pull/5963
Re: RFR: 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec
On Fri, 15 Oct 2021 07:17:52 GMT, Сергей Цыпанов wrote: > It looks like we cannot use `Long.hashCode(long)` for > `java.rmi.server.ObjID.hashCode()` due to JavaDoc: > https://docs.oracle.com/en/java/javase/17/docs/api/java.rmi/java/rmi/server/ObjID.html#hashCode(). It's important to be precise here: it's not "JavaDoc", it's the specification. - PR: https://git.openjdk.java.net/jdk/pull/5963
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v4]
On Thu, 14 Oct 2021 02:08:45 GMT, Wu Yan wrote:
>> Hi,
>> Please help me review the change to enhance getting time zone ID from
>> /etc/localtime on linux.
>>
>> We use `realpath` instead of `readlink` to obtain the link name of
>> /etc/localtime, because `readlink` can only read the value of a symbolic of
>> link, not the canonicalized absolute pathname.
>>
>> For example, the value of /etc/localtime is
>> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by
>> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of
>> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which
>> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you
>> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“.
>>
>> Thanks,
>> wuyan
>
> Wu Yan 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 four additional commits since the
> last revision:
>
> - Merge branch 'master' into timezone
> - change functions to be static
> - replace realpath
> - 8273111: Default timezone should return zone ID if /etc/localtime is valid
> but not canonicalization on linux
src/java.base/unix/native/libjava/TimeZone_md.c line 82:
> 80:
> 81: /*
> 82: * remove repeated path separators ('/') in the giving 'path'.
given?
src/java.base/unix/native/libjava/TimeZone_md.c line 89:
> 87: char *left = path;
> 88: char *right = path;
> 89: char *end = path + strlen(path);
"char* end"? better to align with existing code style
src/java.base/unix/native/libjava/TimeZone_md.c line 95:
> 93: for (; right < end; right++) {
> 94: if (*right == '/' && *(right + 1) == '/') break;
> 95: }
Is this for loop necessary? Seems it's ok to merge with the nested loop below.
-
PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8266936: Add a finalization JFR event [v16]
On Thu, 14 Oct 2021 21:43:27 GMT, Coleen Phillimore wrote: >> Markus Grönlund 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. > > src/hotspot/share/jfr/support/jfrSymbolTable.cpp line 75: > >> 73: >> 74: JfrSymbolTable::JfrSymbolTable() : >> 75: _symbol_table(new SymbolTable(this)), > > I'm confused about which symbol table this is. Is this the same SymbolTable > as classfile/symbolTable.cpp? that instance is assumed to be a singleton. Is > this a different SymbolTable and can it have a different name (thought it was > JfrSymbolTable). Renamed. - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v16]
On Thu, 14 Oct 2021 22:36:30 GMT, Markus Grönlund wrote: >> src/hotspot/share/jfr/support/jfrSymbolTable.hpp line 68: >> >>> 66: template class, >>> typename, size_t> >>> 67: friend class HashTableHost; >>> 68: typedef HashTableHost>> JfrSymbolTable> SymbolTable; >> >> Oh here it is. Since it's an enclosed type, can you rename it something >> simpler like Symbols and the other Strings? > > Maybe :) Renamed. - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v5]
> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in > the following cases: > > 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. >(test31) > > 2. `pos - 1 + length > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. *1 >(test32) > > 3. `pos == this.length() + 1` >The original implementation throws `SerialException` but this case should > end successfully. *2 >(test33) > > 4. `length < 0` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test34) > > 5. `offset + length > Integer.MAX_VALUE` >The original implementation throws `ArrayIndexOutOfBoundsException` (or > `OutOfMemoryError` in most cases) but this case should throw > `SerialException`. >(test35) > > Additionally, fix `SerialClob.setString(long pos, String str, int offset, int > length)` in the following cases: > > 1. `offset > str.length()` >The original implementaion throws `StringIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test39) > > 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. >(test32) > > 3. `pos - 1 + length > this.length()` >The original implementaion throws `SerialException` but this case should > end successfully. *3 >(test40) > > 4. `pos == this.length() + 1` >The original implementaion throws `SerialException` but this case should > end successfully. *4 >(test41) > > 5. `length < 0` >The original implementation throws `StringIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test42) > > 6. `offset + length > Integer.MAX_VALUE` >The original implementation throws `ArrayIndexOutOfBoundsException` (or > `OutOfMemoryError` in most cases) but this case should throw > `SerialException`. >(test43) > > > The javadoc has also been modified according to the above. > > *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob value > is reached while writing the array of bytes, then the length of the Blob > value will be increased to accommodate the extra bytes." > > *2 The documentation of `Blob.setBytes()` says, "If the value specified for > pos is greater than the length+1 of the BLOB value then the behavior is > undefined." >So, it should work correctly when pos == length+1 of the BLOB value. > > *3 The documentation of `Clob.setString()` says, "If the end of the Clob > value is eached while writing the given string, then the length of the Clob > value will be increased to accommodate the extra characters." > > *4 The documentation of `Clob.setString()` says, "If the value specified for > pos is greater than the length+1 of the CLOB value then the behavior is > undefined." >So, it should work correctly when pos == length+1 of the CLOB value. Mitsuru Kariya has updated the pull request incrementally with one additional commit since the last revision: Follow the comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/4001/files - new: https://git.openjdk.java.net/jdk/pull/4001/files/69fb2f4a..c5d8056b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4001=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4001=03-04 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4001.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4001/head:pull/4001 PR: https://git.openjdk.java.net/jdk/pull/4001
Re: RFR: 8266936: Add a finalization JFR event [v17]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with two additional commits since the last revision: - renames - spelling - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/96a9d6bf..9b418149 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=16 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=15-16 Stats: 80 lines in 6 files changed: 0 ins; 0 del; 80 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
Re: Optimization (?) of HashSet(Collection)
On 10/4/21 12:57, Сергей Цыпанов wrote:
in the code of HashSet(Collection) we have an optimization opportunity:
public HashSet(Collection c) {
map = new HashMap<>(Math.max((int) (c.size()/.75f) + 1, 16));
addAll(c);
}
instead of using addAll() inherited from j.u.Collection we can use
c.forEach(this::add):
> public HashSet(Collection c) {
map = new HashMap<>(Math.max((int) (c.size()/.75f) + 1, 16));
c.forEach(this::add);
}
This allows to rid Iterator and use counting loops for e.g. ArrayList instead
of hasNext()/next().
Well, perhaps, but this sort of low-level hackery is not IMO something we
should reflexively do in the core class libraries.
We are also likely to benefit from this change in case the argument collection
is synchronized as it's going to be locked only once.
That's a good point, but would a synchronized collection really lock
coarsely around forEach(), rather than in add().
I've used the benchmark [1] to analyze the impact and it demonstrates
measurable improvement [2].
What puzzles we a bit is the result for j.u.ArrayList. At warm-up time it
demonstrates better results than at measurement time:
length = 10
# Run progress: 4,44% complete, ETA 00:21:41
# Fork: 3 of 5
# Warmup Iteration 1: 134,699 ns/op
# Warmup Iteration 2: 115,391 ns/op
# Warmup Iteration 3: 130,110 ns/op
# Warmup Iteration 4: 114,465 ns/op <
# Warmup Iteration 5: 114,849 ns/op
# Warmup Iteration 6: 115,073 ns/op
# Warmup Iteration 7: 113,988 ns/op
# Warmup Iteration 8: 115,301 ns/op
# Warmup Iteration 9: 115,452 ns/op
# Warmup Iteration 10: 124,493 ns/op <
Iteration 1: 123,776 ns/op
Iteration 2: 123,719 ns/op
Iteration 3: 123,725 ns/op
Iteration 4: 126,892 ns/op
Iteration 5: 125,493 ns/op
Iteration 6: 124,661 ns/op
Iteration 7: 123,783 ns/op
Iteration 8: 123,975 ns/op
Iteration 9: 124,047 ns/op
Iteration 10: 123,899 ns/op
length = 100
# Run progress: 11,11% complete, ETA 00:20:10
# Fork: 1 of 5
# Warmup Iteration 1: 1236,695 ns/op
# Warmup Iteration 2: 1069,909 ns/op
# Warmup Iteration 3: 1243,852 ns/op
# Warmup Iteration 4: 1059,038 ns/op <
# Warmup Iteration 5: 1057,670 ns/op
# Warmup Iteration 6: 1057,117 ns/op
# Warmup Iteration 7: 1056,932 ns/op
# Warmup Iteration 8: 1054,909 ns/op
# Warmup Iteration 9: 1058,106 ns/op
# Warmup Iteration 10: 1145,699 ns/op <
Iteration 1: 1139,155 ns/op
Iteration 2: 1141,662 ns/op
Iteration 3: 1139,061 ns/op
Iteration 4: 1139,306 ns/op
Iteration 5: 1138,475 ns/op
Iteration 6: 1140,491 ns/op
Iteration 7: 1144,017 ns/op
Iteration 8: 1139,411 ns/op
Iteration 9: 1142,729 ns/op
Iteration 10: 1144,042 ns/op
Here I use 1 s warm-up iteration on 2s measurement iteration. It looks like at
iteration 4 the code is top-optimized,
and later at the end of warm-up a kind of deoptimization happens. There's still
visible improvement however.
The benchmark is run with 5 forks, and this deoptimization is visible for
length = {10, 100}.
So my two question are:
- What is the reason of the deoptimization and can we do something about it?
C2 does this sort of thing all the time: it's a heuristic probabilistic
optimizer. I've seen plenty of bimodal examples, where inlining changes
and each time a program is run it's either fast or slow, quasi-randomly.
If you really want to know, use -prof perfasm in JMH.
- Whether suggested changes is worth implementations?
IMO the gain is too small. Others may disagree.
--
Andrew Haley (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd.
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Thu, 14 Oct 2021 16:51:42 GMT, Raffaello Giulietti wrote: > All have the form 2^q*10^(-k) not quite, for example 0x1.3eaba12cap-804 = 2251808225167717/2^855 is not of this form, but these cases come from the continued fraction expansion of 2^q*10^(-k). - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results "All have the form 2^q10^(-k)" (Paul Zimmermann's reply) not quite, for example 0x1.3eaba12cap-804 = 2251808225167717/2^855 is not of this form, but these cases come from the continued fraction expansion of 2^q10^(-k). (my reply) Right, it is correctly stated in the test class (to be pushed) but incorrectly in my previous post Raffaello - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec
On Fri, 15 Oct 2021 07:17:52 GMT, Сергей Цыпанов wrote: > It looks like we cannot use `Long.hashCode(long)` for > `java.rmi.server.ObjID.hashCode()` due to JavaDoc: > https://docs.oracle.com/en/java/javase/17/docs/api/java.rmi/java/rmi/server/ObjID.html#hashCode(). @stuart-marks FYI - PR: https://git.openjdk.java.net/jdk/pull/5963
RFR: 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec
It looks like we cannot use `Long.hashCode(long)` for `java.rmi.server.ObjID.hashCode()` due to JavaDoc: https://docs.oracle.com/en/java/javase/17/docs/api/java.rmi/java/rmi/server/ObjID.html#hashCode(). - Commit messages: - 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec Changes: https://git.openjdk.java.net/jdk/pull/5963/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5963=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275293 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5963.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5963/head:pull/5963 PR: https://git.openjdk.java.net/jdk/pull/5963
