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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
