You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/05/18 18:15:10 UTC

[GitHub] [lucene] jdconrad opened a new pull request #144: LUCENE-9965: Add tooling to introspect query execution time

jdconrad opened a new pull request #144:
URL: https://github.com/apache/lucene/pull/144


   # Description
   
   Based on the discussion from [this email thread](https://lists.apache.org/thread.html/r7957a2d9ca38af45b1c370753b3c10542fd9faaf9bf95944c5224e12%40%3Cdev.lucene.apache.org%3E) we could add a set of classes to compile timings for different pieces of a query or multiple queries. This could be used to better debug changes in performance moving forward.
   
   # Solution
   
   This change adds a multitude of new classes to help profile query timings. These classes have all been added to the Lucene sandbox including a simple extension of a IndexSearcher with the name ProfileIndexSearcher that includes a QueryProfiler. The QueryProfiler includes the total time spent in each of the following categories along with the number of times visited:
   
   * create weight
   * build scorer
   * next doc
   * advance
   * score
   * match
   
   # Tests
   
   Tests have been added to ensure that a simple query generate values within a ProfileIndexSearcher.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/lucene/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [x] I have created a Jira issue and added the issue ID to my pull request title.
   - [x] I have given Lucene maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [x] I have developed this patch against the `main` branch.
   - [x] I have run `./gradlew check`.
   - [x] I have added tests for my changes.
   


-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636391311



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractInternalProfileTree.java
##########
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;

Review comment:
       Moved.




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636263687



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfileBreakdown.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static java.util.Collections.emptyMap;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * 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).
+ */
+public abstract class AbstractProfileBreakdown<T extends Enum<T>> {
+
+  /** The accumulated timings for this query node */
+  private final ProfileTimer[] timings;
+
+  private final T[] timingTypes;
+
+  /** Sole constructor. */
+  public AbstractProfileBreakdown(Class<T> clazz) {
+    this.timingTypes = clazz.getEnumConstants();
+    timings = new ProfileTimer[timingTypes.length];
+    for (int i = 0; i < timings.length; ++i) {
+      timings[i] = new ProfileTimer();
+    }
+  }
+
+  public ProfileTimer getTimer(T timing) {
+    return timings[timing.ordinal()];
+  }
+
+  public void setTimer(T timing, ProfileTimer timer) {
+    timings[timing.ordinal()] = timer;
+  }
+
+  /** Build a timing count breakdown. */
+  public final Map<String, Long> toBreakdownMap() {
+    Map<String, Long> map = new HashMap<>(timings.length * 2);
+    for (T timingType : timingTypes) {
+      map.put(timingType.toString(), timings[timingType.ordinal()].getApproximateTiming());
+      map.put(timingType.toString() + "_count", timings[timingType.ordinal()].getCount());
+    }
+    return Collections.unmodifiableMap(map);
+  }
+
+  /** Fetch extra debugging information. */
+  protected Map<String, Object> toDebugMap() {
+    return emptyMap();
+  }

Review comment:
       Removed.




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on pull request #144:
URL: https://github.com/apache/lucene/pull/144#issuecomment-845452957


   Just a note that these comments are still valid and not really outdated. I changed packages and renamed some of the files so it's no longer referencing the correct path.


-- 
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.

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



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


[GitHub] [lucene] jtibshirani commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r639181752



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileIndexSearcher.java
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.io.IOException;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Weight;
+
+/**
+ * A simple extension of {@link IndexSearcher} to add a {@link QueryProfiler} that can be set to
+ * test query timings.
+ */
+public class ProfileIndexSearcher extends IndexSearcher {
+
+  private QueryProfiler profiler;
+
+  public ProfileIndexSearcher(IndexReader reader) {
+    super(reader);
+  }
+
+  public void setProfiler(QueryProfiler profiler) {

Review comment:
       I don't have a strong intuition here -- maybe @jpountz has an opinion?




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636264078



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfileBreakdown.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static java.util.Collections.emptyMap;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * 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).
+ */
+public abstract class AbstractProfileBreakdown<T extends Enum<T>> {
+
+  /** The accumulated timings for this query node */
+  private final ProfileTimer[] timings;
+
+  private final T[] timingTypes;
+
+  /** Sole constructor. */
+  public AbstractProfileBreakdown(Class<T> clazz) {
+    this.timingTypes = clazz.getEnumConstants();
+    timings = new ProfileTimer[timingTypes.length];
+    for (int i = 0; i < timings.length; ++i) {
+      timings[i] = new ProfileTimer();
+    }
+  }
+
+  public ProfileTimer getTimer(T timing) {
+    return timings[timing.ordinal()];
+  }
+
+  public void setTimer(T timing, ProfileTimer timer) {
+    timings[timing.ordinal()] = timer;
+  }
+
+  /** Build a timing count breakdown. */
+  public final Map<String, Long> toBreakdownMap() {
+    Map<String, Long> map = new HashMap<>(timings.length * 2);
+    for (T timingType : timingTypes) {
+      map.put(timingType.toString(), timings[timingType.ordinal()].getApproximateTiming());
+      map.put(timingType.toString() + "_count", timings[timingType.ordinal()].getCount());
+    }
+    return Collections.unmodifiableMap(map);
+  }
+
+  /** Fetch extra debugging information. */
+  protected Map<String, Object> toDebugMap() {
+    return emptyMap();
+  }
+
+  public final long toNodeTime() {

Review comment:
       Changed.




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636392637



##########
File path: lucene/sandbox/src/test/org/apache/lucene/sandbox/queries/profile/TestProfileQuery.java
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field.Store;
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.RandomIndexWriter;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.LRUQueryCache;
+import org.apache.lucene.search.LeafCollector;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.RandomApproximationQuery;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.TotalHitCountCollector;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.TestUtil;
+import org.hamcrest.MatcherAssert;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+public class TestProfileQuery extends LuceneTestCase {

Review comment:
       Renamed.




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636392294



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractInternalProfileTree.java
##########
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.List;
+
+/**
+ * This class tracks a Query tree for profiling. This class can be extended to allow different
+ * element types for timing with the tree.
+ */
+public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown<?>, E> {

Review comment:
       Done. I was thinking it may make sense to preserve the original, but that wouldn't make sense for Lucene.

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfiler.java
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.List;
+
+/**
+ * This class acts as as storage for a profile tree. This class may be extended to define how the
+ * profile contains and how it's broken into different pieces.
+ */
+public class AbstractProfiler<PB extends AbstractProfileBreakdown<?>, E> {

Review comment:
       Done. I was thinking it may make sense to preserve the original, but that wouldn't make sense for Lucene.




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636262712



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileResult.java
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * This class is the internal representation of a profiled Query, corresponding to a single node in
+ * the query tree. It is built after the query has finished executing and is merely a structured
+ * representation, rather than the entity that collects the timing profile (see InternalProfiler for
+ * that)
+ *
+ * <p>Each InternalProfileResult has a List of InternalProfileResults, which will contain "children"
+ * queries if applicable
+ */
+public final class ProfileResult {
+
+  private final String type;
+  private final String description;
+  private final Map<String, Long> breakdown;
+  private final Map<String, Object> debug;
+  private final long nodeTime;
+  private final List<ProfileResult> children;
+
+  public ProfileResult(
+      String type,
+      String description,
+      Map<String, Long> breakdown,
+      Map<String, Object> debug,
+      long nodeTime,
+      List<ProfileResult> children) {
+    this.type = type;
+    this.description = description;
+    this.breakdown = Objects.requireNonNull(breakdown, "required breakdown argument missing");
+    this.debug = debug == null ? Collections.emptyMap() : debug;
+    this.children = children == null ? Collections.emptyList() : children;
+    this.nodeTime = nodeTime;
+  }
+
+  /** Retrieve the lucene description of this query (e.g. the "explain" text) */
+  public String getLuceneDescription() {

Review comment:
       Changed.




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on pull request #144:
URL: https://github.com/apache/lucene/pull/144#issuecomment-845152558


   @jpountz @jtibshirani Thank you for the feedback! I will address these soon.


-- 
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.

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



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


[GitHub] [lucene] jpountz commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r648031093



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfiler.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.lucene.search.Query;
+
+/**
+ * This class acts as storage for profiling a query. It also builds a representation of the query
+ * tree which is built constructed "online" as the weights are wrapped by {@link
+ * QueryProfilerIndexSearcher}. This allows us to know the relationship between nodes in tree
+ * without explicitly walking the tree or pre-wrapping everything.
+ */
+public class QueryProfiler {

Review comment:
       nit
   ```suggestion
   public final class QueryProfiler {
   ```

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerCollector.java
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.io.IOException;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.Collector;
+import org.apache.lucene.search.FilterCollector;
+import org.apache.lucene.search.FilterLeafCollector;
+import org.apache.lucene.search.LeafCollector;
+import org.apache.lucene.search.Scorable;
+import org.apache.lucene.search.ScoreMode;
+
+/** A collector that profiles how much time is spent calling it. */
+public class QueryProfilerCollector extends FilterCollector {

Review comment:
       make pkg-private?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerBreakdown.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * 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).
+ */
+public class QueryProfilerBreakdown {

Review comment:
       This looks like an internal class that could be made pkg-private (and final)?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerCollectorResult.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.util.List;
+
+/**
+ * Public class for profiled timings of the Collectors used in the search. Children
+ * CollectorResult's may be embedded inside of a parent CollectorResult
+ */
+public class QueryProfilerCollectorResult {
+
+  /** A more friendly representation of the Collector's class name */
+  protected final String collectorName;
+
+  /** A "hint" to help provide some context about this Collector */
+  protected final String reason;
+
+  /** The total elapsed time for this Collector */
+  protected final Long time;
+
+  /** A list of children collectors "embedded" inside this collector */
+  protected List<QueryProfilerCollectorResult> children;
+
+  public QueryProfilerCollectorResult(
+      String collectorName, String reason, Long time, List<QueryProfilerCollectorResult> children) {

Review comment:
       use `long` instead of `Long`?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerCollectorWrapper.java
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.Collector;
+import org.apache.lucene.search.LeafCollector;
+import org.apache.lucene.search.ScoreMode;
+
+/**
+ * This class wraps a Collector and times the execution of: - setScorer() - collect() -
+ * doSetNextReader() - needsScores()
+ *
+ * <p>QueryProfiler facilitates the linking of the Collector graph
+ */
+public class QueryProfilerCollectorWrapper implements Collector {

Review comment:
       this one looks like it could be pkg-private too?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerScorer.java
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.io.IOException;
+import java.util.Collection;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.TwoPhaseIterator;
+import org.apache.lucene.search.Weight;
+
+/**
+ * {@link Scorer} wrapper that will compute how much time is spent on moving the iterator,
+ * confirming matches and computing scores.
+ */
+public class QueryProfilerScorer extends Scorer {

Review comment:
       can it be pkg-private?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerCollectorResult.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.util.List;
+
+/**
+ * Public class for profiled timings of the Collectors used in the search. Children
+ * CollectorResult's may be embedded inside of a parent CollectorResult
+ */
+public class QueryProfilerCollectorResult {
+
+  /** A more friendly representation of the Collector's class name */
+  protected final String collectorName;
+
+  /** A "hint" to help provide some context about this Collector */
+  protected final String reason;
+
+  /** The total elapsed time for this Collector */
+  protected final Long time;

Review comment:
       make it a native long?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerWeight.java
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.io.IOException;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.BulkScorer;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.search.ScorerSupplier;
+import org.apache.lucene.search.Weight;
+
+/**
+ * Weight wrapper that will compute how much time it takes to build the {@link Scorer} and then
+ * return a {@link Scorer} that is wrapped in order to compute timings as well.
+ */
+public class QueryProfilerWeight extends Weight {

Review comment:
       make it pkg-private?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerTimer.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+/**
+ * Helps measure how much time is spent running some methods. The {@link #start()} and {@link
+ * #stop()} methods should typically be called in a try/finally clause with {@link #start()} being
+ * called right before the try block and {@link #stop()} being called at the beginning of the
+ * finally block:
+ *
+ * <pre>
+ *  timer.start();
+ *  try {
+ *    // code to time
+ *  } finally {
+ *    timer.stop();
+ *  }
+ *  </pre>
+ */
+public class QueryProfilerTimer {

Review comment:
       can it be pkg-private?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfilerResult.java
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * This class is the internal representation of a profiled Query, corresponding to a single node in
+ * the query tree. It is built after the query has finished executing and is merely a structured
+ * representation, rather than the entity that collects the timing profile (see InternalProfiler for
+ * that)
+ *
+ * <p>Each InternalProfileResult has a List of InternalProfileResults, which will contain "children"

Review comment:
       Update to the new names of these classes?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/search/QueryProfiler.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.search;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.lucene.search.Query;
+
+/**
+ * This class acts as storage for profiling a query. It also builds a representation of the query
+ * tree which is built constructed "online" as the weights are wrapped by {@link
+ * QueryProfilerIndexSearcher}. This allows us to know the relationship between nodes in tree
+ * without explicitly walking the tree or pre-wrapping everything.
+ */
+public class QueryProfiler {

Review comment:
       Also I believe that all methods but `getTree` and `getCollector` are only for internal usage, should we make them pkg-private?




-- 
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.

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



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


[GitHub] [lucene] jtibshirani commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r635628013



##########
File path: lucene/sandbox/src/test/org/apache/lucene/sandbox/queries/profile/TestProfileQuery.java
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field.Store;
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.RandomIndexWriter;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.LRUQueryCache;
+import org.apache.lucene.search.LeafCollector;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.RandomApproximationQuery;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.TotalHitCountCollector;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.TestUtil;
+import org.hamcrest.MatcherAssert;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+public class TestProfileQuery extends LuceneTestCase {

Review comment:
       Small comment, `TestProfileIndexSearcher` could be a clearer name.

##########
File path: lucene/sandbox/src/test/org/apache/lucene/sandbox/queries/profile/TestProfileQuery.java
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field.Store;
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.RandomIndexWriter;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.LRUQueryCache;
+import org.apache.lucene.search.LeafCollector;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.RandomApproximationQuery;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.TotalHitCountCollector;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.IOUtils;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.TestUtil;
+import org.hamcrest.MatcherAssert;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+public class TestProfileQuery extends LuceneTestCase {
+
+  private static Directory dir;
+  private static IndexReader reader;
+  private static ProfileIndexSearcher searcher;
+
+  @BeforeClass
+  public static void setup() throws IOException {
+    dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    final int numDocs = TestUtil.nextInt(random(), 1, 20);
+    for (int i = 0; i < numDocs; ++i) {
+      final int numHoles = random().nextInt(5);
+      for (int j = 0; j < numHoles; ++j) {
+        w.addDocument(new Document());
+      }
+      Document doc = new Document();
+      doc.add(new StringField("foo", "bar", Store.NO));
+      w.addDocument(doc);
+    }
+    reader = w.getReader();
+    w.close();
+    searcher = new ProfileIndexSearcher(reader);
+  }
+
+  @After
+  public void checkNoCache() {
+    LRUQueryCache cache = (LRUQueryCache) searcher.getQueryCache();
+    MatcherAssert.assertThat(cache.getHitCount(), equalTo(0L));
+    MatcherAssert.assertThat(cache.getCacheCount(), equalTo(0L));
+    MatcherAssert.assertThat(cache.getTotalCount(), equalTo(cache.getMissCount()));
+    MatcherAssert.assertThat(cache.getCacheSize(), equalTo(0L));
+  }
+
+  @AfterClass
+  public static void cleanup() throws IOException {
+    IOUtils.close(reader, dir);
+    dir = null;
+    reader = null;
+    searcher = null;
+  }
+
+  public void testBasic() throws IOException {
+    QueryProfiler profiler = new QueryProfiler();
+    searcher.setProfiler(profiler);
+    Query query = new TermQuery(new Term("foo", "bar"));
+    searcher.search(query, 1);
+    List<ProfileResult> results = profiler.getTree();
+    assertEquals(1, results.size());
+    Map<String, Long> breakdown = results.get(0).getTimeBreakdown();
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.CREATE_WEIGHT.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.BUILD_SCORER.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.NEXT_DOC.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.ADVANCE.toString()), equalTo(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.SCORE.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.MATCH.toString()), equalTo(0L));
+
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.CREATE_WEIGHT.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.BUILD_SCORER.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.NEXT_DOC.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.ADVANCE.toString() + "_count"), equalTo(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.SCORE.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.MATCH.toString() + "_count"), equalTo(0L));
+
+    long rewriteTime = profiler.getRewriteTime();
+    MatcherAssert.assertThat(rewriteTime, greaterThan(0L));
+  }
+
+  public void testNoScoring() throws IOException {
+    QueryProfiler profiler = new QueryProfiler();
+    searcher.setProfiler(profiler);
+    Query query = new TermQuery(new Term("foo", "bar"));
+    searcher.search(query, 1, Sort.INDEXORDER); // scores are not needed
+    List<ProfileResult> results = profiler.getTree();
+    assertEquals(1, results.size());
+    Map<String, Long> breakdown = results.get(0).getTimeBreakdown();
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.CREATE_WEIGHT.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.BUILD_SCORER.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.NEXT_DOC.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.ADVANCE.toString()), equalTo(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.SCORE.toString()), equalTo(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.MATCH.toString()), equalTo(0L));
+
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.CREATE_WEIGHT.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.BUILD_SCORER.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.NEXT_DOC.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.ADVANCE.toString() + "_count"), equalTo(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.SCORE.toString() + "_count"), equalTo(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.MATCH.toString() + "_count"), equalTo(0L));
+
+    long rewriteTime = profiler.getRewriteTime();
+    MatcherAssert.assertThat(rewriteTime, greaterThan(0L));
+  }
+
+  public void testUseIndexStats() throws IOException {
+    QueryProfiler profiler = new QueryProfiler();
+    searcher.setProfiler(profiler);
+    Query query = new TermQuery(new Term("foo", "bar"));
+    searcher.count(query); // will use index stats
+    List<ProfileResult> results = profiler.getTree();
+    assertEquals(0, results.size());
+
+    long rewriteTime = profiler.getRewriteTime();
+    MatcherAssert.assertThat(rewriteTime, greaterThan(0L));
+  }
+
+  public void testApproximations() throws IOException {
+    QueryProfiler profiler = new QueryProfiler();
+    searcher.setProfiler(profiler);
+    Query query = new RandomApproximationQuery(new TermQuery(new Term("foo", "bar")), random());
+    searcher.count(query);
+    List<ProfileResult> results = profiler.getTree();
+    assertEquals(1, results.size());
+    Map<String, Long> breakdown = results.get(0).getTimeBreakdown();
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.CREATE_WEIGHT.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.BUILD_SCORER.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.NEXT_DOC.toString()), greaterThan(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.ADVANCE.toString()), equalTo(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.SCORE.toString()), equalTo(0L));
+    MatcherAssert.assertThat(breakdown.get(QueryTimingType.MATCH.toString()), greaterThan(0L));
+
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.CREATE_WEIGHT.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.BUILD_SCORER.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.NEXT_DOC.toString() + "_count"), greaterThan(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.ADVANCE.toString() + "_count"), equalTo(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.SCORE.toString() + "_count"), equalTo(0L));
+    MatcherAssert.assertThat(
+        breakdown.get(QueryTimingType.MATCH.toString() + "_count"), greaterThan(0L));
+
+    long rewriteTime = profiler.getRewriteTime();
+    MatcherAssert.assertThat(rewriteTime, greaterThan(0L));
+  }
+
+  public void testCollector() throws IOException {

Review comment:
       It'd be nice to have one test showing how the collector would actually be used in a search, through something like `IndexSearcher#search(Query query, Collector results)`.

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileIndexSearcher.java
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.io.IOException;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Weight;
+
+/**
+ * A simple extension of {@link IndexSearcher} to add a {@link QueryProfiler} that can be set to
+ * test query timings.
+ */
+public class ProfileIndexSearcher extends IndexSearcher {
+
+  private QueryProfiler profiler;
+
+  public ProfileIndexSearcher(IndexReader reader) {
+    super(reader);
+  }
+
+  public void setProfiler(QueryProfiler profiler) {

Review comment:
       Maybe we could add `QueryProfiler` as a constructor parameter and ensure it's always non-null. That'd simplify this class a bit, and make it clear that it's always meant to be used for profiling.
   
   We could even take this further and remove the `QueryProfiler` class, folding its logic into `ProfileIndexSearcher`. It doesn't seem like a helpful abstraction on its own?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractInternalProfileTree.java
##########
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.List;
+
+/**
+ * This class tracks a Query tree for profiling. This class can be extended to allow different
+ * element types for timing with the tree.
+ */
+public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown<?>, E> {

Review comment:
       +1 to simplifying the class hierarchy as much as possible. That may mean Elasticsearch can't reuse these classes instead of the original ones, but that seems okay! We could also remove the name `Internal` from these classes to simplify (so we just have `QueryProfileTree` and `ProfileCollector`).




-- 
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.

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



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


[GitHub] [lucene] jdconrad commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jdconrad commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r636442384



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileIndexSearcher.java
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.io.IOException;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Weight;
+
+/**
+ * A simple extension of {@link IndexSearcher} to add a {@link QueryProfiler} that can be set to
+ * test query timings.
+ */
+public class ProfileIndexSearcher extends IndexSearcher {
+
+  private QueryProfiler profiler;
+
+  public ProfileIndexSearcher(IndexReader reader) {
+    super(reader);
+  }
+
+  public void setProfiler(QueryProfiler profiler) {

Review comment:
       I like this idea a lot as fewer classes in this case is ideal. My question here is do want to have to create a new IndexSearcher for each query?




-- 
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.

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



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


[GitHub] [lucene] jtibshirani merged pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jtibshirani merged pull request #144:
URL: https://github.com/apache/lucene/pull/144


   


-- 
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.

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



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


[GitHub] [lucene] jtibshirani commented on pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #144:
URL: https://github.com/apache/lucene/pull/144#issuecomment-857916002


   @jdconrad asked that I finish off this work as he'll be away for several weeks. I pushed a few changes:
   * Adjust class visibility and other small clean-ups
   * Fold QueryProfiler into QueryProfilerIndexSearcher
   * Rename profiler collectors and create dedicated test
   
   I'll plan to merge soon unless there are more comments.


-- 
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.

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



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


[GitHub] [lucene] jpountz commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r635308121



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractInternalProfileTree.java
##########
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;

Review comment:
       We use the `queries` package for Query implementations in other modules, but this functionality is more something that introspects query execution, so I'd rather put it in a `search` package, e.g.
   ```suggestion
   package org.apache.lucene.sandbox.search;
   ```

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfiler.java
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.List;
+
+/**
+ * This class acts as as storage for a profile tree. This class may be extended to define how the
+ * profile contains and how it's broken into different pieces.
+ */
+public class AbstractProfiler<PB extends AbstractProfileBreakdown<?>, E> {

Review comment:
       likewise here, this class has a single implementation so let's merge it with its only implementation?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileIndexSearcher.java
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.io.IOException;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Weight;
+
+/**
+ * A simple extension of {@link IndexSearcher} to add a {@link QueryProfiler} that can be set to
+ * test query timings.
+ */

Review comment:
       Add an example of how it may be used in the javadocs?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractInternalProfileTree.java
##########
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.List;
+
+/**
+ * This class tracks a Query tree for profiling. This class can be extended to allow different
+ * element types for timing with the tree.
+ */
+public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown<?>, E> {

Review comment:
       Since this class has a single implementation, can we merge it into its only implementation and avoid the generics?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfileBreakdown.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static java.util.Collections.emptyMap;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * 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).
+ */
+public abstract class AbstractProfileBreakdown<T extends Enum<T>> {
+
+  /** The accumulated timings for this query node */
+  private final ProfileTimer[] timings;
+
+  private final T[] timingTypes;
+
+  /** Sole constructor. */
+  public AbstractProfileBreakdown(Class<T> clazz) {
+    this.timingTypes = clazz.getEnumConstants();
+    timings = new ProfileTimer[timingTypes.length];
+    for (int i = 0; i < timings.length; ++i) {
+      timings[i] = new ProfileTimer();
+    }
+  }
+
+  public ProfileTimer getTimer(T timing) {
+    return timings[timing.ordinal()];
+  }
+
+  public void setTimer(T timing, ProfileTimer timer) {
+    timings[timing.ordinal()] = timer;
+  }
+
+  /** Build a timing count breakdown. */
+  public final Map<String, Long> toBreakdownMap() {
+    Map<String, Long> map = new HashMap<>(timings.length * 2);
+    for (T timingType : timingTypes) {
+      map.put(timingType.toString(), timings[timingType.ordinal()].getApproximateTiming());
+      map.put(timingType.toString() + "_count", timings[timingType.ordinal()].getCount());
+    }
+    return Collections.unmodifiableMap(map);
+  }
+
+  /** Fetch extra debugging information. */
+  protected Map<String, Object> toDebugMap() {
+    return emptyMap();
+  }
+
+  public final long toNodeTime() {

Review comment:
       maybe rename `totalTime` since nodes don't make much sense in the context of Lucene?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfileBreakdown.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static java.util.Collections.emptyMap;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * 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).
+ */
+public abstract class AbstractProfileBreakdown<T extends Enum<T>> {

Review comment:
       maybe add a useful `toString()` method so that users could more easily print profiling info of their queries?

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileResult.java
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * This class is the internal representation of a profiled Query, corresponding to a single node in
+ * the query tree. It is built after the query has finished executing and is merely a structured
+ * representation, rather than the entity that collects the timing profile (see InternalProfiler for
+ * that)
+ *
+ * <p>Each InternalProfileResult has a List of InternalProfileResults, which will contain "children"
+ * queries if applicable
+ */
+public final class ProfileResult {
+
+  private final String type;
+  private final String description;
+  private final Map<String, Long> breakdown;
+  private final Map<String, Object> debug;
+  private final long nodeTime;
+  private final List<ProfileResult> children;
+
+  public ProfileResult(
+      String type,
+      String description,
+      Map<String, Long> breakdown,
+      Map<String, Object> debug,
+      long nodeTime,
+      List<ProfileResult> children) {
+    this.type = type;
+    this.description = description;
+    this.breakdown = Objects.requireNonNull(breakdown, "required breakdown argument missing");
+    this.debug = debug == null ? Collections.emptyMap() : debug;
+    this.children = children == null ? Collections.emptyList() : children;
+    this.nodeTime = nodeTime;
+  }
+
+  /** Retrieve the lucene description of this query (e.g. the "explain" text) */
+  public String getLuceneDescription() {

Review comment:
       ```suggestion
     public String getDescription() {
   ```

##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/AbstractProfileBreakdown.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import static java.util.Collections.emptyMap;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * 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).
+ */
+public abstract class AbstractProfileBreakdown<T extends Enum<T>> {
+
+  /** The accumulated timings for this query node */
+  private final ProfileTimer[] timings;
+
+  private final T[] timingTypes;
+
+  /** Sole constructor. */
+  public AbstractProfileBreakdown(Class<T> clazz) {
+    this.timingTypes = clazz.getEnumConstants();
+    timings = new ProfileTimer[timingTypes.length];
+    for (int i = 0; i < timings.length; ++i) {
+      timings[i] = new ProfileTimer();
+    }
+  }
+
+  public ProfileTimer getTimer(T timing) {
+    return timings[timing.ordinal()];
+  }
+
+  public void setTimer(T timing, ProfileTimer timer) {
+    timings[timing.ordinal()] = timer;
+  }
+
+  /** Build a timing count breakdown. */
+  public final Map<String, Long> toBreakdownMap() {
+    Map<String, Long> map = new HashMap<>(timings.length * 2);
+    for (T timingType : timingTypes) {
+      map.put(timingType.toString(), timings[timingType.ordinal()].getApproximateTiming());
+      map.put(timingType.toString() + "_count", timings[timingType.ordinal()].getCount());
+    }
+    return Collections.unmodifiableMap(map);
+  }
+
+  /** Fetch extra debugging information. */
+  protected Map<String, Object> toDebugMap() {
+    return emptyMap();
+  }

Review comment:
       this doesn't seem used?




-- 
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.

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



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


[GitHub] [lucene] jpountz commented on a change in pull request #144: LUCENE-9965: Add tooling to introspect query execution time

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #144:
URL: https://github.com/apache/lucene/pull/144#discussion_r648030421



##########
File path: lucene/sandbox/src/java/org/apache/lucene/sandbox/queries/profile/ProfileIndexSearcher.java
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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.apache.lucene.sandbox.queries.profile;
+
+import java.io.IOException;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.Weight;
+
+/**
+ * A simple extension of {@link IndexSearcher} to add a {@link QueryProfiler} that can be set to
+ * test query timings.
+ */
+public class ProfileIndexSearcher extends IndexSearcher {
+
+  private QueryProfiler profiler;
+
+  public ProfileIndexSearcher(IndexReader reader) {
+    super(reader);
+  }
+
+  public void setProfiler(QueryProfiler profiler) {

Review comment:
       Yes, it's totally fine. IndexReaders are costly to create, but IndexSearchers are very cheap.




-- 
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.

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



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