On Mon, 31 May 2021 06:37:40 GMT, Ambarish Rapte <ara...@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 kcr (Lead).

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

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

Reply via email to