[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8239138: StyleManager should use a BufferedInputStream [v4]
From:       Pankaj Bansal <pbansal () openjdk ! java ! net>
Date:       2021-05-31 11:18:23
Message-ID: amXQzAjnFiUiXTw6o0niKCxVNbo-OjRQqgh2HPVUs0w=.33b72f1f-3d34-4068-b5c6-dddf322f0e1d () github ! com
[Download RAW message or body]

On Mon, 31 May 2021 06:37:40 GMT, Ambarish Rapte <arapte@openjdk.org> wrote:

> > `StyleManager.calculateCheckSum()` uses a raw InputStream as the input to a \
> > `DigestInputStream` and reads one byte at a time. This is slower in performance \
> > and should be changed, either to use `BufferedInputStream` or read byte buffer of \
> > 4096 from the stream or use both. 
> > I have tried three approaches and tested with modena.css which is ~134 KB in \
> > size. Following are the approaches and time in milliseconds taken by the method \
> > StyleManager.calculateCheckSum(), collected from 25 readings, 
> > 1. Use BufferedInputStream and read one byte at a time \
> > [commit#1](https://github.com/openjdk/jfx/commit/6cd7d44d0ce1084c6cdb06a698c7aca127a615ef) \
> >                 : 
> > - Maximum: 53 ms,  Minimum: 27 ms, Average: 39 ms
> > 2. Use BufferedInputStream and read buffer of 4096 at a time \
> > [commit#1+2](https://github.com/openjdk/jfx/pull/518/files/6e0c621ea62691d5402b2bca5951d1012a5b1b91)
> >                 
> > - Maximum: 17 ms,  Minimum: 14 ms, Average: 15.58 ms
> > 3. Use raw InputStream(current implementation) and read buffer of 4096 at a time \
> > [commit#1+2+3](https://github.com/openjdk/jfx/pull/518/files/215e1a183cfb902247f0d48685f7a901cb5fb003), \
> > which also similar to `NativeLibLoader.calculateCheckSum()` and looks faster than \
> >                 previous two.
> > - Maximum: 16 ms,  Minimum: 13 ms, Average: 14.25 ms
> > 
> > 
> > The time taken by `StyleManager.calculateCheckSum()` with current implementation \
> >                 is,
> > - Maximum: 61 ms,  Minimum: 38 ms, Average: 50.58 ms
> > 
> > 
> > Both approaches 2 & 3 show good improvement. I would prefer approach#3 as it is \
> > similar to `NativeLibLoader.calculateCheckSum()`. However we can choose \
> > approach#2 also. If we choose approach#3 then bug summary should be changed \
> > accordingly.
> 
> Ambarish Rapte 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 additional commits since the last \
> revision: 
> - Merge branch 'master' into SM_BufferedInputStream_8239138
> - review update
> - minor: format correction
> - add test
> - only buffer[4096], similar to NativeLibLoader.calculateCheckSum()
> - BufferedInputStream + buffer[4096]
> - use BufferedInputStream

Marked as reviewed by pbansal (Committer).

-------------

PR: https://git.openjdk.java.net/jfx/pull/518


[prev in list] [next in list] [prev in thread] [next in thread]