Author: epbernard Date: 2008-05-30 18:28:11 -0400 (Fri, 30 May 2008) New Revision: 14719 Modified: search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java search/trunk/src/java/org/hibernate/search/store/DirectoryProviderFactory.java search/trunk/src/test/org/hibernate/search/test/RamDirectoryTest.java Log: HSEARCH-19 no longer filter classes when DPs contains all the targeted classes Modified: search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java 2008-05-30 22:26:13 UTC (rev 14718) +++ search/trunk/src/java/org/hibernate/search/engine/SearchFactoryImplementor.java 2008-05-30 22:28:11 UTC (rev 14719) @@ -2,6 +2,7 @@ package org.hibernate.search.engine; import java.util.Map; +import java.util.Set; import java.util.concurrent.locks.ReentrantLock; import org.hibernate.search.SearchFactory; @@ -44,4 +45,8 @@ public String getIndexingStrategy(); public void close(); + + void addClassToDirectoryProvider(Class clazz, DirectoryProvider<?> directoryProvider); + + Set<Class> getClassesInDirectoryProvider(DirectoryProvider<?> directoryProvider); } Modified: search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-05-30 22:26:13 UTC (rev 14718) +++ search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-05-30 22:28:11 UTC (rev 14719) @@ -4,7 +4,9 @@ import java.beans.Introspector; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -13,6 +15,7 @@ import java.util.WeakHashMap; import java.util.concurrent.locks.ReentrantLock; +import org.apache.lucene.analysis.Analyzer; import org.hibernate.annotations.common.reflection.ReflectionManager; import org.hibernate.annotations.common.reflection.XClass; import org.hibernate.annotations.common.reflection.java.JavaReflectionManager; @@ -43,7 +46,6 @@ import org.hibernate.search.store.DirectoryProvider; import org.hibernate.search.store.DirectoryProviderFactory; import org.hibernate.search.store.optimization.OptimizerStrategy; -import org.apache.lucene.analysis.Analyzer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,10 +64,11 @@ private final Map<Class, DocumentBuilder<Object>> documentBuilders = new HashMap<Class, DocumentBuilder<Object>>(); //keep track of the index modifiers per DirectoryProvider since multiple entity can use the same directory provider + //TODO move the ReentrantLock into DirectoryProviderData.lock, add a getDPLock(DP) and add a Set<DP> getDirectoryProviders() method. private final Map<DirectoryProvider, ReentrantLock> lockableDirectoryProviders = new HashMap<DirectoryProvider, ReentrantLock>(); - private final Map<DirectoryProvider, OptimizerStrategy> dirProviderOptimizerStrategies = - new HashMap<DirectoryProvider, OptimizerStrategy>(); + private final Map<DirectoryProvider, DirectoryProviderData> dirProviderData = + new HashMap<DirectoryProvider, DirectoryProviderData>(); private Worker worker; private ReaderProvider readerProvider; private BackendQueueProcessorFactory backendQueueProcessorFactory; @@ -139,6 +142,19 @@ } } + public void addClassToDirectoryProvider(Class clazz, DirectoryProvider<?> directoryProvider) { + DirectoryProviderData data = dirProviderData.get(directoryProvider); + if (data == null) { + data = new DirectoryProviderData(); + dirProviderData.put( directoryProvider, data ); + } + data.classes.add(clazz); + } + + public Set<Class> getClassesInDirectoryProvider(DirectoryProvider<?> directoryProvider) { + return Collections.unmodifiableSet( dirProviderData.get(directoryProvider).classes ); + } + private void bindFilterDefs(XClass mappedXClass) { FullTextFilterDef defAnn = mappedXClass.getAnnotation( FullTextFilterDef.class ); if ( defAnn != null ) { @@ -227,7 +243,12 @@ } public void addOptimizerStrategy(DirectoryProvider<?> provider, OptimizerStrategy optimizerStrategy) { - dirProviderOptimizerStrategies.put( provider, optimizerStrategy ); + DirectoryProviderData data = dirProviderData.get(provider); + if (data == null) { + data = new DirectoryProviderData(); + dirProviderData.put( provider, data ); + } + data.optimizerStrategy = optimizerStrategy; } public void addIndexingParameters(DirectoryProvider<?> provider, LuceneIndexingParameters indexingParams) { @@ -235,7 +256,7 @@ } public OptimizerStrategy getOptimizerStrategy(DirectoryProvider<?> provider) { - return dirProviderOptimizerStrategies.get( provider ); + return dirProviderData.get( provider ).optimizerStrategy; } public LuceneIndexingParameters getIndexingParameters(DirectoryProvider<?> provider ) { @@ -301,7 +322,7 @@ XClass mappedXClass = reflectionManager.toXClass(mappedClass); if ( mappedXClass != null) { if ( mappedXClass.isAnnotationPresent( Indexed.class ) ) { - DirectoryProviderFactory.DirectoryProviders providers = factory.createDirectoryProviders( mappedXClass, cfg, this ); + DirectoryProviderFactory.DirectoryProviders providers = factory.createDirectoryProviders( mappedXClass, cfg, this, reflectionManager ); final DocumentBuilder<Object> documentBuilder = new DocumentBuilder<Object>( mappedXClass, context, providers.getProviders(), providers.getSelectionStrategy(), @@ -349,4 +370,9 @@ public FilterDef getFilterDefinition(String name) { return filterDefinitions.get( name ); } + + private static class DirectoryProviderData { + public OptimizerStrategy optimizerStrategy; + public Set<Class> classes = new HashSet<Class>(2); + } } Modified: search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java 2008-05-30 22:26:13 UTC (rev 14718) +++ search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java 2008-05-30 22:28:11 UTC (rev 14719) @@ -69,6 +69,8 @@ private org.apache.lucene.search.Query luceneQuery; private Class[] classes; private Set<Class> classesAndSubclasses; + //optimization: if we can avoid the filter clause (we can most of the time) do it as it has a significant perf impact + private boolean needClassFilterClause; private Integer firstResult; private Integer maxResults; private Integer resultSize; @@ -384,12 +386,12 @@ } private org.apache.lucene.search.Query filterQueryByClasses(org.apache.lucene.search.Query luceneQuery) { - //A query filter is more practical than a manual class filtering post query (esp on scrollable resultsets) - //it also probably minimise the memory footprint - if ( classesAndSubclasses == null ) { + if ( ! needClassFilterClause ) { return luceneQuery; } else { + //A query filter is more practical than a manual class filtering post query (esp on scrollable resultsets) + //it also probably minimise the memory footprint BooleanQuery classFilter = new BooleanQuery(); //annihilate the scoring impact of DocumentBuilder.CLASS_FIELDNAME classFilter.setBoost( 0 ); @@ -429,13 +431,13 @@ List<DirectoryProvider> directories = new ArrayList<DirectoryProvider>(); Similarity searcherSimilarity = null; - + //TODO check if caching this work for the last n list of classes makes a perf boost if ( classes == null || classes.length == 0 ) { //no class means all classes for (DocumentBuilder builder : builders.values()) { searcherSimilarity = checkSimilarity( searcherSimilarity, builder ); final DirectoryProvider[] directoryProviders = builder.getDirectoryProviderSelectionStrategy().getDirectoryProvidersForAllShards(); - populateDirectories( directories, directoryProviders ); + populateDirectories( directories, directoryProviders, searchFactoryImplementor ); } classesAndSubclasses = null; } @@ -455,11 +457,30 @@ final DirectoryProvider[] directoryProviders = builder.getDirectoryProviderSelectionStrategy().getDirectoryProvidersForAllShards(); searcherSimilarity = checkSimilarity( searcherSimilarity, builder ); - populateDirectories( directories, directoryProviders ); + populateDirectories( directories, directoryProviders, searchFactoryImplementor ); } classesAndSubclasses = involvedClasses; } + //compute optimization needClassFilterClause + //if at least one DP contains one class that is not part of the targeted classesAndSubclasses we can't optimize + if ( classesAndSubclasses != null) { + for (DirectoryProvider dp : directories) { + final Set<Class> classesInDirectoryProvider = searchFactoryImplementor.getClassesInDirectoryProvider( dp ); + // if a DP contains only one class, we know for sure it's part of classesAndSubclasses + if ( classesInDirectoryProvider.size() > 1 ) { + //risk of needClassFilterClause + for (Class clazz : classesInDirectoryProvider) { + if ( ! classesAndSubclasses.contains( clazz ) ) { + this.needClassFilterClause = true; + break; + } + } + } + if ( this.needClassFilterClause ) break; + } + } + //set up the searcher final DirectoryProvider[] directoryProviders = directories.toArray( new DirectoryProvider[directories.size()] ); IndexSearcher is = new IndexSearcher( searchFactoryImplementor.getReaderProvider().openReader( directoryProviders ) ); @@ -467,7 +488,8 @@ return is; } - private void populateDirectories(List<DirectoryProvider> directories, DirectoryProvider[] directoryProviders) { + private void populateDirectories(List<DirectoryProvider> directories, DirectoryProvider[] directoryProviders, + SearchFactoryImplementor searchFactoryImplementor) { for (DirectoryProvider provider : directoryProviders) { if ( !directories.contains( provider ) ) { directories.add( provider ); Modified: search/trunk/src/java/org/hibernate/search/store/DirectoryProviderFactory.java =================================================================== --- search/trunk/src/java/org/hibernate/search/store/DirectoryProviderFactory.java 2008-05-30 22:26:13 UTC (rev 14718) +++ search/trunk/src/java/org/hibernate/search/store/DirectoryProviderFactory.java 2008-05-30 22:28:11 UTC (rev 14719) @@ -51,7 +51,9 @@ private static final String SHARDING_STRATEGY = "sharding_strategy"; private static final String NBR_OF_SHARDS = SHARDING_STRATEGY + ".nbr_of_shards"; - public DirectoryProviders createDirectoryProviders(XClass entity, Configuration cfg, SearchFactoryImplementor searchFactoryImplementor) { + public DirectoryProviders createDirectoryProviders(XClass entity, Configuration cfg, + SearchFactoryImplementor searchFactoryImplementor, + ReflectionManager reflectionManager) { //get properties String directoryProviderName = getDirectoryProviderName( entity, cfg ); Properties[] indexProps = getDirectoryProperties( cfg, directoryProviderName ); @@ -63,7 +65,8 @@ String providerName = nbrOfProviders > 1 ? directoryProviderName + "." + index : directoryProviderName; - providers[index] = createDirectoryProvider( providerName, indexProps[index], searchFactoryImplementor ); + providers[index] = createDirectoryProvider( providerName, indexProps[index], + reflectionManager.toClass( entity ), searchFactoryImplementor ); } //define sharding strategy @@ -110,7 +113,8 @@ } } - private DirectoryProvider<?> createDirectoryProvider(String directoryProviderName, Properties indexProps, SearchFactoryImplementor searchFactoryImplementor) { + private DirectoryProvider<?> createDirectoryProvider(String directoryProviderName, Properties indexProps, + Class entity, SearchFactoryImplementor searchFactoryImplementor) { String className = indexProps.getProperty( "directory_provider" ); if ( StringHelper.isEmpty( className ) ) { className = DEFAULT_DIRECTORY_PROVIDER; @@ -135,12 +139,15 @@ int index = providers.indexOf( provider ); if ( index != -1 ) { //share the same Directory provider for the same underlying store - return providers.get( index ); + final DirectoryProvider<?> directoryProvider = providers.get( index ); + searchFactoryImplementor.addClassToDirectoryProvider(entity, directoryProvider); + return directoryProvider; } else { configureOptimizerStrategy( searchFactoryImplementor, indexProps, provider ); configureIndexingParameters( searchFactoryImplementor, indexProps, provider ); providers.add( provider ); + searchFactoryImplementor.addClassToDirectoryProvider(entity, provider); if ( !searchFactoryImplementor.getLockableDirectoryProviders().containsKey( provider ) ) { searchFactoryImplementor.getLockableDirectoryProviders().put( provider, new ReentrantLock() ); } Modified: search/trunk/src/test/org/hibernate/search/test/RamDirectoryTest.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/RamDirectoryTest.java 2008-05-30 22:26:13 UTC (rev 14718) +++ search/trunk/src/test/org/hibernate/search/test/RamDirectoryTest.java 2008-05-30 22:28:11 UTC (rev 14719) @@ -2,7 +2,11 @@ package org.hibernate.search.test; import org.hibernate.Session; +import org.hibernate.search.FullTextSession; +import org.hibernate.search.Search; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.TermQuery; /** * @author Emmanuel Bernard @@ -28,6 +32,11 @@ s = getSessions().openSession(); s.getTransaction().begin(); + TermQuery q = new TermQuery(new Term("alt_title", "hibernate")); + assertEquals( "does not properly filter", 0, + Search.createFullTextSession( s ).createFullTextQuery( q, Document.class ).list().size() ); + assertEquals( "does not properly filter", 1, + Search.createFullTextSession( s ).createFullTextQuery( q, Document.class, AlternateDocument.class ).list().size() ); s.delete( s.get( AlternateDocument.class, document.getId() ) ); s.getTransaction().commit(); s.close();