[incubator-druid] branch master updated: Cache selectors in QueryableIndexColumnSelectorFactory. (#7216)

[fj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

fjy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git


The following commit(s) were added to refs/heads/master by this push:
     new 4290e5a  Cache selectors in QueryableIndexColumnSelectorFactory. (#7216)
4290e5a is described below

commit 4290e5ae7a9be0cfff68bddae0501602c5ed1c73
Author: Gian Merlino <gi...@gmail.com>
AuthorDate: Mon Mar 11 11:33:02 2019 -0700

    Cache selectors in QueryableIndexColumnSelectorFactory. (#7216)
    
    For selectors with internal caches (like SingleScanTimeDimensionSelector,
    SingleLongInputCachingExpressionColumnValueSelector, etc) we can get a perf
    boost and memory usage decrease by sharing selectors.
---
 .../QueryableIndexColumnSelectorFactory.java       | 93 +++++++++++++---------
 1 file changed, 57 insertions(+), 36 deletions(-)

diff --git a/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java b/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java
index c0b5ee5..ec5ee1d 100644
--- a/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java
+++ b/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java
@@ -30,6 +30,7 @@ import org.apache.druid.segment.column.ValueType;
 import org.apache.druid.segment.data.ReadableOffset;
 
 import javax.annotation.Nullable;
+import java.util.HashMap;
 import java.util.Map;
 
 /**
@@ -45,8 +46,14 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
   private final Closer closer;
   protected final ReadableOffset offset;
 
+  // Share Column objects, since they cache decompressed buffers internally, and we can avoid recomputation if the
+  // same column is used by more than one part of a query.
   private final Map<String, BaseColumn> columnCache;
 
+  // Share selectors too, for the same reason that we cache columns (they may cache things internally).
+  private final Map<DimensionSpec, DimensionSelector> dimensionSelectorCache;
+  private final Map<String, ColumnValueSelector> valueSelectorCache;
+
   QueryableIndexColumnSelectorFactory(
       QueryableIndex index,
       VirtualColumns virtualColumns,
@@ -62,16 +69,23 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
     this.closer = closer;
     this.offset = offset;
     this.columnCache = columnCache;
+    this.dimensionSelectorCache = new HashMap<>();
+    this.valueSelectorCache = new HashMap<>();
   }
 
   @Override
   public DimensionSelector makeDimensionSelector(DimensionSpec dimensionSpec)
   {
-    if (virtualColumns.exists(dimensionSpec.getDimension())) {
-      return virtualColumns.makeDimensionSelector(dimensionSpec, this);
-    }
-
-    return dimensionSpec.decorate(makeDimensionSelectorUndecorated(dimensionSpec));
+    return dimensionSelectorCache.computeIfAbsent(
+        dimensionSpec,
+        spec -> {
+          if (virtualColumns.exists(spec.getDimension())) {
+            return virtualColumns.makeDimensionSelector(spec, this);
+          }
+
+          return spec.decorate(makeDimensionSelectorUndecorated(spec));
+        }
+    );
   }
 
   private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensionSpec)
@@ -93,20 +107,10 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
       return type.makeNumericWrappingDimensionSelector(makeColumnValueSelector(dimension), extractionFn);
     }
 
-    BaseColumn column = columnCache.computeIfAbsent(dimension, d -> {
-      BaseColumn col = columnHolder.getColumn();
-      if (col instanceof DictionaryEncodedColumn) {
-        return closer.register(col);
-      } else {
-        // Return null from the lambda in computeIfAbsent() results in no recorded value in the columnCache and
-        // the column variable is set to null.
-        return null;
-      }
-    });
-
-    if (column instanceof DictionaryEncodedColumn) {
-      //noinspection unchecked
-      return ((DictionaryEncodedColumn<String>) column).makeDimensionSelector(offset, extractionFn);
+    final DictionaryEncodedColumn column = getCachedColumn(dimension, DictionaryEncodedColumn.class);
+
+    if (column != null) {
+      return column.makeDimensionSelector(offset, extractionFn);
     } else {
       return DimensionSelector.constant(null, extractionFn);
     }
@@ -115,24 +119,41 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
   @Override
   public ColumnValueSelector<?> makeColumnValueSelector(String columnName)
   {
-    if (virtualColumns.exists(columnName)) {
-      return virtualColumns.makeColumnValueSelector(columnName, this);
-    }
-
-    BaseColumn column = columnCache.computeIfAbsent(columnName, name -> {
-      ColumnHolder holder = index.getColumnHolder(name);
-      if (holder != null) {
-        return closer.register(holder.getColumn());
-      } else {
-        return null;
-      }
-    });
+    return valueSelectorCache.computeIfAbsent(
+        columnName,
+        name -> {
+          if (virtualColumns.exists(columnName)) {
+            return virtualColumns.makeColumnValueSelector(columnName, this);
+          }
+
+          BaseColumn column = getCachedColumn(columnName, BaseColumn.class);
+
+          if (column != null) {
+            return column.makeColumnValueSelector(offset);
+          } else {
+            return NilColumnValueSelector.instance();
+          }
+        }
+    );
+  }
 
-    if (column != null) {
-      return column.makeColumnValueSelector(offset);
-    } else {
-      return NilColumnValueSelector.instance();
-    }
+  @Nullable
+  @SuppressWarnings("unchecked")
+  private <T extends BaseColumn> T getCachedColumn(final String columnName, final Class<T> clazz)
+  {
+    return (T) columnCache.computeIfAbsent(
+        columnName,
+        name -> {
+          ColumnHolder holder = index.getColumnHolder(name);
+          if (holder != null && clazz.isAssignableFrom(holder.getColumn().getClass())) {
+            return closer.register(holder.getColumn());
+          } else {
+            // Return null from the lambda in computeIfAbsent() results in no recorded value in the columnCache and
+            // the column variable is set to null.
+            return null;
+          }
+        }
+    );
   }
 
   @Override


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org