RFR: 8255729: com.sun.tools.javac.processing.JavacFiler.FilerOutputStream is inefficient [v2]
Joel Borggrén-Franck
jfranck at openjdk.java.net
Sun Jan 3 10:07:58 UTC 2021
On Sat, 2 Jan 2021 22:29:08 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:
>> I'd like to see some kind of evidence that performance was actually improved. I realise this might be hard to make a good standalone test for, but it should be doable as an experiment observing how many times write(int) is called for a huge file.
>
> I read Joel's (@jbf) comment as understanding where you are coming from with the patch.
>
> Since the code now provides the recommended pattern, I don't think any more evidence is needed. I think you're OK to integrate.
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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1854
More information about the compiler-dev
mailing list