lhotari commented on PR #24463: URL: https://github.com/apache/pulsar/pull/24463#issuecomment-3116604855
@BewareMyPower My point from an earlier comment isn't addressed: > If we'd completely remove the cache, there would be more duplicate TopicName and NamespaceName instances and more duplicate string instance. For very long tenant, namespace and topic names, that could add up a significant amount of wasted heap memory. In this case, I think it's irrelevant to just benchmark the performance of creating/looking up TopicName instances. Duplicate `java.lang.String` instances could increase significantly after introducing a non-caching solution. Heapdump analysis tools have such checks for duplicate instances so that's one way how to compare the difference. I do agree that a lot of the TopicName handling code is a mess. For example in Topic listing, the topic name is converted from/to String multiple times. That's adding up a lot of pressure for a fast lookup solution when instances are cached. So I'm not against changing the current caching, it's just that there's a need to consider duplicate instances as well. It's likely that the caching is more relevant for NamespaceName than TopicName regarding duplicate instances. We might not be keeping a reference to TopicName instance in that many places when broker is running. -- 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...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
