On Tue, 28 Mar 2023 18:15:57 GMT, Eirik Bjorsnos wrote:
> > @eirbjo I've updated the PR so we hold zero-length arrays rather than null
> > arrays. Please re-run your benchmark to see if there is any change.
>
> @minborg I ran the test now after your integration and I no longer observe
> the re
On Thu, 23 Mar 2023 14:00:40 GMT, Eirik Bjorsnos wrote:
>> I cannot explain why, but the changes suggested in this PR seem to introduce
>> a small, but significant performance regression on the
>> `DataInputStreamTest.readInt` benchmark:
>>
>> Baseline:
>>
>>
>> Benchmark
On Tue, 28 Mar 2023 08:13:31 GMT, Per Minborg wrote:
>> This PR proposed to lazily initialize the scratch arrays only if/when needed.
>
> Per Minborg 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
On Tue, 28 Mar 2023 07:21:15 GMT, Per Minborg wrote:
>> This PR proposed to lazily initialize the scratch arrays only if/when needed.
>
> Per Minborg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Use zero-lenght arrays rather than null
W
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Per Minborg 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 seven additi
On Thu, 23 Mar 2023 14:00:40 GMT, Eirik Bjorsnos wrote:
>> I cannot explain why, but the changes suggested in this PR seem to introduce
>> a small, but significant performance regression on the
>> `DataInputStreamTest.readInt` benchmark:
>>
>> Baseline:
>>
>>
>> Benchmark
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Per Minborg has updated the pull request incrementally with one additional
commit since the last revision:
Use zero-lenght arrays rather than null
-
Changes:
- all: https://git.openjdk.org/jdk/pull/
On Wed, 22 Mar 2023 10:12:26 GMT, Per Minborg wrote:
>> This PR proposed to lazily initialize the scratch arrays only if/when needed.
>
> Per Minborg 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
On Thu, 23 Mar 2023 13:51:35 GMT, Eirik Bjorsnos wrote:
> The reason this puzzles me is that the `readInt` benchmark don't involve the
> UTF code paths at all. So why would it become slower, just because two fields
> are no longer initialized with the default arrays?
Could there be some JIT op
On Wed, 22 Mar 2023 10:12:26 GMT, Per Minborg wrote:
>> This PR proposed to lazily initialize the scratch arrays only if/when needed.
>
> Per Minborg 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
On Wed, 22 Mar 2023 10:17:36 GMT, Per Minborg wrote:
> > Nice idea. Just wonder could we do the same for `BufferedInputStream`?
>
> I think it is doable but would require some more work as nullity is used as a
> "closed flag".
I would nervous about changing BIS. The buf field is a protected fi
On Wed, 22 Mar 2023 10:12:26 GMT, Per Minborg wrote:
>> This PR proposed to lazily initialize the scratch arrays only if/when needed.
>
> Per Minborg 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
On Wed, 22 Mar 2023 10:07:57 GMT, Per Minborg wrote:
>> src/java.base/share/classes/java/io/DataInputStream.java line 64:
>>
>>> 62: * working arrays initialized on demand by readUTF
>>> 63: */
>>> 64: private byte[] bytearr;
>>
>> Could the `bytearr` and `chararr` instance variab
On Tue, 21 Mar 2023 20:35:36 GMT, Sergey Tsypanov wrote:
> Nice idea. Just wonder could we do the same for `BufferedInputStream`?
I think it is doable but would require some more work as nullity is used as a
"closed flag". Please raise a separate issue about this.
-
PR Comment: ht
On Tue, 21 Mar 2023 17:27:42 GMT, Eirik Bjorsnos wrote:
> Looks like `DataInputStream.readUTF` could benefit from using
> `JavaLangAccess.countPositives` and return early for the common case that all
> bytes are ASCII. Food for another PR?
Yep. Please raise an issue with the ideas around this
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Per Minborg 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 five additio
On Tue, 21 Mar 2023 16:17:35 GMT, Brian Burkhalter wrote:
>> Per Minborg 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 five additional
>> comm
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Per Minborg has updated the pull request incrementally with one additional
commit since the last revision:
Update src/java.base/share/classes/java/io/DataInputStream.java
Co-authored-by: Eirik Bjorsnos
-
On Tue, 21 Mar 2023 15:41:13 GMT, Per Minborg wrote:
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Nice idea. Just wonder could we do the same for `BufferedInputStream`?
-
PR Comment: https://git.openjdk.org/jdk/pull/13121#issuecomment-1478546070
On Tue, 21 Mar 2023 15:41:13 GMT, Per Minborg wrote:
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Looks like `DataInputStream.readUTF` could benefit from using
`JavaLangAccess.countPositives` and return early for the common case that all
bytes are ASCII. Food
On Tue, 21 Mar 2023 15:41:13 GMT, Per Minborg wrote:
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
src/java.base/share/classes/java/io/DataInputStream.java line 64:
> 62: * working arrays initialized on demand by readUTF
> 63: */
> 64: private byt
On Tue, 21 Mar 2023 15:41:13 GMT, Per Minborg wrote:
> This PR proposed to lazily initialize the scratch arrays only if/when needed.
Looks good. Many DataInputStreams probably never read UTF. (Not a reviewer)
src/java.base/share/classes/java/io/DataInputStream.java line 576:
> 574: cha
This PR proposed to lazily initialize the scratch arrays only if/when needed.
-
Commit messages:
- Remove redundant initilizers
- Lazily initialize (byte, char)arr in java.io.DataInputStream
Changes: https://git.openjdk.org/jdk/pull/13121/files
Webrev: https://webrevs.openjdk.org/
23 matches
Mail list logo
