lucene-java-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Oliver Hutchison" <ohutchi...@aconex.com>
Subject RE: Poor performance "race condition" in FieldSortedHitQueue
Date Wed, 09 Aug 2006 22:47:17 GMT
Yonik, 

> most easily implemented in Java5 via Future.

I didn't use Java5 as I had a feeling that code is Lucene needs to compile
on Java1.3 right? 

> I don't think you need two maps though, right?  just stick a 
> placeholder in the outer map.

I'm using 2 maps mainly because it simplifies the implementation.
Technically all that is needed is a singe map with a key that is a composite
of index reader and field name however, given that there is also the
requirement that we only maintain a weak reference to the index reader and
the associated need to clean up the cache if the reader gets gc'd, it was
simpler for me to simulate the composite key using the 2 maps. 

On a related note it would be great if there was a way to plug a custom
FieldCache implementation into Lucene, given there is a FieldCache interface
it's a shame there's no way to actually provide an alternative
implementation.

Since there seems to be a decent level of interest in this I'll try to put
together a patch and raise an issue over the next few days. 

Cheers,

Oliver

> On 8/9/06, Oliver Hutchison <ohutchison@aconex.com> wrote:
> > Otis, Doron, thanks for the feedback.
> >
> > First up I'd just like to say that I totally agree with 
> Doron on this 
> > - any attempt to fix this issue needs to be done using as 
> fine grain 
> > synchronization as is possible or you'd just be introducing a new 
> > bottle neck.
> >
> > It terms of the level of granularity, the work around I 
> posted in my 
> > previous email and the approach suggested by Doron are 
> basically the 
> > same (though Doron's code is certainly preferable) and I 
> can certainly 
> > say that synchronizing the object creation against the 
> field name does 
> > solve the problem.
> >
> > However I have another solution that I'm working on that may be 
> > cleaner - by encapsulate the caching logic that is currently spread 
> > across FieldCacheImpl and FieldSortedHitQueue it becomes 
> quite easy to 
> > implement a more complex but certainly more fine grained level of 
> > synchronization and we don't have to worry about 
> synchronizing against 
> > an interned String or using some other trick to synchronize 
> on the field name.
> >
> > I currently have:
> >
> > public abstract class Cache {
> >
> >         private final Map readerCache = new WeakHashMap();
> >
> >         protected Cache() {
> >         }
> >
> >         protected abstract Object createValue(IndexReader reader, 
> > Object
> > key)
> >                         throws IOException;
> >
> >         public Object get(IndexReader reader, Object key) throws 
> > IOException {
> >                 Map innerCache;
> >                 Object value;
> >                 synchronized (readerCache) {
> >                         innerCache = (Map) readerCache.get(reader);
> >                         // no inner cache create it
> >                         if (innerCache == null) {
> >                                 innerCache = new HashMap();
> >                                 readerCache.put(reader, innerCache);
> >                                 value = null;
> >                         } else {
> >                                 value = innerCache.get(key);
> >                         }
> >                         if (value == null) {
> >                                 value = new CreationPlaceholder();
> >                                 readerCache.put(reader, value);
> >                         }
> >                 }
> >                 if (value instanceof CreationPlaceholder) {
> >                         // must be one of the first threads 
> to request 
> > this value,
> >                         // synchronize on the 
> CreationPlaceholder so 
> > we don't block
> >                         // any other calls for different values
> >                         CreationPlaceholder ph = 
> (CreationPlaceholder) 
> > value;
> >                         synchronized (ph) {
> >                                 // if this thread is the very first 
> > one to reach this point
> >                                 // then this test will be 
> true and we 
> > should do the creation
> >                                 if (ph.value == null) {
> >                                         ph.value = 
> createValue(reader, key);
> >                                         synchronized (readerCache) {
> >                                                 innerCache.put(key, 
> > ph.value);
> >                                         }
> >                                 }
> >                                 return ph.value;
> >                         }
> >                 }
> >                 return value;
> >         }
> >
> >         static final class CreationPlaceholder {
> >                 Object value;
> >         }
> > }
> >
> >
> > class FieldCacheImpl implements FieldCache {
> >
> > ...
> >
> >         public String[] getStrings(IndexReader reader, String field)
> >                         throws IOException {
> >                 return (String[]) stringsCache.get(reader, field);
> >         }
> >
> >         Cache stringsCache = new Cache() {
> >
> >                 protected Object createValue(IndexReader reader, 
> > Object
> > fieldKey)
> >                                 throws IOException {
> >                         String field = ((String) fieldKey).intern();
> >
> > ... create String[] ...
> >
> >                         return retArray;
> >                 }
> >         };
> >
> >         public StringIndex getStringIndex(IndexReader 
> reader, String field)
> >                         throws IOException {
> >                 return (StringIndex) 
> stringsIndexCache.get(reader, field);
> >         }
> >
> >         Cache stringsIndexCache = new Cache() {
> >
> >                 protected Object createValue(IndexReader reader, 
> > Object
> > fieldKey)
> >                                 throws IOException {
> >                         String field = ((String) fieldKey).intern();
> >
> > ... create StringIndex ...
> >
> >                         return value;
> >                 }
> >         };
> >
> > ... etc
> >
> > }
> >
> > Is this an avenue worth pursuing further? Or are you guys happy to 
> > simply synchronizing on the field?
> >
> > Thanks again,
> >
> > Oliver
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-user-help@lucene.apache.org
> 
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-user-help@lucene.apache.org


Mime
View raw message