jainankitk commented on code in PR #14413:
URL: https://github.com/apache/lucene/pull/14413#discussion_r2032043915


##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerBreakdown.java:
##########
@@ -17,46 +17,113 @@
 
 package org.apache.lucene.sandbox.search;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import org.apache.lucene.search.Query;
 import org.apache.lucene.util.CollectionUtil;
 
 /**
  * A record of timings for the various operations that may happen during query 
execution. A node's
  * time may be composed of several internal attributes (rewriting, weighting, 
scoring, etc).
  */
 class QueryProfilerBreakdown {
-
-  /** The accumulated timings for this query node */
-  private final QueryProfilerTimer[] timers;
+  private static final Collection<QueryProfilerTimingType> 
QUERY_LEVEL_TIMING_TYPE =
+      Arrays.stream(QueryProfilerTimingType.values()).filter(t -> 
!t.isSliceLevel()).toList();
+  private final Map<QueryProfilerTimingType, QueryProfilerTimer> 
queryProfilerTimers;
+  private final ConcurrentMap<Long, QuerySliceProfilerBreakdown> 
queryThreadBreakdowns;
 
   /** Sole constructor. */
   public QueryProfilerBreakdown() {
-    timers = new QueryProfilerTimer[QueryProfilerTimingType.values().length];
-    for (int i = 0; i < timers.length; ++i) {
-      timers[i] = new QueryProfilerTimer();
+    queryProfilerTimers = new HashMap<>();

Review Comment:
   Thanks @jpountz. This is a good suggestion, and I missed that while making 
the changes. But when I started using it, I was curious about the 
implementation and saw that it initializes reference for all the Enum values 
during initialization:
   
   ```
   public EnumMap(Class<K> keyType) {
           this.keyType = keyType;
           keyUniverse = getKeyUniverse(keyType);
           vals = new Object[keyUniverse.length];
       }
   ```
   
   Since we are using small subset of `QueryProfilerTimingType`, I am wondering 
if this is a good idea. We can also consider separating 
`QueryProfilerTimingType` on the basis of slice level or query level.



-- 
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: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to