search

oblac / jodd-util

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

StringBuilderPool #2

Closed igr closed 3 years ago

codecov[bot] commented 3 years ago

Codecov Report

Merging #2 into master will decrease coverage by 0.08%. The diff coverage is 93.05%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #2      +/-   ##
============================================
- Coverage     74.11%   74.03%   -0.09%     
- Complexity     2878     2886       +8     
============================================
  Files           122      123       +1     
  Lines          9179     9212      +33     
  Branches       1920     1923       +3     
============================================
+ Hits           6803     6820      +17     
- Misses         1881     1896      +15     
- 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.14% <100.00%> (ø) 517.00 <4.00> (ø)
src/main/java/jodd/io/StreamGobbler.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)

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 159a2a9...c03d01f. Read the comment docs.

igr commented 3 years ago

@slandelle please see my benchmark

StringUtil_removeBenchmark.removeNew                a  thrpt    5  29089.686 ± 222.621  ops/ms
StringUtil_removeBenchmark.removeNew               ba  thrpt    5  24278.007 ± 145.268  ops/ms
StringUtil_removeBenchmark.removeNew              cab  thrpt    5  22196.510 ± 212.803  ops/ms
StringUtil_removeBenchmark.removeNew            abcde  thrpt    5  23367.545 ± 430.193  ops/ms
StringUtil_removeBenchmark.removeNew  bcdefghaijklman  thrpt    5  17268.509 ± 163.351  ops/ms
StringUtil_removeBenchmark.removeOld                a  thrpt    5  57116.090 ± 881.963  ops/ms
StringUtil_removeBenchmark.removeOld               ba  thrpt    5  40605.504 ± 463.230  ops/ms
StringUtil_removeBenchmark.removeOld              cab  thrpt    5  40103.663 ± 609.040  ops/ms
StringUtil_removeBenchmark.removeOld            abcde  thrpt    5  35670.546 ± 535.558  ops/ms
StringUtil_removeBenchmark.removeOld  bcdefghaijklman  thrpt    5  29990.477 ± 348.026  ops/ms

Am i doing something wrong?

slandelle commented 3 years ago

@igr Sadly, it looks like the new strategy is only a win for large Strings. I think you can revert latest commit.

slandelle commented 3 years ago

@igr Maybe I wasn't clear. The StringBuilderPool is definitely a good thing. Only this commit 99ad746ecd603e87587d0e1653a9aaf07952c07d wasn't.

slandelle commented 3 years ago

I don't own the PR, otherwise I would have remove the wrong commit.

igr commented 3 years ago

Closing this one and opening: https://github.com/oblac/jodd-util/pull/4