Jackie-Jiang commented on code in PR #19307:
URL: https://github.com/apache/pinot/pull/19307#discussion_r3890217146
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/codec/ChunkCodecHandler.java:
##########
@@ -49,6 +53,32 @@ interface ChunkCodecHandler<O extends CodecOptions> extends
CodecDefinition<O> {
/// @return encoded buffer ready for read; caller owns this buffer
ByteBuffer encode(O options, CodecContext ctx, ByteBuffer src) throws
IOException;
+ /// Encodes `src` directly into `dst`, avoiding an extra allocation.
+ /// Implementations must treat `dst` as freshly cleared and flip it before
returning.
+ ///
+ /// The default implementation preserves the package-private test extension
point by adapting
+ /// an allocation-returning [#encode()] implementation. Built-in handlers
override it and write
+ /// directly into `dst`.
+ ///
+ /// @param options parsed options for this codec invocation
+ /// @param ctx column context
+ /// @param src unencoded data, ready for read
+ /// @param dst output buffer; must have sufficient capacity
+ default void encodeInto(O options, CodecContext ctx, ByteBuffer src,
ByteBuffer dst) throws IOException {
Review Comment:
Suggest just naming it `encode`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/codec/ChunkCodecHandler.java:
##########
@@ -49,6 +53,32 @@ interface ChunkCodecHandler<O extends CodecOptions> extends
CodecDefinition<O> {
/// @return encoded buffer ready for read; caller owns this buffer
ByteBuffer encode(O options, CodecContext ctx, ByteBuffer src) throws
IOException;
+ /// Encodes `src` directly into `dst`, avoiding an extra allocation.
+ /// Implementations must treat `dst` as freshly cleared and flip it before
returning.
+ ///
+ /// The default implementation preserves the package-private test extension
point by adapting
+ /// an allocation-returning [#encode()] implementation. Built-in handlers
override it and write
+ /// directly into `dst`.
+ ///
+ /// @param options parsed options for this codec invocation
+ /// @param ctx column context
+ /// @param src unencoded data, ready for read
+ /// @param dst output buffer; must have sufficient capacity
+ default void encodeInto(O options, CodecContext ctx, ByteBuffer src,
ByteBuffer dst) throws IOException {
Review Comment:
Do we need this default impl? I don't think we need backward compatibility
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
