Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
On Wed, 13 Apr 2022 22:59:13 GMT, liach wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add jmh
>
> test/micro/org/openjdk/bench/java/io/InputStreamSkipBenchmark.java line 54:
>
>> 52: @Benchmark
>> 53: public long testSkip0(Data data) throws IOException {
>> 54: TestBaseInputStream0 testBaseInputStream = new
>> TestBaseInputStream0(data.inputStreamSize);
>
> Instead of creating 3 methods with identical bodies, I recommend using an
> enum to represent the type of buffers. An example at
> https://github.com/openjdk/jdk/blob/7a9844cb1cd18c18ce097741cba7db1148c83de0/test/micro/org/openjdk/bench/java/util/HashMapBench.java#L65-L71
>
> For enum values, you can name them like `LOCAL_VARIABLE`, `FIELD`,
> `SOFT_REFERENCE`, which is more descriptive than current "0, 1, 2," etc.
@liach done.
-
PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
On Wed, 13 Apr 2022 22:53:11 GMT, liach wrote: > I suggest we actually write into the byte array to better simulate overheads > (maybe by generating a number with `ThreadLocalRandom`). This sounds reasonable, as each InputStream implementation always need to copy it. > To simulate overhead on each read call, you can perform some extra activity > consumed by the blackhole (possibly pass jmh blackhole through input stream > constructor) This one not, as each InputStream implementation have different read cost, and there is no way to simulate a good value. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
On Tue, 12 Apr 2022 23:35:06 GMT, liach wrote: > Shouldn't be too problematic, as most skip usages in JDK as I see are > skipping small number of bytes like 2 or 4, or like skipping over attributes > of Java class files. A minimum skip buffer size isn't that helpful, as I > don't think we often see skip calls with slowly incremental sizes. Yep usually people skip for fixed length... It is rare to see people skip larger and larger... - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
On Thu, 14 Apr 2022 15:36:20 GMT, Roger Riggs wrote: > Can you summarize the performance results and implications. Including the > whole jmh run isn't necessary. @RogerRiggs In short, making such a cache can help performance, and the larger int skip length people call `skip()`, the larger difference. And adding SoftReference mechanism have a little acceptable performance cost. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
On Wed, 13 Apr 2022 21:58:06 GMT, XenoAmess wrote: >> @jmehrens what about this then? >> I think it safe now(actually this mechanism is learned from Reader) > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > add jmh Can you summarize the performance results and implications. Including the whole jmh run isn't necessary. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
On Wed, 13 Apr 2022 21:58:06 GMT, XenoAmess wrote:
>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add jmh
test/micro/org/openjdk/bench/java/io/InputStreamSkipBenchmark.java line 54:
> 52: @Benchmark
> 53: public long testSkip0(Data data) throws IOException {
> 54: TestBaseInputStream0 testBaseInputStream = new
> TestBaseInputStream0(data.inputStreamSize);
Instead of creating 3 methods with identical bodies, I recommend using an enum
to represent the type of buffers. An example at
https://github.com/openjdk/jdk/blob/7a9844cb1cd18c18ce097741cba7db1148c83de0/test/micro/org/openjdk/bench/java/util/HashMapBench.java#L65-L71
For enum values, you can name them like `LOCAL_VARIABLE`, `FIELD`,
`SOFT_REFERENCE`, which is more descriptive than current "0, 1, 2," etc.
test/micro/org/openjdk/bench/java/io/InputStreamSkipBenchmark.java line 127:
> 125:
> 126: @Override
> 127: public int read(byte[] b, int off, int len) throws IOException {
I suggest we actually write into the byte array to better simulate overheads
(maybe by generating a number with `ThreadLocalRandom`). Otherwise, the
benchmark may have exaggerated the performance gains of large skips.
To simulate overhead on each read call, you can perform some extra activity
consumed by the blackhole (possibly pass jmh blackhole through input stream
constructor)
-
PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: 8284638: store skip buffers in InputStream Object [v4]
> @jmehrens what about this then? > I think it safe now(actually this mechanism is learned from Reader) XenoAmess has updated the pull request incrementally with one additional commit since the last revision: add jmh - Changes: - all: https://git.openjdk.java.net/jdk/pull/5872/files - new: https://git.openjdk.java.net/jdk/pull/5872/files/85cec668..7a9844cb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=02-03 Stats: 288 lines in 1 file changed: 288 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5872/head:pull/5872 PR: https://git.openjdk.java.net/jdk/pull/5872
