RFR: 8239138: StyleManager should use a BufferedInputStream [v3]

[Kevin Rushforth]

On Thu, 27 May 2021 17:22:35 GMT, Ambarish Rapte <arapte at 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 incrementally with one additional commit since the last revision:
> 
>   review update

The fix and test look fine now. Unfortunately, if you look at the checks for this PR, the test fails on the GitHub Actions Windows build:


2021-05-27T17:31:48.5184758Z test.com.sun.javafx.css.StyleManagerTest > testCalculateCheckSum FAILED
2021-05-27T17:31:48.5186991Z     java.lang.AssertionError: 
2021-05-27T17:31:48.5188560Z         at org.junit.Assert.fail(Assert.java:91)
2021-05-27T17:31:48.5190261Z         at org.junit.Assert.assertTrue(Assert.java:43)
2021-05-27T17:31:48.5192290Z         at org.junit.Assert.assertTrue(Assert.java:54)
2021-05-27T17:31:48.5195303Z         at test.com.sun.javafx.css.StyleManagerTest.testCalculateCheckSum(StyleManagerTest.java:1121)
2021-05-27T17:31:49.3450264Z 


This is almost certainly because of DOS line endings courtesy of the native Git for Windows where the default configuration is set to `core.autocrlf = true`. This causes git to convert all files on checkout to Windows-style line endings. I note that others could run into this problem as well -- in fact someone recently ran into an WebKit build error caused by `autocrlf` on their system.

I will file a new bug to add a `.gitattributes` to our repo. I note that the `jdk` repo already did this prior to the Skara switch. See [JDK-8241768](https://bugs.openjdk.java.net/browse/JDK-8241768).

You can either wait for that newly-filed bug to be integrated or else skip this test on Windows, referring to that bug. I'd probably recommend waiting.

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

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