I agree we should only promise best-effort, not immediate detection on close, so we shouldn't be checking volatile refCount.
Mike On Sat, Jun 20, 2009 at 2:32 PM, Yonik Seeley<yo...@lucidimagination.com> wrote: > On Sat, Jun 20, 2009 at 2:10 PM, Shai Erera<ser...@gmail.com> wrote: >> I noticed both IndexReader and IndexWriter call ensureOpen in almost every >> method. How critical is this check? Why would someone call any of these when >> the writer or reader are close? > > It's to catch user errors, calling methods after the reader is closed. > > I do have a slight issue with it in that it's a penalty to correctly > implemented programs. The original implementation checked a > non-volatile "boolean closed" variable (from what I remember). I > argued against ensureOpen() for maxDoc() and numDocs() for performance > reasons. At some point, ensureOpen() was changed to check the > volatile refCount. On current x86 systems, this volatile read simply > prevents optimizations and instruction reordering across the volatile > read, but the penalty on some other systems is greater since it can > cause/include L1/L2 cache flushes. It's even worse for something like > ParallelReader which has multiple levels of checks. > > So, IMO: ensureOpen() should not consult volatile variables. open and > isOpen variables should not be volatile. Throwing of AlreadyClosed > exceptions should be on a best-effort basis (not immediately > guaranteed). > > -Yonik > http://www.lucidimagination.com > > --------------------------------------------------------------------- > To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: java-dev-h...@lucene.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org For additional commands, e-mail: java-dev-h...@lucene.apache.org
