Introduce StringBuilderPool

oblac / jodd

Jodd! Lightweight. Java. Zero dependencies. Use what you like.

https://jodd.org

BSD 2-Clause "Simplified" License

4.06k stars 723 forks source link

Introduce StringBuilderPool #773

Closed slandelle closed 3 years ago

slandelle commented 4 years ago

Motivation:

A lot of StringBuilders are allocated on the hotpath, in particular in utils used Lagarto.

Modifications:

StringBuilderPool uses a ConcurrentLinkedQueue pool instead of a ThreadLocal so that containers don't complain about leaks on classloader reboot. Also, this strategy supports re-entrant calls.
Offered StringBuilders length is capped to avoid large resident memory usage.
Usage is a borrow pattern where user has to pass a Function<StringBuilder, T>.

Result:

Util to pool StringBuilder instances

slandelle commented 4 years ago

@igr I don't have time to run benchmarks atm, sorry. Feel free to run some on your side or wait for me before merging.

igr commented 4 years ago

I will wait with this a bit, some tests are failing. Will be moved to new jodd-util repo.

slandelle commented 4 years ago

@igr Could you please point to the failing tests?

igr commented 4 years ago

See here: https://github.com/oblac/jodd-util/pull/2

I just copy/paste the code, so maybe I am missing something

slandelle commented 4 years ago

@igr I found my stupid mistake. I commented on your PR.

igr commented 4 years ago

It works! I am adding a simple JMH test and will release

slandelle commented 4 years ago

Mmm, I see that StringUtils#remove can be implemented in a more efficient fashion. Please let me send a PR before benchmarking and releasing.

Also, please note I only used StringBuilderPool on the methods that are used in Lagarto. If experiment is successful, you'll probably want to update lots of code in Jodd (eg StringUtils is full for allocated StringBuilders).

igr commented 4 years ago

Yes yes, just want to do it a step by step. The first release is for the Lagarto, and then I will slowly apply furthermore.