gortiz commented on code in PR #12223: URL: https://github.com/apache/pinot/pull/12223#discussion_r1445893993
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexType.java: ########## @@ -91,6 +95,11 @@ public class DictionaryIndexType private static final Logger LOGGER = LoggerFactory.getLogger(DictionaryIndexType.class); private static final List<String> EXTENSIONS = Collections.singletonList(V1Constants.Dict.FILE_EXTENSION); + // Map containing columnName as key and the interner as value. The interner is common across all the segments. + // TODO: Check if tablename can be appended to the columnName. + private static Map<String, FALFInterner<String>> _strInternerInfoMap = new ConcurrentHashMap<>(); + private static Map<String, FALFInterner<byte[]>> _byteInternerInfoMap = new ConcurrentHashMap<>(); Review Comment: I'm thinking about this and one way to remove the static is to follow the current Pinot design pattern where these kind of structures have a lifetime bound to the query that use them. Specifically, we could keep these two maps in the index reader itself. This has an obvious disadvantage: there would be one interner per segment and query, so a single query touching two segments or two different queries touching the same segment will not share memory. But it would be a more conservative approach to what we do right now and we could extend the interner to be shared between segments in the future. -- 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. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org 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
