Skip to content

Commit

Permalink
Improve performance for many new fields introduction in mapping
Browse files Browse the repository at this point in the history
When we have many new fields keep being introduced, the immutable open map we used becomes more and more expensive because of its clone characteristics, and we use it in several places.

The usage semantics of it allows us to also use a CHM if we want to, but it would be nice to still maintain the concurrency aspects of volatile immutable map when the number of fields is sane.

Introduce a new map like data structure, that can switch internally to CHM when a certain threshold is met.

Also add a benchmark class to exploit the many new field mappings use case, which shows significant gains by using this change, to a level where mapping introduction is no longer a significant bottleneck.
closes #6707
  • Loading branch information
kimchy committed Jul 5, 2014
1 parent a9abf18 commit c8e5530
Show file tree
Hide file tree
Showing 14 changed files with 595 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@

import java.io.IOException;
import java.util.Collection;
import java.util.List;

/**
*/
Expand Down Expand Up @@ -181,7 +182,7 @@ public Boolean paramAsBooleanOptional(String key, Boolean defaultValue) {

private ImmutableMap<String, FieldMappingMetaData> findFieldMappingsByType(DocumentMapper documentMapper, GetFieldMappingsIndexRequest request) throws ElasticsearchException {
MapBuilder<String, FieldMappingMetaData> fieldMappings = new MapBuilder<>();
ImmutableList<FieldMapper> allFieldMappers = documentMapper.mappers().mappers();
final List<FieldMapper> allFieldMappers = documentMapper.mappers().mappers();
for (String field : request.fields()) {
if (Regex.isMatchAllPattern(field)) {
for (FieldMapper fieldMapper : allFieldMappers) {
Expand Down
177 changes: 177 additions & 0 deletions src/main/java/org/elasticsearch/common/collect/UpdateInPlaceMap.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.common.collect;

import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
import com.google.common.collect.Iterables;
import org.elasticsearch.ElasticsearchIllegalStateException;
import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;

import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicBoolean;

/**
* A map that exposes only read only methods, and can be mutated using a {@link #mutator()}. It
* allows for a cutoff switch between {@link ImmutableOpenMap} and {@link ConcurrentMap}, based on size, since as
* the size grows bigger, cloning the immutable map cost gets bigger and bigger, and might as well move to CHM.
* <p/>
* Note, its important to understand the semantics of the class and its mutator, its not an update in place, when
* CHM is used, changes to the mutator will be reflected in the existing maps!. This class should be used as if
* its a regular, mutable concurrent map, mutation can affect the existing map.
* <p/>
* This class only allows for a single concurrent mutator to execute at the same time.
*/
public final class UpdateInPlaceMap<K, V> {

final int switchSize;
final AtomicBoolean mutating = new AtomicBoolean();
volatile ImmutableOpenMap<K, V> immutableMap;
volatile ConcurrentMap<K, V> concurrentMap;

UpdateInPlaceMap(int switchSize) {
this.switchSize = switchSize;
if (switchSize == 0) {
this.concurrentMap = ConcurrentCollections.newConcurrentMap();
this.immutableMap = null;
} else {
this.concurrentMap = null;
this.immutableMap = ImmutableOpenMap.of();
}
}

/**
* Returns if the map is empty or not.
*/
public boolean isEmpty() {
final ImmutableOpenMap<K, V> immutableMap = this.immutableMap;
final ConcurrentMap<K, V> concurrentMap = this.concurrentMap;
return immutableMap != null ? immutableMap.isEmpty() : concurrentMap.isEmpty();
}

/**
* Returns the value matching a key, or null if not matched.
*/
public V get(K key) {
final ImmutableOpenMap<K, V> immutableMap = this.immutableMap;
final ConcurrentMap<K, V> concurrentMap = this.concurrentMap;
return immutableMap != null ? immutableMap.get(key) : concurrentMap.get(key);
}

/**
* Returns all the values in the map, on going mutator changes might or might not be reflected
* in the values.
*/
public Iterable<V> values() {
return new Iterable<V>() {
@Override
public Iterator<V> iterator() {
final ImmutableOpenMap<K, V> immutableMap = UpdateInPlaceMap.this.immutableMap;
final ConcurrentMap<K, V> concurrentMap = UpdateInPlaceMap.this.concurrentMap;
if (immutableMap != null) {
return immutableMap.valuesIt();
} else {
return Iterables.unmodifiableIterable(concurrentMap.values()).iterator();
}
}
};
}

/**
* Opens a mutator allowing to mutate this map. Note, only one mutator is allowed to execute.
*/
public Mutator mutator() {
if (!mutating.compareAndSet(false, true)) {
throw new ElasticsearchIllegalStateException("map is already mutating, can't have another mutator on it");
}
return new Mutator();
}

public static <K, V> UpdateInPlaceMap<K, V> of(int switchSize) {
return new UpdateInPlaceMap<>(switchSize);
}

public final class Mutator implements Releasable {

private ImmutableOpenMap.Builder<K, V> immutableBuilder;

private Mutator() {
if (immutableMap != null) {
immutableBuilder = ImmutableOpenMap.builder(immutableMap);
} else {
immutableBuilder = null;
}
}

public V get(K key) {
if (immutableBuilder != null) {
return immutableBuilder.get(key);
}
return concurrentMap.get(key);
}

public V put(K key, V value) {
if (immutableBuilder != null) {
V v = immutableBuilder.put(key, value);
switchIfNeeded();
return v;
} else {
return concurrentMap.put(key, value);
}
}

public Mutator putAll(Map<K, V> map) {
for (Map.Entry<K, V> entry : map.entrySet()) {
put(entry.getKey(), entry.getValue());
}
return this;
}

public V remove(K key) {
return immutableBuilder != null ? immutableBuilder.remove(key) : concurrentMap.remove(key);
}

private void switchIfNeeded() {
if (concurrentMap != null) {
assert immutableBuilder == null;
return;
}
if (immutableBuilder.size() <= switchSize) {
return;
}
concurrentMap = ConcurrentCollections.newConcurrentMap();
for (ObjectObjectCursor<K, V> cursor : immutableBuilder) {
concurrentMap.put(cursor.key, cursor.value);
}
immutableBuilder = null;
immutableMap = null;
}

public void close() {
if (immutableBuilder != null) {
immutableMap = immutableBuilder.build();
}
assert (immutableBuilder != null && concurrentMap == null) || (immutableBuilder == null && concurrentMap != null);
mutating.set(false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,22 @@

import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.SimpleAnalyzerWrapper;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.collect.UpdateInPlaceMap;

/**
*
*/
public final class FieldNameAnalyzer extends SimpleAnalyzerWrapper {

private final ImmutableOpenMap<String, Analyzer> analyzers;

private final UpdateInPlaceMap<String, Analyzer> analyzers;
private final Analyzer defaultAnalyzer;

public FieldNameAnalyzer(ImmutableOpenMap<String, Analyzer> analyzers, Analyzer defaultAnalyzer) {
public FieldNameAnalyzer(UpdateInPlaceMap<String, Analyzer> analyzers, Analyzer defaultAnalyzer) {
this.analyzers = analyzers;
this.defaultAnalyzer = defaultAnalyzer;
}

public ImmutableOpenMap<String, Analyzer> analyzers() {
public UpdateInPlaceMap<String, Analyzer> analyzers() {
return analyzers;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.UnmodifiableIterator;
import org.apache.lucene.analysis.Analyzer;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.collect.UpdateInPlaceMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.analysis.FieldNameAnalyzer;
import org.elasticsearch.index.settings.IndexSettings;

import java.util.List;
import java.util.Set;

/**
Expand All @@ -35,48 +40,48 @@ public class DocumentFieldMappers implements Iterable<FieldMapper> {
private final DocumentMapper docMapper;
private final FieldMappersLookup fieldMappers;

private volatile FieldNameAnalyzer indexAnalyzer;
private volatile FieldNameAnalyzer searchAnalyzer;
private volatile FieldNameAnalyzer searchQuoteAnalyzer;
private final FieldNameAnalyzer indexAnalyzer;
private final FieldNameAnalyzer searchAnalyzer;
private final FieldNameAnalyzer searchQuoteAnalyzer;

public DocumentFieldMappers(DocumentMapper docMapper) {
public DocumentFieldMappers(@Nullable @IndexSettings Settings settings, DocumentMapper docMapper) {
this.docMapper = docMapper;
this.fieldMappers = new FieldMappersLookup();
this.indexAnalyzer = new FieldNameAnalyzer(ImmutableOpenMap.<String, Analyzer>of(), docMapper.indexAnalyzer());
this.searchAnalyzer = new FieldNameAnalyzer(ImmutableOpenMap.<String, Analyzer>of(), docMapper.searchAnalyzer());
this.searchQuoteAnalyzer = new FieldNameAnalyzer(ImmutableOpenMap.<String, Analyzer>of(), docMapper.searchQuotedAnalyzer());
this.fieldMappers = new FieldMappersLookup(settings);
this.indexAnalyzer = new FieldNameAnalyzer(UpdateInPlaceMap.<String, Analyzer>of(MapperService.getFieldMappersCollectionSwitch(settings)), docMapper.indexAnalyzer());
this.searchAnalyzer = new FieldNameAnalyzer(UpdateInPlaceMap.<String, Analyzer>of(MapperService.getFieldMappersCollectionSwitch(settings)), docMapper.searchAnalyzer());
this.searchQuoteAnalyzer = new FieldNameAnalyzer(UpdateInPlaceMap.<String, Analyzer>of(MapperService.getFieldMappersCollectionSwitch(settings)), docMapper.searchQuotedAnalyzer());
}

public void addNewMappers(Iterable<FieldMapper> newMappers) {
public void addNewMappers(List<FieldMapper> newMappers) {
fieldMappers.addNewMappers(newMappers);

final ImmutableOpenMap.Builder<String, Analyzer> indexAnalyzers = ImmutableOpenMap.builder(this.indexAnalyzer.analyzers());
final ImmutableOpenMap.Builder<String, Analyzer> searchAnalyzers = ImmutableOpenMap.builder(this.searchAnalyzer.analyzers());
final ImmutableOpenMap.Builder<String, Analyzer> searchQuoteAnalyzers = ImmutableOpenMap.builder(this.searchQuoteAnalyzer.analyzers());
final UpdateInPlaceMap<String, Analyzer>.Mutator indexAnalyzersMutator = this.indexAnalyzer.analyzers().mutator();
final UpdateInPlaceMap<String, Analyzer>.Mutator searchAnalyzersMutator = this.searchAnalyzer.analyzers().mutator();
final UpdateInPlaceMap<String, Analyzer>.Mutator searchQuoteAnalyzersMutator = this.searchQuoteAnalyzer.analyzers().mutator();

for (FieldMapper fieldMapper : newMappers) {
if (fieldMapper.indexAnalyzer() != null) {
indexAnalyzers.put(fieldMapper.names().indexName(), fieldMapper.indexAnalyzer());
indexAnalyzersMutator.put(fieldMapper.names().indexName(), fieldMapper.indexAnalyzer());
}
if (fieldMapper.searchAnalyzer() != null) {
searchAnalyzers.put(fieldMapper.names().indexName(), fieldMapper.searchAnalyzer());
searchAnalyzersMutator.put(fieldMapper.names().indexName(), fieldMapper.searchAnalyzer());
}
if (fieldMapper.searchQuoteAnalyzer() != null) {
searchQuoteAnalyzers.put(fieldMapper.names().indexName(), fieldMapper.searchQuoteAnalyzer());
searchQuoteAnalyzersMutator.put(fieldMapper.names().indexName(), fieldMapper.searchQuoteAnalyzer());
}
}

this.indexAnalyzer = new FieldNameAnalyzer(indexAnalyzers.build(), docMapper.indexAnalyzer());
this.searchAnalyzer = new FieldNameAnalyzer(searchAnalyzers.build(), docMapper.searchAnalyzer());
this.searchQuoteAnalyzer = new FieldNameAnalyzer(searchQuoteAnalyzers.build(), docMapper.searchQuotedAnalyzer());
indexAnalyzersMutator.close();
searchAnalyzersMutator.close();
searchQuoteAnalyzersMutator.close();
}

@Override
public UnmodifiableIterator<FieldMapper> iterator() {
return fieldMappers.iterator();
}

public ImmutableList<FieldMapper> mappers() {
public List<FieldMapper> mappers() {
return this.fieldMappers.mappers();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public DocumentMapper(String index, @Nullable Settings indexSettings, DocumentMa
// now traverse and get all the statically defined ones
rootObjectMapper.traverse(fieldMappersAgg);

this.fieldMappers = new DocumentFieldMappers(this);
this.fieldMappers = new DocumentFieldMappers(indexSettings, this);
this.fieldMappers.addNewMappers(fieldMappersAgg.mappers);

final Map<String, ObjectMapper> objectMappers = Maps.newHashMap();
Expand Down Expand Up @@ -566,7 +566,7 @@ public ParsedDocument parse(SourceToParse source, @Nullable ParseListener listen
return doc;
}

public void addFieldMappers(Iterable<FieldMapper> fieldMappers) {
public void addFieldMappers(List<FieldMapper> fieldMappers) {
synchronized (mappersMutex) {
this.fieldMappers.addNewMappers(fieldMappers);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void fieldMapper(FieldMapper fieldMapper) {

public abstract void fieldMapper(FieldMapper fieldMapper);

public void fieldMappers(Iterable<FieldMapper> fieldMappers) {
public void fieldMappers(List<FieldMapper> fieldMappers) {
for (FieldMapper mapper : fieldMappers) {
fieldMapper(mapper);
}
Expand Down
Loading

0 comments on commit c8e5530

Please sign in to comment.