uschindler commented on PR #15585: URL: https://github.com/apache/lucene/pull/15585#issuecomment-3810828129
Hi, > Thanks @shubhamsrkdev -- I think this is ready. I'll merge soon. Let's watch builds closely... I worry a bit about the GC implications of this change, for Lucene usage that has a great many clones (many queries in flight, GC being lazy since there's lots of heap headroom say) and all of those clones pointing to this one `AtomicInteger`... likely it's fine but we've had GC issues in the past when `MMapDirectory` tried to carefully track clones (before Panama gave us better mmap APIs -- no more scary `Unsafe` for unmapping). this is not an issue here. The clones only share the same AtomicInteger. Basically all clones also refer to the same Arena (which is grouped, too so has additional stuff). Basically this is all fine, the problems with tracking clones were a different category of issues because we needed a pointer from the parent input to its childs and this goes worse. Here we just refer to some same object, which itsself has no reference to the clones. Once refcounting is zero the atomic integer gets free for GC. The problem with the master refering to childs was that as ong as the master input refers to its childs, those cannot be GCed. As we never call close on clones there is no way to figure out for the master that it is gone. The hard relationship master -> clone was the killer. Uwe -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
