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

[Guoxiong Li]

On Sun, 3 Jan 2021 17:32:03 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> Hi
>> 
>> In general when closing a performance related issue it is good to provide some kind of evidence that performance has actually improved. Many times I have seen fixes that *should* have solved some performance issue that turned out to not do that. That is why I still like to see some kind of evidence that this fixes the reported issue.
>> 
>> My second comment was about understanding how the code *could* solve the issue. After the explanation from @lgxbslgx I now believe this can be the case, it looks plausible that this will improve the buffering in writes. *However* the fact that I now think this could fix the issue isn't the proof I would look for when closing a performance bug.
>> 
>> I would still like to see some kind of performance metric or benchmark that shows that buffering of writes actually occur, this doesn't need to be a repeatable test, but can be a report or summary from an instrumented test run or something similar. However I do *believe* this should fix the problem so I don't object to Jon sponsoring this if he is confident with the change.
>
> 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)

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

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