[Bug-66532] Improve performance of SheetDataWriter

[rascmatt]

Simplify loop and avoid code point to string conversions.

[rascmatt]

Following you will find the results of a JMH benchmark I ran with 3 different versions. Unfortunately I was unable to get JMH running with Gradle, so I ran this in an external Maven Project.

The latest released version (5.2.3): 'Main.benchOriginal'
An unreleased improvement (#405): 'Main.bench_405'
And the version proposed in this commit: 'Main.bench_66532'

Benchmark Mode Cnt Score Error Units
Main.benchOriginal thrpt 15 520433.760 ± 81525.743 ops/s
Main.bench_405 thrpt 15 579381.912 ± 30395.196 ops/s
Main.bench_66532 thrpt 15 3029062.360 ± 141908.012 ops/s

[pjfanning]

this code is wrong - s.length() is not the number of codepoints - it is the number of chars - in a string.

see https://www.w3resource.com/java-tutorial/string/string_codepointcount.php

[rascmatt]

I might need to improve the readability of the loop, however I think this is covered by line 441, where (in case of a character pair) the counter is additionally increased.

[pjfanning]

loops should not increment in multiple places (ideally) - if you use the codePointCount for the limit of the loop then the count will be correct

[rascmatt]

Yes, but then the codePointAt() method will not receive the correct index. I switched to using the codePoints iterator, which does not seem to have a big performance impact.

[pjfanning]

could you test with '<'? - this should have similar performance but would be more readable

[rascmatt]

Good idea. Done.

[pjfanning]

Following you will find the results of a JMH benchmark I ran with 3 different versions. Unfortunately I was unable to get JMH running with Gradle, so I ran this in an external Maven Project.

The latest released version (5.2.3): 'Main.benchOriginal' An unreleased improvement (#405): 'Main.bench_405' And the version proposed in this commit: 'Main.bench_66532'

Benchmark Mode Cnt Score Error Units Main.benchOriginal thrpt 15 520433.760 ± 81525.743 ops/s Main.bench_405 thrpt 15 579381.912 ± 30395.196 ops/s Main.bench_66532 thrpt 15 3029062.360 ± 141908.012 ops/s

could you share your benchmark code? a GitHub project or some such

[rascmatt]

https://github.com/rascmatt/poi-benchmark
The (updated) benchmark with code and results.

[pjfanning]

thanks - looks good. Let me look over this but it looks like this should be safe to merge over the coming days.

[pjfanning]

I added 0275daa - not exactly what is in your PR. Could you try this out in your benchmark?

[rascmatt]

Thank you, I added this version to the benchmark and it looks good. In addition I added a benchmark for a more integrated setting (rascmatt/poi-benchmark@d7fa4a8). It shows a speedup of about ~45% for the SheetDataWriter.