dsmiley commented on PR #11847: URL: https://github.com/apache/lucene/pull/11847#issuecomment-1279653616
The ThreadLocal aspect was a needless addition here; it could be reverted. > the private map just leaks for the caller by unnecessarily saving strings. it's a lot of overhead to datainput to add this map. keep in mind datainput isn't closeable and has other implementations such as ByteArrayDataInput. Let's imagine a small improvement to the patch to lazily create the map on first use. Some usages of ByteArrayDataInput and perhaps others would never use it (e.g. SynonymGraphFilter). Are there cases you see that would lead to undesirable use of these canonical strings? That it's not closable doesn't seem to be an issue to me since DataInput should not be outliving the things it was used to read; quite the reverse, no? -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
