wjhypo commented on a change in pull request #11307:
URL: https://github.com/apache/druid/pull/11307#discussion_r752779114



##########
File path: 
processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java
##########
@@ -57,16 +64,42 @@ private static String emptyToNullIfNeeded(@Nullable Object 
o)
     return o != null ? NullHandling.emptyToNullIfNeeded(o.toString()) : null;
   }
 
+  /**
+   * The same idea as {@link 
org.apache.druid.query.aggregation.datasketches.tuple.ArrayOfDoublesSketchBuildBufferAggregator#NUM_STRIPES}
+   *
+   * Locking per bitmap
+   */
+  private static final int NUM_STRIPES = 64;
+
   private final MultiValueHandling multiValueHandling;
   private final boolean hasBitmapIndexes;
   private final boolean hasSpatialIndexes;
   private volatile boolean hasMultipleValues = false;
 
-  public StringDimensionIndexer(MultiValueHandling multiValueHandling, boolean 
hasBitmapIndexes, boolean hasSpatialIndexes)
+  private final boolean enableInMemoryBitmap;
+  // Used only as a read lock to check if modification is needed for 
inMemoryBitmaps
+  private final ReentrantReadWriteLock inMemoryBitmapsReadLock;
+  // Used as a write lock to modify inMemoryBitmaps. Can't use 
inMemoryBitmapsReadLock because it's not easy to upgrade
+  // the same lock after holding a read lock first.
+  private final ReentrantLock inMemoryBitmapsWriteLock;
+  // Per bitmap lock
+  private final Striped<Lock> stripedLock;
+  private final List<MutableBitmap> inMemoryBitmaps;
+  private final BitmapFactory inMemoryBitmapFactory;
+
+  public StringDimensionIndexer(MultiValueHandling multiValueHandling, boolean 
hasBitmapIndexes,
+                                boolean hasSpatialIndexes, boolean 
enableInMemoryBitmap)
   {
     this.multiValueHandling = multiValueHandling == null ? 
MultiValueHandling.ofDefault() : multiValueHandling;
     this.hasBitmapIndexes = hasBitmapIndexes;
     this.hasSpatialIndexes = hasSpatialIndexes;
+
+    this.enableInMemoryBitmap = enableInMemoryBitmap;
+    this.inMemoryBitmaps = new ArrayList<>();
+    this.inMemoryBitmapFactory = new RoaringBitmapFactory();
+    this.inMemoryBitmapsReadLock = new ReentrantReadWriteLock();
+    this.inMemoryBitmapsWriteLock = new ReentrantLock();
+    this.stripedLock = Striped.lock(NUM_STRIPES);

Review comment:
       Good catch! I'll avoid the initialization behind a flag.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to