From e715c5601e2e8bb69fc73d25efbb152fa6f54e3e Mon Sep 17 00:00:00 2001 From: Satia Herfert Date: Thu, 29 Mar 2018 17:25:08 +0200 Subject: [PATCH] Make TemporalIndexCache use a ConcurrentHashMap --- .../index/schema/TemporalIndexAccessor.java | 4 +- .../impl/index/schema/TemporalIndexCache.java | 210 ++++++++++-------- .../impl/index/schema/TemporalIndexFiles.java | 3 +- .../TemporalIndexPopulatingUpdater.java | 17 +- .../index/schema/TemporalIndexPopulator.java | 4 +- .../index/schema/TemporalIndexReader.java | 4 +- .../index/schema/TemporalIndexUpdater.java | 17 +- .../index/schema/TemporalIndexCacheTest.java | 28 ++- 8 files changed, 152 insertions(+), 135 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexAccessor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexAccessor.java index f10be2ea1affa..4370b7db60f5b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexAccessor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexAccessor.java @@ -47,7 +47,7 @@ import static org.neo4j.helpers.collection.Iterators.concatResourceIterators; import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexBase.forAll; -class TemporalIndexAccessor extends TemporalIndexCache, IOException> implements IndexAccessor +class TemporalIndexAccessor extends TemporalIndexCache> implements IndexAccessor { private final SchemaIndexDescriptor descriptor; @@ -197,7 +197,7 @@ public TemporalIndexPartReader newReader() } } - static class PartFactory implements TemporalIndexCache.Factory, IOException> + static class PartFactory implements TemporalIndexCache.Factory> { private final PageCache pageCache; private final FileSystemAbstraction fs; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCache.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCache.java index 281b20831dada..ac6b288ed7c54 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCache.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCache.java @@ -19,33 +19,33 @@ */ package org.neo4j.kernel.impl.index.schema; -import java.util.Arrays; +import java.io.IOException; +import java.io.UncheckedIOException; import java.util.Iterator; -import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import org.neo4j.values.storable.ValueGroup; import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.date; +import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.duration; import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.localDateTime; -import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.zonedDateTime; import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.localTime; +import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.zonedDateTime; import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.zonedTime; -import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.duration; /** * Cache for lazily creating parts of the temporal index. Each part is created using the factory * the first time it is selected in a select() query, or the first time it's explicitly * asked for using e.g. date(). - * + *

* Iterating over the cache will return all currently created parts. * * @param Type of parts - * @param Type of exception potentially thrown during creation */ -class TemporalIndexCache implements Iterable +class TemporalIndexCache implements Iterable { - private final Factory factory; + private final Factory factory; enum Offset { @@ -57,20 +57,11 @@ enum Offset duration } - private final Object dateLock = new Object(); - private final Object localDateTimeLock = new Object(); - private final Object zonedDateTimeLock = new Object(); - private final Object localTimeLock = new Object(); - private final Object zonedTimeLock = new Object(); - private final Object durationLock = new Object(); + private final ConcurrentHashMap cache = new ConcurrentHashMap<>(); - private T[] parts; - - TemporalIndexCache( Factory factory ) + TemporalIndexCache( Factory factory ) { this.factory = factory; - //noinspection unchecked - this.parts = (T[]) new Object[Offset.values().length]; } /** @@ -81,26 +72,6 @@ enum Offset * @return selected part */ T uncheckedSelect( ValueGroup valueGroup ) - { - try - { - return select( valueGroup ); - } - catch ( Exception t ) - { - throw new RuntimeException( t ); - } - } - - /** - * Select the part corresponding to the given ValueGroup. Creates the part if needed, - * in which case an exception of type E might be thrown. - * - * @param valueGroup target value group - * @return selected part - * @throws E exception potentially thrown during creation - */ - T select( ValueGroup valueGroup ) throws E { switch ( valueGroup ) { @@ -127,6 +98,25 @@ T select( ValueGroup valueGroup ) throws E } } + /** + * Select the part corresponding to the given ValueGroup. Creates the part if needed, + * in which case an exception of type E might be thrown. + * + * @param valueGroup target value group + * @return selected part + */ + T select( ValueGroup valueGroup ) throws IOException + { + try + { + return uncheckedSelect( valueGroup ); + } + catch ( UncheckedIOException e ) + { + throw e.getCause(); + } + } + /** * Select the part corresponding to the given ValueGroup, apply function to it and return the result. * If the part isn't created yet return orElse. @@ -137,103 +127,125 @@ T select( ValueGroup valueGroup ) throws E * @param type of result * @return the result */ - RESULT selectOrElse( ValueGroup valueGroup, Function function, RESULT orElse ) + RESULT selectOrElse( ValueGroup valueGroup, Function function, RESULT orElse ) { + T cachedValue; switch ( valueGroup ) { case DATE: - return parts[date.ordinal()] != null ? function.apply( parts[date.ordinal()] ) : orElse; - + cachedValue = cache.get( date ); + break; case LOCAL_DATE_TIME: - return parts[localDateTime.ordinal()] != null ? function.apply( parts[localDateTime.ordinal()] ) : orElse; - + cachedValue = cache.get( localDateTime ); + break; case ZONED_DATE_TIME: - return parts[zonedDateTime.ordinal()] != null ? function.apply( parts[zonedDateTime.ordinal()] ) : orElse; - + cachedValue = cache.get( zonedDateTime ); + break; case LOCAL_TIME: - return parts[localTime.ordinal()] != null ? function.apply( parts[localTime.ordinal()] ) : orElse; - + cachedValue = cache.get( localTime ); + break; case ZONED_TIME: - return parts[zonedTime.ordinal()] != null ? function.apply( parts[zonedTime.ordinal()] ) : orElse; - + cachedValue = cache.get( zonedTime ); + break; case DURATION: - return parts[duration.ordinal()] != null ? function.apply( parts[duration.ordinal()] ) : orElse; - + cachedValue = cache.get( duration ); + break; default: throw new IllegalStateException( "Unsupported value group " + valueGroup ); } + + return cachedValue != null ? function.apply( cachedValue ) : orElse; } - T date() throws E + private T date() throws UncheckedIOException { - synchronized ( dateLock ) + return cache.computeIfAbsent( date, d -> { - if ( parts[date.ordinal()] == null ) + try { - parts[date.ordinal()] = factory.newDate(); + return factory.newDate(); } - } - return parts[date.ordinal()]; + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); } - T localDateTime() throws E + private T localDateTime() throws UncheckedIOException { - synchronized ( localDateTimeLock ) + + return cache.computeIfAbsent( localDateTime, d -> { - if ( parts[localDateTime.ordinal()] == null ) + try { - parts[localDateTime.ordinal()] = factory.newLocalDateTime(); + return factory.newLocalDateTime(); } - } - return parts[localDateTime.ordinal()]; + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); } - T zonedDateTime() throws E + private T zonedDateTime() throws UncheckedIOException { - synchronized ( zonedDateTimeLock ) + return cache.computeIfAbsent( zonedDateTime, d -> { - if ( parts[zonedDateTime.ordinal()] == null ) + try { - parts[zonedDateTime.ordinal()] = factory.newZonedDateTime(); + return factory.newZonedDateTime(); } - } - return parts[zonedDateTime.ordinal()]; + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); } - T localTime() throws E + private T localTime() throws UncheckedIOException { - synchronized ( localTimeLock ) + return cache.computeIfAbsent( localTime, d -> { - if ( parts[localTime.ordinal()] == null ) + try { - parts[localTime.ordinal()] = factory.newLocalTime(); + return factory.newLocalTime(); } - } - return parts[localTime.ordinal()]; + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); } - T zonedTime() throws E + private T zonedTime() throws UncheckedIOException { - synchronized ( zonedTimeLock ) + return cache.computeIfAbsent( zonedTime, d -> { - if ( parts[zonedTime.ordinal()] == null ) + try { - parts[zonedTime.ordinal()] = factory.newZonedTime(); + return factory.newZonedTime(); } - } - return parts[zonedTime.ordinal()]; + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); } - T duration() throws E + private T duration() throws UncheckedIOException { - synchronized ( durationLock ) + return cache.computeIfAbsent( duration, d -> { - if ( parts[duration.ordinal()] == null ) + try { - parts[duration.ordinal()] = factory.newDuration(); + return factory.newDuration(); } - } - return parts[duration.ordinal()]; + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); } void loadAll() @@ -256,22 +268,26 @@ void loadAll() @Override public Iterator iterator() { - return Arrays.stream( parts ).filter( Objects::nonNull ).iterator(); + return cache.values().iterator(); } /** * Factory used by the TemporalIndexCache to create parts. * * @param Type of parts - * @param Type of exception potentially thrown during create */ - interface Factory + interface Factory { - T newDate() throws E; - T newLocalDateTime() throws E; - T newZonedDateTime() throws E; - T newLocalTime() throws E; - T newZonedTime() throws E; - T newDuration() throws E; + T newDate() throws IOException; + + T newLocalDateTime() throws IOException; + + T newZonedDateTime() throws IOException; + + T newLocalTime() throws IOException; + + T newZonedTime() throws IOException; + + T newDuration() throws IOException; } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexFiles.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexFiles.java index 2354dfdc02243..ca8954082d13f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexFiles.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexFiles.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.index.schema; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -63,7 +64,7 @@ Iterable existing() return existing; } - void loadExistingIndexes( TemporalIndexCache indexCache ) throws E + void loadExistingIndexes( TemporalIndexCache indexCache ) throws IOException { for ( FileLayout fileLayout : existing() ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexPopulatingUpdater.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexPopulatingUpdater.java index 70d81fb3456c3..f2097af30e510 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexPopulatingUpdater.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexPopulatingUpdater.java @@ -25,8 +25,9 @@ import org.neo4j.kernel.api.index.IndexEntryUpdate; import org.neo4j.kernel.api.index.IndexUpdater; import org.neo4j.kernel.api.index.PropertyAccessor; +import org.neo4j.values.storable.ValueGroup; -public class TemporalIndexPopulatingUpdater extends TemporalIndexCache implements IndexUpdater +public class TemporalIndexPopulatingUpdater extends TemporalIndexCache implements IndexUpdater { TemporalIndexPopulatingUpdater( TemporalIndexPopulator populator, PropertyAccessor propertyAccessor ) { @@ -78,7 +79,7 @@ public void close() throws IOException, IndexEntryConflictException } } - static class PartFactory implements TemporalIndexCache.Factory + static class PartFactory implements TemporalIndexCache.Factory { private final TemporalIndexPopulator populator; private PropertyAccessor propertyAccessor; @@ -92,37 +93,37 @@ static class PartFactory implements TemporalIndexCache.Factory, IOException> implements IndexPopulator +class TemporalIndexPopulator extends TemporalIndexCache> implements IndexPopulator { private final IndexSamplerWrapper sampler; @@ -207,7 +207,7 @@ public IndexSample sampleResult() } } - static class PartFactory implements TemporalIndexCache.Factory, IOException> + static class PartFactory implements TemporalIndexCache.Factory> { private final PageCache pageCache; private final FileSystemAbstraction fs; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexReader.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexReader.java index cbdf1b0a1b9cb..3b8ff08629c1d 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexReader.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexReader.java @@ -40,7 +40,7 @@ import static org.neo4j.kernel.impl.index.schema.fusion.FusionIndexBase.forAll; -class TemporalIndexReader extends TemporalIndexCache,IOException> implements IndexReader +class TemporalIndexReader extends TemporalIndexCache> implements IndexReader { private final SchemaIndexDescriptor descriptor; @@ -131,7 +131,7 @@ private boolean validPredicate( IndexQuery predicate ) * To create TemporalIndexPartReaders on demand, the PartFactory maintains a reference to the parent TemporalIndexAccessor. * The creation of a part reader can then be delegated to the correct PartAccessor. */ - static class PartFactory implements TemporalIndexCache.Factory, IOException> + static class PartFactory implements TemporalIndexCache.Factory> { private final TemporalIndexAccessor accessor; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexUpdater.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexUpdater.java index b4a38c408865b..39bb44a8f529e 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexUpdater.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexUpdater.java @@ -26,8 +26,9 @@ import org.neo4j.kernel.api.index.IndexUpdater; import org.neo4j.kernel.impl.api.index.IndexUpdateMode; import org.neo4j.kernel.impl.index.schema.fusion.FusionIndexBase; +import org.neo4j.values.storable.ValueGroup; -public class TemporalIndexUpdater extends TemporalIndexCache,IOException> implements IndexUpdater +public class TemporalIndexUpdater extends TemporalIndexCache> implements IndexUpdater { TemporalIndexUpdater( TemporalIndexAccessor accessor, IndexUpdateMode mode ) { @@ -70,7 +71,7 @@ public void close() throws IOException FusionIndexBase.forAll( NativeSchemaIndexUpdater::close, this ); } - static class PartFactory implements TemporalIndexCache.Factory,IOException> + static class PartFactory implements TemporalIndexCache.Factory> { private final TemporalIndexAccessor accessor; @@ -85,37 +86,37 @@ static class PartFactory implements TemporalIndexCache.Factory newDate() throws IOException { - return accessor.date().newUpdater( mode ); + return accessor.select( ValueGroup.DATE ).newUpdater( mode ); } @Override public NativeSchemaIndexUpdater newLocalDateTime() throws IOException { - return accessor.localDateTime().newUpdater( mode ); + return accessor.select(ValueGroup.LOCAL_DATE_TIME).newUpdater( mode ); } @Override public NativeSchemaIndexUpdater newZonedDateTime() throws IOException { - return accessor.zonedDateTime().newUpdater( mode ); + return accessor.select(ValueGroup.ZONED_DATE_TIME).newUpdater( mode ); } @Override public NativeSchemaIndexUpdater newLocalTime() throws IOException { - return accessor.localTime().newUpdater( mode ); + return accessor.select(ValueGroup.LOCAL_TIME).newUpdater( mode ); } @Override public NativeSchemaIndexUpdater newZonedTime() throws IOException { - return accessor.zonedTime().newUpdater( mode ); + return accessor.select(ValueGroup.ZONED_TIME).newUpdater( mode ); } @Override public NativeSchemaIndexUpdater newDuration() throws IOException { - return accessor.duration().newUpdater( mode ); + return accessor.select(ValueGroup.DURATION).newUpdater( mode ); } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCacheTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCacheTest.java index cee76edf348cf..d1514abff20e8 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCacheTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCacheTest.java @@ -45,24 +45,22 @@ public class TemporalIndexCacheTest public void shouldIterateOverCreatedParts() throws Exception { StringFactory factory = new StringFactory(); - TemporalIndexCache cache = new TemporalIndexCache<>( factory ); + TemporalIndexCache cache = new TemporalIndexCache<>( factory ); assertEquals( Iterables.count( cache ), 0 ); - cache.localDateTime(); - cache.zonedTime(); + cache.select( LOCAL_DATE_TIME ); + cache.select( ZONED_TIME ); - assertThat( factory.localDateTimeCounter.get(), equalTo( 1 ) ); assertThat( cache, containsInAnyOrder( "LocalDateTime", "ZonedTime" ) ); - cache.date(); - cache.localTime(); - cache.localDateTime(); - cache.zonedTime(); - cache.zonedDateTime(); - cache.duration(); + cache.select( DATE ); + cache.select( LOCAL_TIME ); + cache.select( LOCAL_DATE_TIME ); + cache.select( ZONED_TIME ); + cache.select( ZONED_DATE_TIME ); + cache.select( DURATION ); - assertThat( factory.localDateTimeCounter.get(), equalTo( 1 ) ); assertThat( cache, containsInAnyOrder( "Date", "LocalDateTime", "ZonedDateTime", "LocalTime", "ZonedTime", "Duration" ) ); } @@ -70,7 +68,7 @@ public void shouldIterateOverCreatedParts() throws Exception public void stressCache() throws Exception { StringFactory factory = new StringFactory(); - TemporalIndexCache cache = new TemporalIndexCache<>( factory ); + TemporalIndexCache cache = new TemporalIndexCache<>( factory ); CacheStresser[] stressers = new CacheStresser[100]; for ( int i = 0; i < stressers.length; i++ ) @@ -107,11 +105,11 @@ public void stressCache() throws Exception static class CacheStresser extends Thread { - TemporalIndexCache cache; + TemporalIndexCache cache; Random r = new Random(); Exception failed; - CacheStresser( TemporalIndexCache cache ) + CacheStresser( TemporalIndexCache cache ) { this.cache = cache; } @@ -151,7 +149,7 @@ private void stress() throws Exception } } - static class StringFactory implements TemporalIndexCache.Factory + static class StringFactory implements TemporalIndexCache.Factory { AtomicInteger dateCounter = new AtomicInteger( 0 ); AtomicInteger localDateTimeCounter = new AtomicInteger( 0 );