Re: Optimizing InputStream.skip(long)

2021-10-10 Thread -
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

2021-10-10 Thread Sergey Bylokhov
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

2021-10-10 Thread Joe Darcy
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

2021-10-10 Thread Pavel Rappo
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

2021-10-10 Thread Brian Goetz
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

2021-10-10 Thread Pavel Rappo
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

2021-10-10 Thread Jim Laskey
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()

2021-10-10 Thread Сергей Цыпанов
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]

2021-10-10 Thread Markus KARG
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