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