RFR: 8255729: com.sun.tools.javac.processing.JavacFiler.FilerOutputStream is inefficient [v2]

[Guoxiong Li]

On Sun, 3 Jan 2021 18:54:26 GMT, Guoxiong Li <github.com+13688759+lgxbslgx at openjdk.org> wrote:

>> In general, I agree with @jbf about validating performance improvements. However, I think that this change is as much about cleanup as it is about performance, especially since `FilterOutputStream` specifically recommends that this method _should_ be overridden:
>> 
>> `
>> Note that this method does not call the write method of its underlying output stream with the same arguments.
>> Subclasses of FilterOutputStream should provide a more efficient implementation of this method.
>> `
>> 
>> It is not clear to be that we can write a reasonable performance test, since as has been noted, it depends on the underlying stream. But I do think that the proposed change makes the code cleaner and simpler, and in general will do no worse than before.
>
> I write two tests(small and large file) by using the JMH to verify the performance. I push the test code to [JDK-8255729-Test](https://github.com/lgxbslgx/JDK-8255729-Test) for you to reproduce.
> 
> ---
> **One is a large file.**
> - Using this patch. The result:
> 
> Result "test.T8255729_Large.testWriteTime":
>   18043.388 ±(99.9%) 1696.325 us/op [Average]
>   (min, avg, max) = (13011.716, 18043.388, 28295.985), stdev = 3426.661
>   CI (99.9%): [16347.063, 19739.714] (assumes normal distribution)
> 
> - Using the master branch. The result:
> 
> Result "test.T8255729_Large.testWriteTime":
>   157795.683 ±(99.9%) 2660.637 us/op [Average]
>   (min, avg, max) = (151689.305, 157795.683, 183340.380), stdev = 5374.619
>   CI (99.9%): [155135.046, 160456.320] (assumes normal distribution)
> 
> You can see that the average time of the master branch is about 8.7 times(157795.683/18043.388) that of this patch.
> 
> ---
> **Another is a small file.**
> - Using this patch. The result:
> 
> Result "test.T8255729_Small.testWriteTime":
>   13830.577 ±(99.9%) 1045.225 us/op [Average]
>   (min, avg, max) = (11352.977, 13830.577, 20271.217), stdev = 2111.406
>   CI (99.9%): [12785.352, 14875.802] (assumes normal distribution)
> 
> - Using the master branch. The result:
> 
> Result "test.T8255729_Small.testWriteTime":
>   17166.427 ±(99.9%) 822.553 us/op [Average]
>   (min, avg, max) = (14612.349, 17166.427, 22088.647), stdev = 1661.597
>   CI (99.9%): [16343.875, 17988.980] (assumes normal distribution)
> 
> You can see that the average time of the master branch is about 1.2 times(17166.427/13830.577) that of this patch. The difference is little because it is a small file.
> 
> ---
> And we can consider another aspect.
> - When the file becomes very large, the average time of this patch increases little(13830.577 -> 18043.388).
> - When the file becomes very large, the average time of the master branch increases a lot(17166.427 -> 157795.683).
> 
> ---
> The concrete reports are shown below.
> [JDK-8255729.txt](https://github.com/openjdk/jdk/files/5762068/JDK-8255729.txt)
> [master.txt](https://github.com/openjdk/jdk/files/5762069/master.txt)

And I think this is an issue about performance instead of cleanup.

Whether it is about performance or cleanup, it may only affect the label of the bug, `noreg-cleanup` or `noreg-perf`. So I think we don't need to continue arguing about it.

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

PR: https://git.openjdk.java.net/jdk/pull/1854