Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDDS-2445. Replace ToStringBuilder in BlockData #132

Closed
wants to merge 1 commit into from

Conversation

adoroszlai
Copy link
Contributor

@adoroszlai adoroszlai commented Nov 8, 2019

What changes were proposed in this pull request?

Eliminate ToStringBuilder from BlockData. Use a single StringBuilder to collect parts of the final result.

Also avoid stream-processing in setChunks for the special cases of 0 or 1 elements.

https://issues.apache.org/jira/browse/HDDS-2445

How was this patch tested?

Added benchmark with various implementations.

bin/ozone genesis -benchmark BenchmarkBlockDataToString

Normalized GC allocation rates are below (absolute values are not important, only relative to one another). Using a single string builder saves ~78% of allocations compared to the current implementation (ToStringBuilderDefaultCapacity).

Benchmark                  (capacity)  (count)   Mode  Cnt        Score       Error   Units
PushDownStringBuilder             112     1000  thrpt   20   503403.364 ±     6.593    B/op
InlineStringBuilder               112     1000  thrpt   20   503625.761 ±     2.665    B/op
SimpleStringBuilder               112     1000  thrpt   20  1133643.831 ±     4.051    B/op
ToStringBuilder                   112     1000  thrpt   20  1429626.864 ±     7.415    B/op
Concatenation                     112     1000  thrpt   20  1523808.749 ±    13.819    B/op
ToStringBuilderDefaultCapacity    N/A     1000  thrpt   20  2229699.096 ±     6.739    B/op

(Tested various capacities with the same benchmark to come up with final value of 112.)

Added a simple unit test to verify the output is unchanged.

Stream-processing change is verified by existing TestBlockData#testSetChunks.

@adoroszlai
Copy link
Contributor Author

/retest

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

thanks the patch @adoroszlai

@elek elek closed this in ef2a9f4 Nov 13, 2019
@adoroszlai adoroszlai deleted the HDDS-2445 branch November 13, 2019 14:10
@adoroszlai
Copy link
Contributor Author

Thanks @elek for reviewing and merging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants