[GitHub] gianm commented on issue #6381: datsketches extension updated to use the latest sketches-core-0.12.0

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

gianm commented on issue #6381: datsketches extension updated to use the latest sketches-core-0.12.0
URL: https://github.com/apache/incubator-druid/pull/6381#issuecomment-454428047
 
 
   We might want to remove `OffheapIncrementalIndex`. It's only used in one place: groupBy v1, if the `useOffheap` context parameter is set. groupBy v1 has been deprecated for a while, and imo the only reason it's still around is something related to https://github.com/apache/incubator-druid/issues/6743 -- buffer aggregators can't resize themselves currently, so groupBy v2, which uses offheap aggregations, allocates more space than necessary for ones that could grow in theory. groupBy v1 by default uses onheap aggregations and doesn't have this problem, so it can still be useful if your workload is mainly composed of groupBys with very growable sketch objects, and you're memory limited. (It has a bunch of _other_ problems, though. This is really the only good thing it does relative to v2.)
   
   However: groupBy v1 with the `useOffheap` parameter is, as far as I know, not useful anymore. groupBy v2 should be better in every way. So I think that does make a case for removing `OffheapIncrementalIndex`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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