dsmiley commented on code in PR #3739: URL: https://github.com/apache/solr/pull/3739#discussion_r2412470588
########## solr/test-framework/src/java/org/apache/solr/util/CallerMatcher.java: ########## @@ -0,0 +1,257 @@ +/* + * 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.solr.util; + +import java.lang.invoke.MethodHandles; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Helper class to collect interesting callers at specific points. These calling points are + * identified by the calling class' simple name and optionally a method name and the optional + * maximum count, e.g. <code>MoreLikeThisComponent + * </code> or <code> + * ClusteringComponent.finishStage</code>, <code>ClusteringComponent.finishStage:100</code>. A Review Comment: newline issues? ########## solr/benchmark/src/java/org/apache/solr/bench/search/ExitableDirectoryReaderSearch.java: ########## @@ -0,0 +1,180 @@ +package org.apache.solr.bench.search; + +import static org.apache.solr.bench.generators.SourceDSL.integers; +import static org.apache.solr.bench.generators.SourceDSL.strings; + +import java.util.Map; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import org.apache.solr.bench.Docs; +import org.apache.solr.bench.MiniClusterState; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.search.CallerSpecificQueryLimit; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.TestInjection; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +@Fork(value = 1) +@BenchmarkMode(Mode.AverageTime) +@Warmup(time = 20, iterations = 2) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@Measurement(time = 30, iterations = 4) +@Threads(value = 1) +public class ExitableDirectoryReaderSearch { + + static final String COLLECTION = "c1"; + + @State(Scope.Benchmark) + public static class BenchState { + + Docs queryFields; + + int NUM_DOCS = 500_000; + int WORDS = NUM_DOCS / 100; + + @Setup(Level.Trial) + public void setupTrial(MiniClusterState.MiniClusterBenchState miniClusterState) + throws Exception { + miniClusterState.setUseHttp1(true); + System.setProperty("documentCache.enabled", "false"); + System.setProperty("queryResultCache.enabled", "false"); + System.setProperty("filterCache.enabled", "false"); + System.setProperty("miniClusterBaseDir", "build/work/mini-cluster"); + // create a lot of small segments + System.setProperty("segmentsPerTier", "200"); Review Comment: Why are so many segments desired here? ########## solr/test-framework/src/java/org/apache/solr/util/CallerMatcher.java: ########## @@ -0,0 +1,257 @@ +/* + * 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.solr.util; + +import java.lang.invoke.MethodHandles; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Helper class to collect interesting callers at specific points. These calling points are + * identified by the calling class' simple name and optionally a method name and the optional + * maximum count, e.g. <code>MoreLikeThisComponent + * </code> or <code> + * ClusteringComponent.finishStage</code>, <code>ClusteringComponent.finishStage:100</code>. A + * single wildcard name <code>*</code> may be used to mean "any class", which may be useful to e.g. + * collect all callers using an expression <code>*:-1</code>. + * + * <p>Within your caller you should invoke {@link #checkCaller()} to count any matching frames in + * the current stack, and check if any of the count limits has been reached. Each invocation will + * increase the call count of the matching expression(s). For one invocation multiple matching + * expression counts can be affected because all current stack frames are examined against the + * matching expressions. + * + * <p>NOTE: implementation details cause the expression <code>simpleName[:NNN]</code> to be disabled + * when also any <code>simpleName.anyMethod[:NNN]</code> expression is used for the same class name. + * + * <p>NOTE 2: when maximum count is a negative number e.g. <code>simpleName[.someMethod]:-1</code> + * then only the number of calls to {@link #checkCaller()} for the matching expression will be + * reported but {@link #checkCaller()} will never return this expression. + */ +public class CallerMatcher { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + public static final String WILDCARD = "*"; + + private final StackWalker stackWalker = + StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE); + // className -> set of method names + private final Map<String, Set<String>> exactCallers = new HashMap<>(); + private final Map<String, Set<String>> excludeCallers = new HashMap<>(); + // expr -> initial count + private final Map<String, Integer> maxCounts = new ConcurrentHashMap<>(); + // expr -> current count + private final Map<String, AtomicInteger> callCounts = new ConcurrentHashMap<>(); + private Set<String> trippedBy = ConcurrentHashMap.newKeySet(); + + /** + * Create an instance that reacts to the specified caller expressions. + * + * @param callerExprs list of expressions in the format of <code> + * ( simpleClassName[.methodName] | * )[:NNN]</code>. If the list is empty or null then the + * first call to {@link #checkCaller()} ()} from any caller will match. + */ + public CallerMatcher(Collection<String> callerExprs, Collection<String> excludeExprs) { + for (String callerExpr : callerExprs) { + String[] exprCount = callerExpr.split(":"); + if (exprCount.length > 2) { + throw new RuntimeException("Invalid count in callerExpr: " + callerExpr); + } + String[] clazzMethod = exprCount[0].split("\\."); + if (clazzMethod.length > 2) { + throw new RuntimeException("Invalid method in callerExpr: " + callerExpr); + } + Set<String> methods = exactCallers.computeIfAbsent(clazzMethod[0], c -> new HashSet<>()); + if (clazzMethod.length > 1) { + methods.add(clazzMethod[1]); + } + if (exprCount.length > 1) { + try { + int count = Integer.parseInt(exprCount[1]); + maxCounts.put(exprCount[0], count); + callCounts.put(exprCount[0], new AtomicInteger(0)); + } catch (NumberFormatException e) { + throw new RuntimeException("Invalid count in callerExpr: " + callerExpr, e); + } + } + } + for (String excludeExpr : excludeExprs) { + String[] clazzMethod = excludeExpr.split("\\."); + if (clazzMethod.length > 2) { + throw new RuntimeException("Invalid method in excludeExpr: " + excludeExpr); + } + Set<String> methods = excludeCallers.computeIfAbsent(clazzMethod[0], c -> new HashSet<>()); + if (clazzMethod.length > 1) { + methods.add(clazzMethod[1]); + } + } + } + + /** + * Returns the set of caller expressions that were tripped (reached their count limit). This + * method can be called after {@link #checkCaller()} returns a matching expression to obtain all + * expressions that exceeded their count limits. + */ + public Set<String> getTrippedBy() { + return Collections.unmodifiableSet(trippedBy); + } + + /** Returns a map of matched caller expressions to their current call counts. */ + public Map<String, Integer> getCallCounts() { + return callCounts.entrySet().stream() + .collect( + Collectors.toMap( + e -> + e.getKey() + + (maxCounts.containsKey(e.getKey()) + ? ":" + maxCounts.get(e.getKey()) + : ""), + e -> e.getValue().get())); + } + + /** + * Returns the matching caller expression when its count limit was reached, or empty if no caller + * or no count limit was reached. Each invocation increases the call count of the matching caller, + * if any. It's up to the caller to decide whether to continue processing after this count limit + * is reached. The matching expression returned by this call will be also present in {@link + * #getTrippedBy()}. + */ + public Optional<String> checkCaller() { + return stackWalker.walk( Review Comment: the cyclomatic complexity here is a bit wild. Can you please break this up; maybe calling an extracted method or two that limits the indentation level? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
