dlg99 commented on code in PR #22799:
URL: https://github.com/apache/pulsar/pull/22799#discussion_r1779286249
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MetaStoreImpl.java:
##########
@@ -453,8 +456,13 @@ public ManagedLedgerInfo parseManagedLedgerInfo(byte[]
data) throws InvalidProto
try {
MLDataFormats.ManagedLedgerInfoMetadata metadata =
MLDataFormats.ManagedLedgerInfoMetadata.parseFrom(metadataBytes);
- return
ManagedLedgerInfo.parseFrom(getCompressionCodec(metadata.getCompressionType())
- .decode(byteBuf,
metadata.getUncompressedSize()).nioBuffer());
+ ByteBuf decode =
getCompressionCodec(metadata.getCompressionType())
+ .decode(byteBuf, metadata.getUncompressedSize());
+ try {
+ return ManagedLedgerInfo.parseFrom(decode.nioBuffer());
+ } finally {
+ decode.release();
Review Comment:
Maybe I am missing something, but as I understand:
* parseFrom() needs a nio buffer.
* .nioBuffer() will return nio buffer with shared data if ByteBuff is backed
by heap array/nio buffer, otherwise it will duplicate the buffer into a nio
buffer.
* ManagedLedgerInfo does not have .release() method or equivalent
* ManagedLedgerInfo does not have .copyFrom() method
decode not being released was a bug that can go unnoticed in some cases, or
result in e.g. slow direct memory leak in others.
I guess you were thinking about google's protobuf with perf
optimized/manually constructed CodedInputStream/ByteString.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MetaStoreImpl.java:
##########
@@ -475,8 +483,13 @@ public ManagedCursorInfo parseManagedCursorInfo(byte[]
data) throws InvalidProto
try {
MLDataFormats.ManagedCursorInfoMetadata metadata =
MLDataFormats.ManagedCursorInfoMetadata.parseFrom(metadataBytes);
- return
ManagedCursorInfo.parseFrom(getCompressionCodec(metadata.getCompressionType())
- .decode(byteBuf,
metadata.getUncompressedSize()).nioBuffer());
+ ByteBuf decode =
getCompressionCodec(metadata.getCompressionType())
+ .decode(byteBuf, metadata.getUncompressedSize());
+ try {
+ return ManagedCursorInfo.parseFrom(decode.nioBuffer());
+ } finally {
+ decode.release();
Review Comment:
see above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
