You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <wa...@gmail.com> on 2022/09/19 22:32:25 UTC
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Hello ShunKang,
Thanks for filing the proposal, and sorry for the late reply!
I looked over your KIP proposal and the PR, in general I think I agree that
adding an overloaded function with `ByteBuffer` param is beneficial, but I
have a meta question regarding it's impact on Kafka consumer: my
understanding from your PR is that, we can only save memory allocations if
the key/value types happen to be ByteBuffer as well, otherwise we would
still do the `return deserialize(topic, headers, Utils.toArray(data));`
from default impls unless the user customized deserializers is augmented to
handle ByteBuffer directly, right?
Guozhang
On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <li...@gmail.com>
wrote:
> Hi all，
>
> I'd like to start a discussion on KIP-863 which is Reduce
> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
> memory allocation by nearly 50% during fetch records.
>
> Please check
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> and https://github.com/apache/kafka/pull/12545 for more details.
>
> Any feedbacks and comments are welcomed.
>
> Thanks.
>
--
-- Guozhang
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Hi Luke,
Thanks for your comments.
Best,
ShunKang
Luke Chen <sh...@gmail.com> 于2022年10月11日周二 20:36写道：
> Hi ShunKang,
>
> Had a quick look, I think It's a good idea.
> I'll check it again tomorrow, and let you know if I have any questions.
>
> Luke
>
> On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin <li...@gmail.com>
> wrote:
>
> > Hi Guozhang,
> >
> > When I try add method `default ByteBuffer serializeToByteBuffer(String
> > topic, Headers headers, T data)` for `ByteBufferSerializer`, I found
> > `ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct.
> > Then I searched JIRA and found this:
> > https://issues.apache.org/jira/browse/KAFKA-4852, I made a comment below
> > this JIRA, PTAL.
> >
> > Best,
> > ShunKang
> >
> > Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:33写道：
> >
> > > A separate question regarding the proposed API as well: what do you
> think
> > > to also augment the serializers with ByteBuffer return type in order to
> > be
> > > symmetric with deserializers?
> > >
> > >
> > >
> > > On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com>
> > wrote:
> > >
> > > > Hello ShunKang,
> > > >
> > > > Thanks for filing the proposal, and sorry for the late reply!
> > > >
> > > > I looked over your KIP proposal and the PR, in general I think I
> agree
> > > > that adding an overloaded function with `ByteBuffer` param is
> > beneficial,
> > > > but I have a meta question regarding it's impact on Kafka consumer:
> my
> > > > understanding from your PR is that, we can only save memory
> allocations
> > > if
> > > > the key/value types happen to be ByteBuffer as well, otherwise we
> would
> > > > still do the `return deserialize(topic, headers,
> Utils.toArray(data));`
> > > > from default impls unless the user customized deserializers is
> > augmented
> > > to
> > > > handle ByteBuffer directly, right?
> > > >
> > > >
> > > > Guozhang
> > > >
> > > >
> > > >
> > > > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > linshunkang.chn@gmail.com>
> > > > wrote:
> > > >
> > > >> Hi all，
> > > >>
> > > >> I'd like to start a discussion on KIP-863 which is Reduce
> > > >> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> Consumer
> > > >> memory allocation by nearly 50% during fetch records.
> > > >>
> > > >> Please check
> > > >>
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > >> and https://github.com/apache/kafka/pull/12545 for more details.
> > > >>
> > > >> Any feedbacks and comments are welcomed.
> > > >>
> > > >> Thanks.
> > > >>
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Chris Egerton <ch...@aiven.io.INVALID>.
Thanks ShunKang, that's a good point about ByteBuffer::hasArray. LGTM!
On Wed, Oct 12, 2022 at 11:12 AM ShunKang Lin <li...@gmail.com>
wrote:
> Hi Chris,
>
> 1. Record keys/values are duplicated from `DefaultRecordBatch#buffer`, so
> modifying key/value offsets will not modify the original ByteBuffer
> offsets. A read-only ByteBuffer calls `ByteBuffer#hasArray()` to return
> false, which means that a read-only ByteBuffer does not expose the
> underlying array, which is safer but slower when using the ByteBuffer API.
>
> 2. Good idea, I modified the KIP compatibility section, please take a look.
>
> Best,
> ShunKang
>
> Chris Egerton <ch...@aiven.io.invalid> 于2022年10月11日周二 23:59写道：
>
> > Hi ShunKang,
> >
> > Thanks for the KIP! I have a couple thoughts:
> >
> > 1. If we pass the ByteBuffer that we're using internally for the record
> > key/value to the deserializer, it may be mutated by writing new bytes or
> > altering the position. Should we permit this, or would it make sense to
> > provide deserializers with a read-only ByteBuffer? [1]
> >
> > 2. The compatibility section should probably be fleshed out a bit further
> > to state that deserializers that wish to be compatible with older
> versions
> > of the Kafka clients library should always implement the byte array-based
> > deserialize method. We should probably also add this information to the
> > Javadocs for the new method, although this can be taken care of during PR
> > review and doesn't have to be included in the KIP itself.
> >
> > Cheers,
> >
> > Chris
> >
> > [1] -
> >
> >
> https://docs.oracle.com/javase/8/docs/api/java/nio/ByteBuffer.html#asReadOnlyBuffer--
> >
> > On Tue, Oct 11, 2022 at 8:36 AM Luke Chen <sh...@gmail.com> wrote:
> >
> > > Hi ShunKang,
> > >
> > > Had a quick look, I think It's a good idea.
> > > I'll check it again tomorrow, and let you know if I have any questions.
> > >
> > > Luke
> > >
> > > On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin <
> linshunkang.chn@gmail.com>
> > > wrote:
> > >
> > > > Hi Guozhang,
> > > >
> > > > When I try add method `default ByteBuffer
> serializeToByteBuffer(String
> > > > topic, Headers headers, T data)` for `ByteBufferSerializer`, I found
> > > > `ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct.
> > > > Then I searched JIRA and found this:
> > > > https://issues.apache.org/jira/browse/KAFKA-4852, I made a comment
> > below
> > > > this JIRA, PTAL.
> > > >
> > > > Best,
> > > > ShunKang
> > > >
> > > > Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:33写道：
> > > >
> > > > > A separate question regarding the proposed API as well: what do you
> > > think
> > > > > to also augment the serializers with ByteBuffer return type in
> order
> > to
> > > > be
> > > > > symmetric with deserializers?
> > > > >
> > > > >
> > > > >
> > > > > On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com>
> > > > wrote:
> > > > >
> > > > > > Hello ShunKang,
> > > > > >
> > > > > > Thanks for filing the proposal, and sorry for the late reply!
> > > > > >
> > > > > > I looked over your KIP proposal and the PR, in general I think I
> > > agree
> > > > > > that adding an overloaded function with `ByteBuffer` param is
> > > > beneficial,
> > > > > > but I have a meta question regarding it's impact on Kafka
> consumer:
> > > my
> > > > > > understanding from your PR is that, we can only save memory
> > > allocations
> > > > > if
> > > > > > the key/value types happen to be ByteBuffer as well, otherwise we
> > > would
> > > > > > still do the `return deserialize(topic, headers,
> > > Utils.toArray(data));`
> > > > > > from default impls unless the user customized deserializers is
> > > > augmented
> > > > > to
> > > > > > handle ByteBuffer directly, right?
> > > > > >
> > > > > >
> > > > > > Guozhang
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > > > linshunkang.chn@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > >> Hi all，
> > > > > >>
> > > > > >> I'd like to start a discussion on KIP-863 which is Reduce
> > > > > >> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> > > Consumer
> > > > > >> memory allocation by nearly 50% during fetch records.
> > > > > >>
> > > > > >> Please check
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > > > >> and https://github.com/apache/kafka/pull/12545 for more
> details.
> > > > > >>
> > > > > >> Any feedbacks and comments are welcomed.
> > > > > >>
> > > > > >> Thanks.
> > > > > >>
> > > > > >
> > > > > >
> > > > > > --
> > > > > > -- Guozhang
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Hi Chris,
1. Record keys/values are duplicated from `DefaultRecordBatch#buffer`, so
modifying key/value offsets will not modify the original ByteBuffer
offsets. A read-only ByteBuffer calls `ByteBuffer#hasArray()` to return
false, which means that a read-only ByteBuffer does not expose the
underlying array, which is safer but slower when using the ByteBuffer API.
2. Good idea, I modified the KIP compatibility section, please take a look.
Best,
ShunKang
Chris Egerton <ch...@aiven.io.invalid> 于2022年10月11日周二 23:59写道：
> Hi ShunKang,
>
> Thanks for the KIP! I have a couple thoughts:
>
> 1. If we pass the ByteBuffer that we're using internally for the record
> key/value to the deserializer, it may be mutated by writing new bytes or
> altering the position. Should we permit this, or would it make sense to
> provide deserializers with a read-only ByteBuffer? [1]
>
> 2. The compatibility section should probably be fleshed out a bit further
> to state that deserializers that wish to be compatible with older versions
> of the Kafka clients library should always implement the byte array-based
> deserialize method. We should probably also add this information to the
> Javadocs for the new method, although this can be taken care of during PR
> review and doesn't have to be included in the KIP itself.
>
> Cheers,
>
> Chris
>
> [1] -
>
> https://docs.oracle.com/javase/8/docs/api/java/nio/ByteBuffer.html#asReadOnlyBuffer--
>
> On Tue, Oct 11, 2022 at 8:36 AM Luke Chen <sh...@gmail.com> wrote:
>
> > Hi ShunKang,
> >
> > Had a quick look, I think It's a good idea.
> > I'll check it again tomorrow, and let you know if I have any questions.
> >
> > Luke
> >
> > On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin <li...@gmail.com>
> > wrote:
> >
> > > Hi Guozhang,
> > >
> > > When I try add method `default ByteBuffer serializeToByteBuffer(String
> > > topic, Headers headers, T data)` for `ByteBufferSerializer`, I found
> > > `ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct.
> > > Then I searched JIRA and found this:
> > > https://issues.apache.org/jira/browse/KAFKA-4852, I made a comment
> below
> > > this JIRA, PTAL.
> > >
> > > Best,
> > > ShunKang
> > >
> > > Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:33写道：
> > >
> > > > A separate question regarding the proposed API as well: what do you
> > think
> > > > to also augment the serializers with ByteBuffer return type in order
> to
> > > be
> > > > symmetric with deserializers?
> > > >
> > > >
> > > >
> > > > On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com>
> > > wrote:
> > > >
> > > > > Hello ShunKang,
> > > > >
> > > > > Thanks for filing the proposal, and sorry for the late reply!
> > > > >
> > > > > I looked over your KIP proposal and the PR, in general I think I
> > agree
> > > > > that adding an overloaded function with `ByteBuffer` param is
> > > beneficial,
> > > > > but I have a meta question regarding it's impact on Kafka consumer:
> > my
> > > > > understanding from your PR is that, we can only save memory
> > allocations
> > > > if
> > > > > the key/value types happen to be ByteBuffer as well, otherwise we
> > would
> > > > > still do the `return deserialize(topic, headers,
> > Utils.toArray(data));`
> > > > > from default impls unless the user customized deserializers is
> > > augmented
> > > > to
> > > > > handle ByteBuffer directly, right?
> > > > >
> > > > >
> > > > > Guozhang
> > > > >
> > > > >
> > > > >
> > > > > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > > linshunkang.chn@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> Hi all，
> > > > >>
> > > > >> I'd like to start a discussion on KIP-863 which is Reduce
> > > > >> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> > Consumer
> > > > >> memory allocation by nearly 50% during fetch records.
> > > > >>
> > > > >> Please check
> > > > >>
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > > >> and https://github.com/apache/kafka/pull/12545 for more details.
> > > > >>
> > > > >> Any feedbacks and comments are welcomed.
> > > > >>
> > > > >> Thanks.
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Chris Egerton <ch...@aiven.io.INVALID>.
Hi ShunKang,
Thanks for the KIP! I have a couple thoughts:
1. If we pass the ByteBuffer that we're using internally for the record
key/value to the deserializer, it may be mutated by writing new bytes or
altering the position. Should we permit this, or would it make sense to
provide deserializers with a read-only ByteBuffer? [1]
2. The compatibility section should probably be fleshed out a bit further
to state that deserializers that wish to be compatible with older versions
of the Kafka clients library should always implement the byte array-based
deserialize method. We should probably also add this information to the
Javadocs for the new method, although this can be taken care of during PR
review and doesn't have to be included in the KIP itself.
Cheers,
Chris
[1] -
https://docs.oracle.com/javase/8/docs/api/java/nio/ByteBuffer.html#asReadOnlyBuffer--
On Tue, Oct 11, 2022 at 8:36 AM Luke Chen <sh...@gmail.com> wrote:
> Hi ShunKang,
>
> Had a quick look, I think It's a good idea.
> I'll check it again tomorrow, and let you know if I have any questions.
>
> Luke
>
> On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin <li...@gmail.com>
> wrote:
>
> > Hi Guozhang,
> >
> > When I try add method `default ByteBuffer serializeToByteBuffer(String
> > topic, Headers headers, T data)` for `ByteBufferSerializer`, I found
> > `ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct.
> > Then I searched JIRA and found this:
> > https://issues.apache.org/jira/browse/KAFKA-4852, I made a comment below
> > this JIRA, PTAL.
> >
> > Best,
> > ShunKang
> >
> > Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:33写道：
> >
> > > A separate question regarding the proposed API as well: what do you
> think
> > > to also augment the serializers with ByteBuffer return type in order to
> > be
> > > symmetric with deserializers?
> > >
> > >
> > >
> > > On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com>
> > wrote:
> > >
> > > > Hello ShunKang,
> > > >
> > > > Thanks for filing the proposal, and sorry for the late reply!
> > > >
> > > > I looked over your KIP proposal and the PR, in general I think I
> agree
> > > > that adding an overloaded function with `ByteBuffer` param is
> > beneficial,
> > > > but I have a meta question regarding it's impact on Kafka consumer:
> my
> > > > understanding from your PR is that, we can only save memory
> allocations
> > > if
> > > > the key/value types happen to be ByteBuffer as well, otherwise we
> would
> > > > still do the `return deserialize(topic, headers,
> Utils.toArray(data));`
> > > > from default impls unless the user customized deserializers is
> > augmented
> > > to
> > > > handle ByteBuffer directly, right?
> > > >
> > > >
> > > > Guozhang
> > > >
> > > >
> > > >
> > > > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > linshunkang.chn@gmail.com>
> > > > wrote:
> > > >
> > > >> Hi all，
> > > >>
> > > >> I'd like to start a discussion on KIP-863 which is Reduce
> > > >> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> Consumer
> > > >> memory allocation by nearly 50% during fetch records.
> > > >>
> > > >> Please check
> > > >>
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > >> and https://github.com/apache/kafka/pull/12545 for more details.
> > > >>
> > > >> Any feedbacks and comments are welcomed.
> > > >>
> > > >> Thanks.
> > > >>
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Luke Chen <sh...@gmail.com>.
Hi ShunKang,
Had a quick look, I think It's a good idea.
I'll check it again tomorrow, and let you know if I have any questions.
Luke
On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin <li...@gmail.com>
wrote:
> Hi Guozhang,
>
> When I try add method `default ByteBuffer serializeToByteBuffer(String
> topic, Headers headers, T data)` for `ByteBufferSerializer`, I found
> `ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct.
> Then I searched JIRA and found this:
> https://issues.apache.org/jira/browse/KAFKA-4852, I made a comment below
> this JIRA, PTAL.
>
> Best,
> ShunKang
>
> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:33写道：
>
> > A separate question regarding the proposed API as well: what do you think
> > to also augment the serializers with ByteBuffer return type in order to
> be
> > symmetric with deserializers?
> >
> >
> >
> > On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com>
> wrote:
> >
> > > Hello ShunKang,
> > >
> > > Thanks for filing the proposal, and sorry for the late reply!
> > >
> > > I looked over your KIP proposal and the PR, in general I think I agree
> > > that adding an overloaded function with `ByteBuffer` param is
> beneficial,
> > > but I have a meta question regarding it's impact on Kafka consumer: my
> > > understanding from your PR is that, we can only save memory allocations
> > if
> > > the key/value types happen to be ByteBuffer as well, otherwise we would
> > > still do the `return deserialize(topic, headers, Utils.toArray(data));`
> > > from default impls unless the user customized deserializers is
> augmented
> > to
> > > handle ByteBuffer directly, right?
> > >
> > >
> > > Guozhang
> > >
> > >
> > >
> > > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> linshunkang.chn@gmail.com>
> > > wrote:
> > >
> > >> Hi all，
> > >>
> > >> I'd like to start a discussion on KIP-863 which is Reduce
> > >> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
> > >> memory allocation by nearly 50% during fetch records.
> > >>
> > >> Please check
> > >>
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > >> and https://github.com/apache/kafka/pull/12545 for more details.
> > >>
> > >> Any feedbacks and comments are welcomed.
> > >>
> > >> Thanks.
> > >>
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
> >
> > --
> > -- Guozhang
> >
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Hi Guozhang,
When I try add method `default ByteBuffer serializeToByteBuffer(String
topic, Headers headers, T data)` for `ByteBufferSerializer`, I found
`ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct.
Then I searched JIRA and found this:
https://issues.apache.org/jira/browse/KAFKA-4852, I made a comment below
this JIRA, PTAL.
Best,
ShunKang
Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:33写道：
> A separate question regarding the proposed API as well: what do you think
> to also augment the serializers with ByteBuffer return type in order to be
> symmetric with deserializers?
>
>
>
> On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com> wrote:
>
> > Hello ShunKang,
> >
> > Thanks for filing the proposal, and sorry for the late reply!
> >
> > I looked over your KIP proposal and the PR, in general I think I agree
> > that adding an overloaded function with `ByteBuffer` param is beneficial,
> > but I have a meta question regarding it's impact on Kafka consumer: my
> > understanding from your PR is that, we can only save memory allocations
> if
> > the key/value types happen to be ByteBuffer as well, otherwise we would
> > still do the `return deserialize(topic, headers, Utils.toArray(data));`
> > from default impls unless the user customized deserializers is augmented
> to
> > handle ByteBuffer directly, right?
> >
> >
> > Guozhang
> >
> >
> >
> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <li...@gmail.com>
> > wrote:
> >
> >> Hi all，
> >>
> >> I'd like to start a discussion on KIP-863 which is Reduce
> >> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
> >> memory allocation by nearly 50% during fetch records.
> >>
> >> Please check
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> >> and https://github.com/apache/kafka/pull/12545 for more details.
> >>
> >> Any feedbacks and comments are welcomed.
> >>
> >> Thanks.
> >>
> >
> >
> > --
> > -- Guozhang
> >
>
>
> --
> -- Guozhang
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Guozhang Wang <wa...@gmail.com>.
A separate question regarding the proposed API as well: what do you think
to also augment the serializers with ByteBuffer return type in order to be
symmetric with deserializers?
On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang <wa...@gmail.com> wrote:
> Hello ShunKang,
>
> Thanks for filing the proposal, and sorry for the late reply!
>
> I looked over your KIP proposal and the PR, in general I think I agree
> that adding an overloaded function with `ByteBuffer` param is beneficial,
> but I have a meta question regarding it's impact on Kafka consumer: my
> understanding from your PR is that, we can only save memory allocations if
> the key/value types happen to be ByteBuffer as well, otherwise we would
> still do the `return deserialize(topic, headers, Utils.toArray(data));`
> from default impls unless the user customized deserializers is augmented to
> handle ByteBuffer directly, right?
>
>
> Guozhang
>
>
>
> On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <li...@gmail.com>
> wrote:
>
>> Hi all，
>>
>> I'd like to start a discussion on KIP-863 which is Reduce
>> Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
>> memory allocation by nearly 50% during fetch records.
>>
>> Please check
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
>> and https://github.com/apache/kafka/pull/12545 for more details.
>>
>> Any feedbacks and comments are welcomed.
>>
>> Thanks.
>>
>
>
> --
> -- Guozhang
>
--
-- Guozhang
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Thanks Guozhang!
Best,
ShunKang
Guozhang Wang <wa...@gmail.com> 于2022年9月23日周五 00:27写道：
> Could you start a separate VOTE email thread calling for votes?
>
> On Thu, Sep 22, 2022 at 9:19 AM ShunKang Lin <li...@gmail.com>
> wrote:
>
> > Hi Guozhang,
> >
> > Thanks for your help! By the way, what should I do next?
> >
> > Best,
> > ShunKang
> >
> > Guozhang Wang <wa...@gmail.com> 于2022年9月22日周四 23:21写道：
> >
> > > Thanks ShunKang,
> > >
> > > I made a few nit edits on the Motivation section as well. LGTM for me
> > now.
> > >
> > > On Thu, Sep 22, 2022 at 7:33 AM ShunKang Lin <
> linshunkang.chn@gmail.com>
> > > wrote:
> > >
> > > > Hi Guozhang,
> > > >
> > > > I've updated the "Motivation" section of the KIP, please take a look.
> > > >
> > > > Thanks.
> > > > ShunKang
> > > >
> > > > Guozhang Wang <wa...@gmail.com> 于2022年9月21日周三 01:26写道：
> > > >
> > > > > In this case, could you update the KIP to clarify the allocation
> > > savings
> > > > > more clearly in the "Motivation" section? Also you could mention
> that
> > > for
> > > > > user customizable serdes, if they could provide overwrites on the
> > > > > overloaded function that's also possible for optimize memory
> > > allocations.
> > > > >
> > > > > Guozhang
> > > > >
> > > > > On Tue, Sep 20, 2022 at 10:24 AM Guozhang Wang <wangguoz@gmail.com
> >
> > > > wrote:
> > > > >
> > > > > > 1. Ack, thanks.
> > > > > > 2. Sounds good, thanks for clarifying.
> > > > > >
> > > > > > On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <
> > > > linshunkang.chn@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > >> Hi Guozhang,
> > > > > >>
> > > > > >> Thanks for your comments!
> > > > > >>
> > > > > >> 1. We can reduce memory allocation if the key/value types happen
> > to
> > > be
> > > > > >> ByteBuffer or String.
> > > > > >> 2. I would like to add `default ByteBuffer
> > > > serializeToByteBuffer(String
> > > > > >> topic, Headers headers, T data)` in Serializer to reduce memory
> > copy
> > > > in
> > > > > >> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this
> change
> > > is a
> > > > > bit
> > > > > >> big, I prefer to submit another one KIP to do the job.
> > > > > >>
> > > > > >> Thanks.
> > > > > >> ShunKang
> > > > > >>
> > > > > >> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
> > > > > >>
> > > > > >> > Hello ShunKang,
> > > > > >> >
> > > > > >> > Thanks for filing the proposal, and sorry for the late reply!
> > > > > >> >
> > > > > >> > I looked over your KIP proposal and the PR, in general I
> think I
> > > > agree
> > > > > >> that
> > > > > >> > adding an overloaded function with `ByteBuffer` param is
> > > beneficial,
> > > > > >> but I
> > > > > >> > have a meta question regarding it's impact on Kafka consumer:
> my
> > > > > >> > understanding from your PR is that, we can only save memory
> > > > > allocations
> > > > > >> if
> > > > > >> > the key/value types happen to be ByteBuffer as well, otherwise
> > we
> > > > > would
> > > > > >> > still do the `return deserialize(topic, headers,
> > > > > Utils.toArray(data));`
> > > > > >> > from default impls unless the user customized deserializers is
> > > > > >> augmented to
> > > > > >> > handle ByteBuffer directly, right?
> > > > > >> >
> > > > > >> >
> > > > > >> > Guozhang
> > > > > >> >
> > > > > >> >
> > > > > >> >
> > > > > >> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > > > > linshunkang.chn@gmail.com
> > > > > >> >
> > > > > >> > wrote:
> > > > > >> >
> > > > > >> > > Hi all，
> > > > > >> > >
> > > > > >> > > I'd like to start a discussion on KIP-863 which is Reduce
> > > > > >> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> > > > > Consumer
> > > > > >> > > memory allocation by nearly 50% during fetch records.
> > > > > >> > >
> > > > > >> > > Please check
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > > > >> > > and https://github.com/apache/kafka/pull/12545 for more
> > > details.
> > > > > >> > >
> > > > > >> > > Any feedbacks and comments are welcomed.
> > > > > >> > >
> > > > > >> > > Thanks.
> > > > > >> > >
> > > > > >> >
> > > > > >> >
> > > > > >> > --
> > > > > >> > -- Guozhang
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > > > --
> > > > > > -- Guozhang
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>
>
> --
> -- Guozhang
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Guozhang Wang <wa...@gmail.com>.
Could you start a separate VOTE email thread calling for votes?
On Thu, Sep 22, 2022 at 9:19 AM ShunKang Lin <li...@gmail.com>
wrote:
> Hi Guozhang,
>
> Thanks for your help! By the way, what should I do next?
>
> Best,
> ShunKang
>
> Guozhang Wang <wa...@gmail.com> 于2022年9月22日周四 23:21写道：
>
> > Thanks ShunKang,
> >
> > I made a few nit edits on the Motivation section as well. LGTM for me
> now.
> >
> > On Thu, Sep 22, 2022 at 7:33 AM ShunKang Lin <li...@gmail.com>
> > wrote:
> >
> > > Hi Guozhang,
> > >
> > > I've updated the "Motivation" section of the KIP, please take a look.
> > >
> > > Thanks.
> > > ShunKang
> > >
> > > Guozhang Wang <wa...@gmail.com> 于2022年9月21日周三 01:26写道：
> > >
> > > > In this case, could you update the KIP to clarify the allocation
> > savings
> > > > more clearly in the "Motivation" section? Also you could mention that
> > for
> > > > user customizable serdes, if they could provide overwrites on the
> > > > overloaded function that's also possible for optimize memory
> > allocations.
> > > >
> > > > Guozhang
> > > >
> > > > On Tue, Sep 20, 2022 at 10:24 AM Guozhang Wang <wa...@gmail.com>
> > > wrote:
> > > >
> > > > > 1. Ack, thanks.
> > > > > 2. Sounds good, thanks for clarifying.
> > > > >
> > > > > On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <
> > > linshunkang.chn@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> Hi Guozhang,
> > > > >>
> > > > >> Thanks for your comments!
> > > > >>
> > > > >> 1. We can reduce memory allocation if the key/value types happen
> to
> > be
> > > > >> ByteBuffer or String.
> > > > >> 2. I would like to add `default ByteBuffer
> > > serializeToByteBuffer(String
> > > > >> topic, Headers headers, T data)` in Serializer to reduce memory
> copy
> > > in
> > > > >> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this change
> > is a
> > > > bit
> > > > >> big, I prefer to submit another one KIP to do the job.
> > > > >>
> > > > >> Thanks.
> > > > >> ShunKang
> > > > >>
> > > > >> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
> > > > >>
> > > > >> > Hello ShunKang,
> > > > >> >
> > > > >> > Thanks for filing the proposal, and sorry for the late reply!
> > > > >> >
> > > > >> > I looked over your KIP proposal and the PR, in general I think I
> > > agree
> > > > >> that
> > > > >> > adding an overloaded function with `ByteBuffer` param is
> > beneficial,
> > > > >> but I
> > > > >> > have a meta question regarding it's impact on Kafka consumer: my
> > > > >> > understanding from your PR is that, we can only save memory
> > > > allocations
> > > > >> if
> > > > >> > the key/value types happen to be ByteBuffer as well, otherwise
> we
> > > > would
> > > > >> > still do the `return deserialize(topic, headers,
> > > > Utils.toArray(data));`
> > > > >> > from default impls unless the user customized deserializers is
> > > > >> augmented to
> > > > >> > handle ByteBuffer directly, right?
> > > > >> >
> > > > >> >
> > > > >> > Guozhang
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > > > linshunkang.chn@gmail.com
> > > > >> >
> > > > >> > wrote:
> > > > >> >
> > > > >> > > Hi all，
> > > > >> > >
> > > > >> > > I'd like to start a discussion on KIP-863 which is Reduce
> > > > >> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> > > > Consumer
> > > > >> > > memory allocation by nearly 50% during fetch records.
> > > > >> > >
> > > > >> > > Please check
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > > >> > > and https://github.com/apache/kafka/pull/12545 for more
> > details.
> > > > >> > >
> > > > >> > > Any feedbacks and comments are welcomed.
> > > > >> > >
> > > > >> > > Thanks.
> > > > >> > >
> > > > >> >
> > > > >> >
> > > > >> > --
> > > > >> > -- Guozhang
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> >
> >
> > --
> > -- Guozhang
> >
>
--
-- Guozhang
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Hi Guozhang,
Thanks for your help! By the way, what should I do next?
Best,
ShunKang
Guozhang Wang <wa...@gmail.com> 于2022年9月22日周四 23:21写道：
> Thanks ShunKang,
>
> I made a few nit edits on the Motivation section as well. LGTM for me now.
>
> On Thu, Sep 22, 2022 at 7:33 AM ShunKang Lin <li...@gmail.com>
> wrote:
>
> > Hi Guozhang,
> >
> > I've updated the "Motivation" section of the KIP, please take a look.
> >
> > Thanks.
> > ShunKang
> >
> > Guozhang Wang <wa...@gmail.com> 于2022年9月21日周三 01:26写道：
> >
> > > In this case, could you update the KIP to clarify the allocation
> savings
> > > more clearly in the "Motivation" section? Also you could mention that
> for
> > > user customizable serdes, if they could provide overwrites on the
> > > overloaded function that's also possible for optimize memory
> allocations.
> > >
> > > Guozhang
> > >
> > > On Tue, Sep 20, 2022 at 10:24 AM Guozhang Wang <wa...@gmail.com>
> > wrote:
> > >
> > > > 1. Ack, thanks.
> > > > 2. Sounds good, thanks for clarifying.
> > > >
> > > > On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <
> > linshunkang.chn@gmail.com>
> > > > wrote:
> > > >
> > > >> Hi Guozhang,
> > > >>
> > > >> Thanks for your comments!
> > > >>
> > > >> 1. We can reduce memory allocation if the key/value types happen to
> be
> > > >> ByteBuffer or String.
> > > >> 2. I would like to add `default ByteBuffer
> > serializeToByteBuffer(String
> > > >> topic, Headers headers, T data)` in Serializer to reduce memory copy
> > in
> > > >> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this change
> is a
> > > bit
> > > >> big, I prefer to submit another one KIP to do the job.
> > > >>
> > > >> Thanks.
> > > >> ShunKang
> > > >>
> > > >> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
> > > >>
> > > >> > Hello ShunKang,
> > > >> >
> > > >> > Thanks for filing the proposal, and sorry for the late reply!
> > > >> >
> > > >> > I looked over your KIP proposal and the PR, in general I think I
> > agree
> > > >> that
> > > >> > adding an overloaded function with `ByteBuffer` param is
> beneficial,
> > > >> but I
> > > >> > have a meta question regarding it's impact on Kafka consumer: my
> > > >> > understanding from your PR is that, we can only save memory
> > > allocations
> > > >> if
> > > >> > the key/value types happen to be ByteBuffer as well, otherwise we
> > > would
> > > >> > still do the `return deserialize(topic, headers,
> > > Utils.toArray(data));`
> > > >> > from default impls unless the user customized deserializers is
> > > >> augmented to
> > > >> > handle ByteBuffer directly, right?
> > > >> >
> > > >> >
> > > >> > Guozhang
> > > >> >
> > > >> >
> > > >> >
> > > >> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > > linshunkang.chn@gmail.com
> > > >> >
> > > >> > wrote:
> > > >> >
> > > >> > > Hi all，
> > > >> > >
> > > >> > > I'd like to start a discussion on KIP-863 which is Reduce
> > > >> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> > > Consumer
> > > >> > > memory allocation by nearly 50% during fetch records.
> > > >> > >
> > > >> > > Please check
> > > >> > >
> > > >> >
> > > >>
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > >> > > and https://github.com/apache/kafka/pull/12545 for more
> details.
> > > >> > >
> > > >> > > Any feedbacks and comments are welcomed.
> > > >> > >
> > > >> > > Thanks.
> > > >> > >
> > > >> >
> > > >> >
> > > >> > --
> > > >> > -- Guozhang
> > > >> >
> > > >>
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>
>
> --
> -- Guozhang
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Guozhang Wang <wa...@gmail.com>.
Thanks ShunKang,
I made a few nit edits on the Motivation section as well. LGTM for me now.
On Thu, Sep 22, 2022 at 7:33 AM ShunKang Lin <li...@gmail.com>
wrote:
> Hi Guozhang,
>
> I've updated the "Motivation" section of the KIP, please take a look.
>
> Thanks.
> ShunKang
>
> Guozhang Wang <wa...@gmail.com> 于2022年9月21日周三 01:26写道：
>
> > In this case, could you update the KIP to clarify the allocation savings
> > more clearly in the "Motivation" section? Also you could mention that for
> > user customizable serdes, if they could provide overwrites on the
> > overloaded function that's also possible for optimize memory allocations.
> >
> > Guozhang
> >
> > On Tue, Sep 20, 2022 at 10:24 AM Guozhang Wang <wa...@gmail.com>
> wrote:
> >
> > > 1. Ack, thanks.
> > > 2. Sounds good, thanks for clarifying.
> > >
> > > On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <
> linshunkang.chn@gmail.com>
> > > wrote:
> > >
> > >> Hi Guozhang,
> > >>
> > >> Thanks for your comments!
> > >>
> > >> 1. We can reduce memory allocation if the key/value types happen to be
> > >> ByteBuffer or String.
> > >> 2. I would like to add `default ByteBuffer
> serializeToByteBuffer(String
> > >> topic, Headers headers, T data)` in Serializer to reduce memory copy
> in
> > >> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this change is a
> > bit
> > >> big, I prefer to submit another one KIP to do the job.
> > >>
> > >> Thanks.
> > >> ShunKang
> > >>
> > >> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
> > >>
> > >> > Hello ShunKang,
> > >> >
> > >> > Thanks for filing the proposal, and sorry for the late reply!
> > >> >
> > >> > I looked over your KIP proposal and the PR, in general I think I
> agree
> > >> that
> > >> > adding an overloaded function with `ByteBuffer` param is beneficial,
> > >> but I
> > >> > have a meta question regarding it's impact on Kafka consumer: my
> > >> > understanding from your PR is that, we can only save memory
> > allocations
> > >> if
> > >> > the key/value types happen to be ByteBuffer as well, otherwise we
> > would
> > >> > still do the `return deserialize(topic, headers,
> > Utils.toArray(data));`
> > >> > from default impls unless the user customized deserializers is
> > >> augmented to
> > >> > handle ByteBuffer directly, right?
> > >> >
> > >> >
> > >> > Guozhang
> > >> >
> > >> >
> > >> >
> > >> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> > linshunkang.chn@gmail.com
> > >> >
> > >> > wrote:
> > >> >
> > >> > > Hi all，
> > >> > >
> > >> > > I'd like to start a discussion on KIP-863 which is Reduce
> > >> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> > Consumer
> > >> > > memory allocation by nearly 50% during fetch records.
> > >> > >
> > >> > > Please check
> > >> > >
> > >> >
> > >>
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > >> > > and https://github.com/apache/kafka/pull/12545 for more details.
> > >> > >
> > >> > > Any feedbacks and comments are welcomed.
> > >> > >
> > >> > > Thanks.
> > >> > >
> > >> >
> > >> >
> > >> > --
> > >> > -- Guozhang
> > >> >
> > >>
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
> >
> > --
> > -- Guozhang
> >
>
--
-- Guozhang
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Hi Guozhang,
I've updated the "Motivation" section of the KIP, please take a look.
Thanks.
ShunKang
Guozhang Wang <wa...@gmail.com> 于2022年9月21日周三 01:26写道：
> In this case, could you update the KIP to clarify the allocation savings
> more clearly in the "Motivation" section? Also you could mention that for
> user customizable serdes, if they could provide overwrites on the
> overloaded function that's also possible for optimize memory allocations.
>
> Guozhang
>
> On Tue, Sep 20, 2022 at 10:24 AM Guozhang Wang <wa...@gmail.com> wrote:
>
> > 1. Ack, thanks.
> > 2. Sounds good, thanks for clarifying.
> >
> > On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <li...@gmail.com>
> > wrote:
> >
> >> Hi Guozhang,
> >>
> >> Thanks for your comments!
> >>
> >> 1. We can reduce memory allocation if the key/value types happen to be
> >> ByteBuffer or String.
> >> 2. I would like to add `default ByteBuffer serializeToByteBuffer(String
> >> topic, Headers headers, T data)` in Serializer to reduce memory copy in
> >> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this change is a
> bit
> >> big, I prefer to submit another one KIP to do the job.
> >>
> >> Thanks.
> >> ShunKang
> >>
> >> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
> >>
> >> > Hello ShunKang,
> >> >
> >> > Thanks for filing the proposal, and sorry for the late reply!
> >> >
> >> > I looked over your KIP proposal and the PR, in general I think I agree
> >> that
> >> > adding an overloaded function with `ByteBuffer` param is beneficial,
> >> but I
> >> > have a meta question regarding it's impact on Kafka consumer: my
> >> > understanding from your PR is that, we can only save memory
> allocations
> >> if
> >> > the key/value types happen to be ByteBuffer as well, otherwise we
> would
> >> > still do the `return deserialize(topic, headers,
> Utils.toArray(data));`
> >> > from default impls unless the user customized deserializers is
> >> augmented to
> >> > handle ByteBuffer directly, right?
> >> >
> >> >
> >> > Guozhang
> >> >
> >> >
> >> >
> >> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <
> linshunkang.chn@gmail.com
> >> >
> >> > wrote:
> >> >
> >> > > Hi all，
> >> > >
> >> > > I'd like to start a discussion on KIP-863 which is Reduce
> >> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka
> Consumer
> >> > > memory allocation by nearly 50% during fetch records.
> >> > >
> >> > > Please check
> >> > >
> >> >
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> >> > > and https://github.com/apache/kafka/pull/12545 for more details.
> >> > >
> >> > > Any feedbacks and comments are welcomed.
> >> > >
> >> > > Thanks.
> >> > >
> >> >
> >> >
> >> > --
> >> > -- Guozhang
> >> >
> >>
> >
> >
> > --
> > -- Guozhang
> >
>
>
> --
> -- Guozhang
>
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Guozhang Wang <wa...@gmail.com>.
In this case, could you update the KIP to clarify the allocation savings
more clearly in the "Motivation" section? Also you could mention that for
user customizable serdes, if they could provide overwrites on the
overloaded function that's also possible for optimize memory allocations.
Guozhang
On Tue, Sep 20, 2022 at 10:24 AM Guozhang Wang <wa...@gmail.com> wrote:
> 1. Ack, thanks.
> 2. Sounds good, thanks for clarifying.
>
> On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <li...@gmail.com>
> wrote:
>
>> Hi Guozhang,
>>
>> Thanks for your comments!
>>
>> 1. We can reduce memory allocation if the key/value types happen to be
>> ByteBuffer or String.
>> 2. I would like to add `default ByteBuffer serializeToByteBuffer(String
>> topic, Headers headers, T data)` in Serializer to reduce memory copy in
>> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this change is a bit
>> big, I prefer to submit another one KIP to do the job.
>>
>> Thanks.
>> ShunKang
>>
>> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
>>
>> > Hello ShunKang,
>> >
>> > Thanks for filing the proposal, and sorry for the late reply!
>> >
>> > I looked over your KIP proposal and the PR, in general I think I agree
>> that
>> > adding an overloaded function with `ByteBuffer` param is beneficial,
>> but I
>> > have a meta question regarding it's impact on Kafka consumer: my
>> > understanding from your PR is that, we can only save memory allocations
>> if
>> > the key/value types happen to be ByteBuffer as well, otherwise we would
>> > still do the `return deserialize(topic, headers, Utils.toArray(data));`
>> > from default impls unless the user customized deserializers is
>> augmented to
>> > handle ByteBuffer directly, right?
>> >
>> >
>> > Guozhang
>> >
>> >
>> >
>> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <linshunkang.chn@gmail.com
>> >
>> > wrote:
>> >
>> > > Hi all，
>> > >
>> > > I'd like to start a discussion on KIP-863 which is Reduce
>> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
>> > > memory allocation by nearly 50% during fetch records.
>> > >
>> > > Please check
>> > >
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
>> > > and https://github.com/apache/kafka/pull/12545 for more details.
>> > >
>> > > Any feedbacks and comments are welcomed.
>> > >
>> > > Thanks.
>> > >
>> >
>> >
>> > --
>> > -- Guozhang
>> >
>>
>
>
> --
> -- Guozhang
>
--
-- Guozhang
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by Guozhang Wang <wa...@gmail.com>.
1. Ack, thanks.
2. Sounds good, thanks for clarifying.
On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin <li...@gmail.com>
wrote:
> Hi Guozhang,
>
> Thanks for your comments!
>
> 1. We can reduce memory allocation if the key/value types happen to be
> ByteBuffer or String.
> 2. I would like to add `default ByteBuffer serializeToByteBuffer(String
> topic, Headers headers, T data)` in Serializer to reduce memory copy in
> `KafkaProducer#doSend(ProducerRecord, Callback)`, but this change is a bit
> big, I prefer to submit another one KIP to do the job.
>
> Thanks.
> ShunKang
>
> Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
>
> > Hello ShunKang,
> >
> > Thanks for filing the proposal, and sorry for the late reply!
> >
> > I looked over your KIP proposal and the PR, in general I think I agree
> that
> > adding an overloaded function with `ByteBuffer` param is beneficial, but
> I
> > have a meta question regarding it's impact on Kafka consumer: my
> > understanding from your PR is that, we can only save memory allocations
> if
> > the key/value types happen to be ByteBuffer as well, otherwise we would
> > still do the `return deserialize(topic, headers, Utils.toArray(data));`
> > from default impls unless the user customized deserializers is augmented
> to
> > handle ByteBuffer directly, right?
> >
> >
> > Guozhang
> >
> >
> >
> > On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <li...@gmail.com>
> > wrote:
> >
> > > Hi all，
> > >
> > > I'd like to start a discussion on KIP-863 which is Reduce
> > > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
> > > memory allocation by nearly 50% during fetch records.
> > >
> > > Please check
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > > and https://github.com/apache/kafka/pull/12545 for more details.
> > >
> > > Any feedbacks and comments are welcomed.
> > >
> > > Thanks.
> > >
> >
> >
> > --
> > -- Guozhang
> >
>
--
-- Guozhang
Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy
Posted by ShunKang Lin <li...@gmail.com>.
Hi Guozhang,
Thanks for your comments!
1. We can reduce memory allocation if the key/value types happen to be
ByteBuffer or String.
2. I would like to add `default ByteBuffer serializeToByteBuffer(String
topic, Headers headers, T data)` in Serializer to reduce memory copy in
`KafkaProducer#doSend(ProducerRecord, Callback)`, but this change is a bit
big, I prefer to submit another one KIP to do the job.
Thanks.
ShunKang
Guozhang Wang <wa...@gmail.com> 于2022年9月20日周二 06:32写道：
> Hello ShunKang,
>
> Thanks for filing the proposal, and sorry for the late reply!
>
> I looked over your KIP proposal and the PR, in general I think I agree that
> adding an overloaded function with `ByteBuffer` param is beneficial, but I
> have a meta question regarding it's impact on Kafka consumer: my
> understanding from your PR is that, we can only save memory allocations if
> the key/value types happen to be ByteBuffer as well, otherwise we would
> still do the `return deserialize(topic, headers, Utils.toArray(data));`
> from default impls unless the user customized deserializers is augmented to
> handle ByteBuffer directly, right?
>
>
> Guozhang
>
>
>
> On Sun, Aug 21, 2022 at 9:56 AM ShunKang Lin <li...@gmail.com>
> wrote:
>
> > Hi all，
> >
> > I'd like to start a discussion on KIP-863 which is Reduce
> > Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer
> > memory allocation by nearly 50% during fetch records.
> >
> > Please check
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035
> > and https://github.com/apache/kafka/pull/12545 for more details.
> >
> > Any feedbacks and comments are welcomed.
> >
> > Thanks.
> >
>
>
> --
> -- Guozhang
>