Re: Optimizing InputStream.skip(long)
Thanks for the suggestion. Forwarding to the core libs list. On Sun, Oct 10, 2021 at 6:52 PM David Holmes wrote: > > Hi, > > This belongs on core-libs-dev@openjdk.java.net > > Thanks, > David > > On 11/10/2021 9:39 am, - wrote: > > On GitHub, xenoamess sent a pull request [1] that optimizes skipBuffer > > array used by InputStream.skip by caching one for each InputStream > > instance (static ones are unsafe per bug 7000600) like the > > java.io.Reader class does (the reader one is behind a lock, while this > > one is possibly concurrent). > > > > Pros: Input streams that inherit the default skip logic can reduce > > buffer array creation by reusing the compatible old array when the > > skip method is called multiple times. > > > > Cons: This adds a field to InputStream; the array cannot be GC'd until > > the InputStream is GC'd. (But it has a length limit and the impact is > > less) > > > > Additional Info: Most JDK InputStream implementations already > > overrides this method to offer a more efficient implementation as > > suggested in the Javadocs. Reader.skip(long) calls Reader.read(char[], > > int, int), and this change doesn't affect the Reader class. > > > > I wonder if this idea is worthy of a JDK issue so this patch may > > eventually be accepted. Feel free to post any feedback, too! > > > > [1]: https://git.openjdk.java.net/jdk/pull/5872 > >
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov wrote: > 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 240: > 238: * OutOfMemoryError: Requested array size exceeds VM limit > 239: */ > 240: private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; Looks like the usage of this field was removed by the https://github.com/openjdk/lanai/commit/03642a01, note that the doc for the "newCapacity" is still mentioned this field. - PR: https://git.openjdk.java.net/jdk/pull/5878
RFR: 8275013: Improve discussion of serialization method declarations in java.io.Object{Input, Output}Stream
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 - Commit messages: - 8275013: Improve discussion of serialization method declarations in java.io.Object{Input, Output}Stream Changes: https://git.openjdk.java.net/jdk/pull/5883/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5883=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275013 Stats: 18 lines in 2 files changed: 13 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/5883.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5883/head:pull/5883 PR: https://git.openjdk.java.net/jdk/pull/5883
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov wrote: > 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE Oops. I think we should also do something about this occurrence of MAX_ARRAY_SIZE: https://github.com/openjdk/jdk/blob/d679bd3ab8b41a14359d3bfb9763a1178d02ecb0/src/java.base/share/classes/java/lang/AbstractStringBuilder.java#L239 - PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
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. 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: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov wrote: > 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE Marked as reviewed by prappo (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov wrote: > 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE Marked as reviewed by jlaskey (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
On Wed, 6 Oct 2021 09:26:06 GMT, Peter Levart wrote: >> This is trivial fix of >> [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8274686) >> which replaces manually-computed `int`-based `long` hash-code. >> >> Because `Long#hashCode(long)` uses other hashing function than the one >> currently used here: >> >> https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc0/src/java.base/share/classes/java/lang/Long.java#L1440-L1442 >> >> the value of `hashCode()` will produce different result, however this does >> not seem to be a breaking change as `UUID#hashCode()` is not specified. >> >> --- >> >> Note: the comment to the bug states: >> >>> Moved to JDK for further discussions of the proposed enhancement. >> >> But as there seemed to be no corresponding discussion in core-libs-dev and >> the change looks trivial, I considered that it is appropriate to open a PR >> which (if needed) may be used for discussion (especially, considering the >> fact that its comments get mirrored to the ML). > > Yes, the reverted change to BitSet.hashCode in > https://github.com/openjdk/jdk/pull/4309 has the same property. It could be > applied without any visible effect. @plevart should I then file a follow-up ticket for `BitSet`? And should we change the JavaDoc providing there we have computation formula explicitly specified? - PR: https://git.openjdk.java.net/jdk/pull/5811
Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v13]
On Sun, 1 Aug 2021 22:01:33 GMT, Markus KARG wrote: >> This PR-*draft* is **work in progress** and an invitation to discuss a >> possible solution for issue >> [JDK-8265891](https://bugs.openjdk.java.net/browse/JDK-8265891). It is *not >> yet* intended for a final review. >> >> As proposed in JDK-8265891, this PR provides an implementation for >> `Channels.newInputStream().transferTo()` which provide superior performance >> compared to the current implementation. The changes are: >> * Prevents transfers through the JVM heap as much as possibly by offloading >> to deeper levels via NIO, hence allowing the operating system to optimize >> the transfer. >> * Using more JRE heap in the fallback case when no NIO is possible (still >> only KiBs, hence mostl ynegligible even on SBCs) to better perform on modern >> hardware / fast I/O devides. >> >> Using JMH I have benchmarked both, the original implementation and this >> implementation, and (depending on the used hardware and use case) >> performance change was approx. doubled performance. So this PoC proofs that >> it makes sense to finalize this work and turn it into an actual OpenJDK >> contribution. >> >> I encourage everybody to discuss this draft: >> * Are there valid arguments for *not* doing this change? >> * Is there a *better* way to improve performance of >> `Channels.newInputStream().transferTo()`? >> * How to go on from here: What is missing to get this ready for an actual >> review? > > Markus KARG has updated the pull request incrementally with two additional > commits since the last revision: > > - Draft: Eliminated duplicate code using lambda expressions > - Draft: Use blocking mode also for target channel Work on this draft will be continued as soon as depenency PRs are resolved. Please keep this issue open. - PR: https://git.openjdk.java.net/jdk/pull/4263