On Thu, Jan 9, 2020 at 2:33 PM Xavi Hernandez <jaher...@redhat.com> wrote:
> On Thu, Jan 9, 2020 at 9:44 AM Amar Tumballi <ama...@gmail.com> wrote: > >> >> >> On Thu, Jan 9, 2020 at 1:38 PM Xavi Hernandez <jaher...@redhat.com> >> wrote: >> >>> On Sun, Dec 22, 2019 at 4:56 PM Yaniv Kaul <yk...@redhat.com> wrote: >>> >>>> I could not find a relevant use for them. Can anyone enlighten me? >>>> >>> >>> I'm not sure why they are needed. They seem to be used to keep the >>> unserialized version of a dict around until the dict is destroyed. I >>> thought this could be because we were using pointers to the unserialized >>> data inside dict, but that's not the case currently. However, checking very >>> old versions (pre 3.2), I see that dict values were not allocated, but a >>> pointer to the unserialized data was used. >>> >> >> Xavi, >> >> While you are right about the intent, it is used still, at least when I >> grepped latest repo to keep a reference in protocol layer. >> >> This is done to reduce a copy after the dictionary's binary content is >> received from RPC. The 'extra_free' flag is used when we use a >> GF_*ALLOC()'d buffer in protocol to receive dictionary, and extra_stdfree >> is used when RPC itself allocates the buffer and hence uses 'free()' to >> free the buffer. >> > > I don't see it. When dict_unserialize() is called, key and value are > allocated and copied, so why do we need to keep the raw data after that ? > > In 3.1 the value was simply a pointer to the unserialized data, but > starting with 3.2, value is memdup'ed. Key is always copied. I don't see > any other reference to the unserialized data right now. I think that > instead of assigning the raw data to extra_(std)free, we should simply > release that memory and remove those fields. > > Am I missing something else ? > I did grep on 'extra_stdfree' and 'extra_free' and saw that many handshake/ and protocol code seemed to use it. Haven't gone deeper to check which part. [amar@kadalu glusterfs]$ git grep extra_stdfree | wc -l 40 [amar@kadalu glusterfs]$ git grep extra_free | wc -l 5 > > >> >>> I think this is not needed anymore. Probably we could remove these >>> fields if that's the only reason. >>> >> >> If keeping them is hard to maintain, we can add few allocation to remove >> those elements, that shouldn't matter much IMO too. We are not using >> dictionary itself as protocol now (which we did in 1.x series though). >> >> Regards, >> Amar >> --- >> https://kadalu.io >> >> >> >>> TIA, >>>> Y. >>>> _______________________________________________ >>>> >>>> Community Meeting Calendar: >>>> >>>> APAC Schedule - >>>> Every 2nd and 4th Tuesday at 11:30 AM IST >>>> Bridge: https://bluejeans.com/441850968 >>>> >>>> >>>> NA/EMEA Schedule - >>>> Every 1st and 3rd Tuesday at 01:00 PM EDT >>>> Bridge: https://bluejeans.com/441850968 >>>> >>>> Gluster-devel mailing list >>>> Gluster-devel@gluster.org >>>> https://lists.gluster.org/mailman/listinfo/gluster-devel >>>> >>>> _______________________________________________ >>> >>> Community Meeting Calendar: >>> >>> APAC Schedule - >>> Every 2nd and 4th Tuesday at 11:30 AM IST >>> Bridge: https://bluejeans.com/441850968 >>> >>> >>> NA/EMEA Schedule - >>> Every 1st and 3rd Tuesday at 01:00 PM EDT >>> Bridge: https://bluejeans.com/441850968 >>> >>> Gluster-devel mailing list >>> Gluster-devel@gluster.org >>> https://lists.gluster.org/mailman/listinfo/gluster-devel >>> >>>
_______________________________________________ Community Meeting Calendar: APAC Schedule - Every 2nd and 4th Tuesday at 11:30 AM IST Bridge: https://bluejeans.com/441850968 NA/EMEA Schedule - Every 1st and 3rd Tuesday at 01:00 PM EDT Bridge: https://bluejeans.com/441850968 Gluster-devel mailing list Gluster-devel@gluster.org https://lists.gluster.org/mailman/listinfo/gluster-devel