siddharthteotia commented on pull request #6559: URL: https://github.com/apache/incubator-pinot/pull/6559#issuecomment-777917013
> > I don't think the following 2 changes are needed for the optimization done in this PR. > > ``` > > private static final int INITIAL_MAP_SIZE = (int) ((1 << 9) * 0.75f); > > private static final int MAX_CACHING_MAP_SIZE = (int) ((1 << 20) * 0.75f); > > ``` > > > > > > We should be able to keep them to their original values right since the optimization done in this PR for IntGroupId map internally uses the capacity of 4K to align with the page size and is not really dependent on the above 2 changes. Just wondering if the above 2 changes can impact performance in any case. I don't think they are related to this PR. > > These 2 changes are just making the value calculation more obvious for readability. Because we use the map size as threshold, we should multiply it by the load factor. There is no performance impact. Discussed offline with @Jackie-Jiang to clarify this. The difference between this and standard Java hashmap is that in the latter we provide initial capacity and the load factor as the input and then load factor is used to compute the threshold size (expected size) after which resize is triggered. However, in case of Int2IntOpenHashMap, the expected size should be provided as the input. That's the reason to use the load factor as the multipler. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
