[GitHub] [incubator-druid] himanshug commented on issue #8126: [Proposal] BufferAggregator support for growable sketches.

[GitBox <gi...@apache.org>]

himanshug commented on issue #8126: [Proposal] BufferAggregator support for growable sketches.
URL: https://github.com/apache/incubator-druid/issues/8126#issuecomment-516993449
 
 
   @Eshcar 
    regarding memory management: I would like it to be an interface inside druid core and later be a pluggable entity that can be changed inside extensions e.g. say a oak based extension or users who know their data and can write a more appropriate one, they should be able to do so. that also allows ppl to experiment with different memory management strategies/impls.
   
   regarding growth strategy: current strategy I described was to double the size every time, but I also thought about  having following(notice that return type is "int" here not boolean)
   
   ```
     // Returns 0, if success
     // Returns positive value, indicating what should be the size of bigger allocated BufferSize
     // Returns negative value, indicating bigger buffer is needed and do something default
     default int aggregate(ByteBuffer buff, int position, int capacity)
     {
       aggregate(buff, position);
       return true;
     }
   ```
   that could let aggregator impl control the growth strategy as it has best information about what it needs.
   
   later if needed, "default" grown strategy could be made pluggable so that extension writers could control that. That said, It doesn't matter for OakIncrementalIndex as that would choose its own default growth strategy.
   
   regarding using Memory instead of ByteBuffer : I am sure that is good idea for performance, so instead of adding more methods to `BufferAggregator` , we can introduce `WritableMemoryAggregator`. Only wrinkle is around maintaining backward compatibility, I haven't thought it through but will definitely try it. That said, this is somewhat orthogonal and could be done separately but I agree it is good to do that from the get go if possible.
   
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org