oblac / jodd-util

Essential Java utilities.
https://util.jodd.org
BSD 2-Clause "Simplified" License
40 stars 9 forks source link

String builder pool2 #4

Closed igr closed 1 year ago

igr commented 4 years ago

@slandelle I got the similar benchmark results:

 StringUtil_removeBenchmark.removeNew                a  thrpt    5  34347.155 ±  973.132  ops/ms
 StringUtil_removeBenchmark.removeNew               ba  thrpt    5  27705.444 ±  373.746  ops/ms
 StringUtil_removeBenchmark.removeNew              cab  thrpt    5  26276.031 ±  176.106  ops/ms
 StringUtil_removeBenchmark.removeNew            abcde  thrpt    5  24189.122 ± 1884.955  ops/ms
 StringUtil_removeBenchmark.removeNew  bcdefghaijklman  thrpt    5  15285.932 ± 1156.311  ops/ms
 StringUtil_removeBenchmark.removeOld                a  thrpt    5  56716.563 ±  561.894  ops/ms
 StringUtil_removeBenchmark.removeOld               ba  thrpt    5  40861.307 ±  459.259  ops/ms
 StringUtil_removeBenchmark.removeOld              cab  thrpt    5  40256.259 ± 1036.259  ops/ms
 StringUtil_removeBenchmark.removeOld            abcde  thrpt    5  38207.019 ±  375.905  ops/ms
 StringUtil_removeBenchmark.removeOld  bcdefghaijklman  thrpt    5  32188.114 ±  379.069  ops/ms

(without the added commit)

codecov[bot] commented 4 years ago

Codecov Report

Merging #4 into master will increase coverage by 0.03%. The diff coverage is 92.75%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #4      +/-   ##
============================================
+ Coverage     74.13%   74.17%   +0.03%     
- Complexity     2878     2888      +10     
============================================
  Files           122      123       +1     
  Lines          9176     9191      +15     
  Branches       1920     1919       -1     
============================================
+ Hits           6803     6817      +14     
  Misses         1878     1878              
- Partials        495      496       +1     
Impacted Files Coverage Δ Complexity Δ
src/main/java/jodd/net/HtmlEncoder.java 93.18% <71.42%> (ø) 12.00 <2.00> (+1.00)
src/main/java/jodd/util/StringBuilderPool.java 94.11% <94.11%> (ø) 6.00 <6.00> (?)
src/main/java/jodd/net/HtmlDecoder.java 87.00% <94.44%> (+0.13%) 19.00 <8.00> (+1.00)
src/main/java/jodd/util/StringUtil.java 96.13% <100.00%> (-0.02%) 519.00 <6.00> (+2.00) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 29d9511...c457a64. Read the comment docs.

slandelle commented 4 years ago

It looks like the original implementation of remove is faster when it operates on very small Strings, which is the case for CssSelector#unescape.

I'll also give it a try without the lamdba and the borrow pattern.

BTW, you should remove the redundant indexOf in CssSelector#unescape, you're doing the job twice.

Now, what would be interesting would be to benchmark HtmlEncoder that operates on large Strings. Recycling the StringBuilder should shine more there.

slandelle commented 4 years ago

So, it turns out that current remove implementation is based on an unpooled char array and that using a StringBuilder instead, which is a way more complex object, is not a win.

Then, I do see a +10% throughput improvement on HtmlDecoder#decode, and I would really like to see the effect on Lagarto where StringBuilders are massively used. And also, I have other pools implementations in mind.

slandelle commented 4 years ago

I'm going to focus my efforts on lagarto. If experiments are successful, you'll be free to decide if you want to move things up into jodd-util.

igr commented 4 years ago

To summarize, I will:

Make sense @slandelle ?

slandelle commented 4 years ago

I would say:

igr commented 4 years ago

make sense! Thank you! I will make it as a draft:)

igr commented 1 year ago

Its old :)