jhuynh1 commented on pull request #6874: URL: https://github.com/apache/geode/pull/6874#issuecomment-941355730
To add to Kirk's prior suggestion (some suggestions are probably duplicated as we are currently discussing) The main difference between the two is the use of vs possible removal of the threadlocal We might be able to avoid the thread local entirely if we add a method to the QueryObserver interface such as createObserver() where it is expected to pass back a unique instance of an observer. Improvements to reuses/pool these can be made to reduce garbage later. Where we currently do start query, perhaps call this new createObserver() method and stuff it into the execution context. (https://github.com/apache/geode/blob/7dd614387e9f3133de77ff17675d6bf00051276c/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java#L342) Wherever we currently do QueryObserverHolder.getInstance(), instead do context.getObserver() There are a few locations of QueryObserverHolder.getInstance() like OrderbyComparator (https://github.com/apache/geode/blob/7dd614387e9f3133de77ff17675d6bf00051276c/geode-core/src/main/java/org/apache/geode/cache/query/internal/OrderByComparator.java#L116), that have a context present as a class variable that can be used. The execution context and observer should be gc'd eventually once the execution context is no longer being used. There is probably more I am missing but thought I'd put this down as a suggestion -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
