bq. The change on LUCENE-3636 really isn't about cheapness True, but I imagine that if it was really cheap to init a new IndexSearcher over an IndexReader, then we would just have ReaderManager return IndexReader, and let the caller init their own IndexSearcher, in which case LUCENE-3636 was pointless because you wouldn't need to worry about setting stats on IndexSearcher returned from SearcherManager.
And I would still propose to create the RefCount interface, even if we only used ReaderManager, because I am already aware of 3 copies of the same logic that manage IndexSearcher-like objects between search threads: SearcherManager, NRTManager, and my own version which manages a pair of IndexSearcher and TaxonomyReader. So are you against the interface, or having IndexSearcher implement it? I'm ok if we don't make IS impl this interface, and only IR, TR will, but I think that this interface could be useful. Uwe, I'm aware of all the changes in trunk, but since trunk is nowhere near release, I have to work with 3x, and therefore proposing changes that will help 3x users too. And in 3x, when I use IndexSearcher(IndexReader) ctor, it goes and gathers sub-readers. In trunk it calls ReaderUtil.leaves(). In the past it used to sort IndexReaders. So if we really believe that init'ing IS is that cheap (on 3x, given IndexReader, and trunk), why don't we change SearcherManager to ReaderManager and be done with it? NOTE: this is unrelated to the RefCount interface at all. Shai On Sun, Dec 11, 2011 at 4:55 PM, Uwe Schindler <[email protected]> wrote: > Hi, > > > I would like to introduce a RefCount interface with the methods: > incRef(), > tryIncRef(), decRef() and getRefCount(). > > I am +/-0. > > > These methods are already declared today on IndexReader and > TaxonomyReader > (module/facet), and I see no reason why we wouldn't have an interface for > doing that. Then it will allow someone to e.g. incRef() a set of RefCount > objects or something like that. > > I am at all unhappy with the ref counting available in public API. > > > Also, I think it will be good if IndexSearcher implements this interface > too, delegating calls to its underlying indexReader. Because today it seems > very awkward to me that if I want to write a SearcherManager-like object, > which manages IndexSearchers and essentially keeps track of their > references, I need to know that I should call > s.getIndexReader().inc/decRef(). > > I strongly disagree with that. Maybe you have not yet noticed, in Lucene > trunk we removed (and deprecated in 3.x) all ctors in IndexSearcher that > take Directory, so IndexSearcher has no expensive open/close anymore. It is > now only a lightweight wrapper around a set of IndexReaders, construction > cost is almost zero. IndexSearcher also no longer takes care of closing > IndexReader it wraps. In general classes like SearcherManager should always > only manage the IndexReaders and return a new IndexSearcher wrapped around > one from the pool of IndexReaders. So SearcherManager only manages > IndexReaders and counts its references, not searchers. This might be still > different in the current implementation, but this is how it should look > like. And please don't tell me about the very expensive cost of creating > Eden java objects that live on the stack... > > > Following that, it may be possible to make SearcherManager / > SearcherLifetimeManager generic objects which take RefCount objects, but > that's for another :). > > +/-0 > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
