On Sat, 19 Aug 2023 20:57:27 GMT, Glavo <[email protected]> wrote: > I mainly made these optimizations: > > * Avoid allocating `StringBuilder` when there are no characters in the URL > that need to be encoded; > * Implement a fast path for UTF-8. > > In addition to improving performance, these optimizations also reduce > temporary objects: > > * It no longer allocates any object when there are no characters in the URL > that need to be encoded; > * The initial size of StringBuilder is larger to avoid expansion as much as > possible; > * For UTF-8, the temporary `CharArrayWriter`, strings and byte arrays are no > longer needed. > > The results of the `URLEncodeDecode` benchmark: > > > Before: > Benchmark (count) (maxLength) (mySeed) Mode Cnt > Score Error Units > URLEncodeDecode.testEncodeUTF8 1024 1024 3 avgt 15 > 5.587 ? 0.010 ms/op > > After: > Benchmark (count) (maxLength) (mySeed) Mode Cnt > Score Error Units > URLEncodeDecode.testEncodeUTF8 1024 1024 3 avgt 15 > 3.582 ? 0.054 ms/op > > > I also updated the tests to add more test cases.
Hello Glavo, for changes like these, I think it would be more productive and useful to create a mailing list discussion first to provide some context on why this change is needed and gathering inputs from people familiar with this code on whether this change is necessary and worth it. Such discussions will then give the Reviewers some context and inputs on what needs to be considered in these changes and to what extent the changes should be done in the code. With the proposed changes in this PR which touches the character encoding handling and such, I think this will need a very thorough review keeping aside the performance aspects. I don't have enough experience of this class to know if it's worth doing this amount of change for any kind of performance improvements which may not be visible outside of micro benchmarks. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15354#issuecomment-1689424850
