HDDS-16277. Reuse CharsetEncoder/Decoder in StringCodec - #11112
Open
rich7420 wants to merge 1 commit into
Open
Conversation
StringCodecBase allocated a fresh CharsetEncoder/CharsetDecoder on every encode/decode call. Cache them per thread (ThreadLocal) since the codec instances are singletons and the coders are stateful and not thread-safe. The encoder is reset() before each reuse; the single-arg CharsetDecoder.decode resets internally, so behavior is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
StringCodecBase(the base ofStringCodecandFixedLengthStringCodec) allocates a freshCharsetEncoderon every encode and a freshCharsetDecoderon every decode:encode(...)->newEncoder().encode(...)decodeNoFallback(...)/decodeWithFallback(...)->newDecoder().decode(...)These codecs are singletons on a hot path: every
StringRocksDB key/value (OM/SCM tables, iterators, compaction) is serialized/deserialized through them, so a short-lived coder is created on each call.CharsetEncoder/CharsetDecoderare stateful and not thread-safe, so this caches one per thread (ThreadLocal) and reuses it:reset()before each use, because the 3-argencode(in, out, endOfInput)does not reset on its own;CharsetDecoder.decode(ByteBuffer), which resets internally (per its javadoc), so no explicit reset is added there.The cache is per codec instance rather than
static, because subclasses use different charsets.reset()only clears coding state and keepsonMalformedInput/onUnmappableCharacter(REPORT), so behavior is unchanged.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16277
How was this patch tested?
fork CI run：
https://github.com/rich7420/ozone/actions/runs/32851264063
Microbenchmark
JMH (out of tree), JDK 21, single thread,
fork=2. Both variants run the identical encode/decode and differ only in coder acquisition (fresh vs. reused per thread), so the delta is attributable to this change.gc.alloc.rate.normerror is +/- 0.001 B/op, i.e. the per-call allocation is exact. that's on my macbook.encode:
/vol1/bucket1/dir1/dir2/object-file-000000123.dat8f14e45f-ceea-467a-9d1b-2f3c4d5e6f70decode:
/vol1/bucket1/dir1/dir2/object-file-000000123.dat8f14e45f-ceea-467a-9d1b-2f3c4d5e6f70