Skip to content

Commit 8fd9aeb

Browse files
neuenfeldttjneuenfeldttj
authored
Extend Profiler to allow for non-timing info (#18631)
* copied from other branch, ready for pr Signed-off-by: TJ Neuenfeldt <[email protected]> * cleanup from merging Signed-off-by: TJ Neuenfeldt <[email protected]> * another spotless apply and fixed breaking change Signed-off-by: TJ Neuenfeldt <[email protected]> * another spotlessApply Signed-off-by: TJ Neuenfeldt <[email protected]> * forgot to change back concurrent agg stuff Signed-off-by: TJ Neuenfeldt <[email protected]> * forgot to spotless after prev fix Signed-off-by: TJ Neuenfeldt <[email protected]> * added more unit tests for coverage Signed-off-by: TJ Neuenfeldt <[email protected]> * changed to supplier, removed timer ctors Signed-off-by: TJ Neuenfeldt <[email protected]> * made feedback changes Signed-off-by: TJ Neuenfeldt <[email protected]> * forgot javadoc for new class Signed-off-by: TJ Neuenfeldt <[email protected]> * forgot to revert name to toString Signed-off-by: TJ Neuenfeldt <[email protected]> * fix count check in concurrentquerypb Signed-off-by: TJ Neuenfeldt <[email protected]> * unmodifiable map add Signed-off-by: TJ Neuenfeldt <[email protected]> --------- Signed-off-by: TJ Neuenfeldt <[email protected]> Signed-off-by: TJ Neuenfeldt <[email protected]>
1 parent 2cec4b5 commit 8fd9aeb

28 files changed

+530
-176
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1515
- Introduce a new pull-based ingestion plugin for file-based indexing (for local testing) ([#18591](https://github.com/opensearch-project/OpenSearch/pull/18591))
1616
- Add support for search pipeline in search and msearch template ([#18564](https://github.com/opensearch-project/OpenSearch/pull/18564))
1717
- Add BooleanQuery rewrite moving constant-scoring must clauses to filter clauses ([#18510](https://github.com/opensearch-project/OpenSearch/issues/18510))
18+
- Add support for non-timing info in profiler ([#18460](https://github.com/opensearch-project/OpenSearch/issues/18460))
1819

1920
### Changed
2021
- Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570))

‎server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ public Weight createWeight(Query query, ScoreMode scoreMode, float boost) throws
213213
// createWeight() is called for each query in the tree, so we tell the queryProfiler
214214
// each invocation so that it can build an internal representation of the query
215215
// tree
216-
ContextualProfileBreakdown<QueryTimingType> profile = profiler.getQueryBreakdown(query);
216+
ContextualProfileBreakdown profile = profiler.getQueryBreakdown(query);
217217
Timer timer = profile.getTimer(QueryTimingType.CREATE_WEIGHT);
218218
timer.start();
219219
final Weight weight;

‎server/src/main/java/org/opensearch/search/profile/AbstractInternalProfileTree.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
*
4646
* @opensearch.internal
4747
*/
48-
public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown<?>, E> {
48+
public abstract class AbstractInternalProfileTree<PB extends AbstractProfileBreakdown, E> {
4949

5050
protected ArrayList<PB> breakdowns;
5151
/** Maps the Query to it's list of children. This is basically the dependency tree */

‎server/src/main/java/org/opensearch/search/profile/AbstractProfileBreakdown.java

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@
3232

3333
package org.opensearch.search.profile;
3434

35+
import java.util.Collection;
3536
import java.util.Collections;
36-
import java.util.HashMap;
3737
import java.util.Map;
38+
import java.util.TreeMap;
39+
import java.util.function.Supplier;
40+
import java.util.stream.Collectors;
3841

3942
import static java.util.Collections.emptyMap;
4043

@@ -45,58 +48,51 @@
4548
*
4649
* @opensearch.internal
4750
*/
48-
public abstract class AbstractProfileBreakdown<T extends Enum<T>> {
51+
public abstract class AbstractProfileBreakdown {
4952

50-
/**
51-
* The accumulated timings for this query node
52-
*/
53-
protected final Timer[] timings;
54-
protected final T[] timingTypes;
55-
public static final String TIMING_TYPE_COUNT_SUFFIX = "_count";
56-
public static final String TIMING_TYPE_START_TIME_SUFFIX = "_start_time";
53+
protected final Map<String, ProfileMetric> metrics;
5754

5855
/** Sole constructor. */
59-
public AbstractProfileBreakdown(Class<T> clazz) {
60-
this.timingTypes = clazz.getEnumConstants();
61-
timings = new Timer[timingTypes.length];
62-
for (int i = 0; i < timings.length; ++i) {
63-
timings[i] = new Timer();
64-
}
56+
public AbstractProfileBreakdown(Collection<Supplier<ProfileMetric>> metricSuppliers) {
57+
this.metrics = metricSuppliers.stream().map(Supplier::get).collect(Collectors.toMap(ProfileMetric::getName, metric -> metric));
6558
}
6659

67-
public Timer getTimer(T timing) {
68-
return timings[timing.ordinal()];
60+
public Timer getTimer(Enum<?> type) {
61+
ProfileMetric metric = metrics.get(type.toString());
62+
assert metric instanceof Timer : "Metric " + type + " is not a timer";
63+
return (Timer) metric;
6964
}
7065

71-
public void setTimer(T timing, Timer timer) {
72-
timings[timing.ordinal()] = timer;
66+
public ProfileMetric getMetric(String name) {
67+
return metrics.get(name);
7368
}
7469

7570
/**
76-
* Build a timing count breakdown for current instance
71+
* Build a breakdown for current instance
7772
*/
7873
public Map<String, Long> toBreakdownMap() {
79-
Map<String, Long> map = new HashMap<>(this.timings.length * 3);
80-
for (T timingType : this.timingTypes) {
81-
map.put(timingType.toString(), this.timings[timingType.ordinal()].getApproximateTiming());
82-
map.put(timingType + TIMING_TYPE_COUNT_SUFFIX, this.timings[timingType.ordinal()].getCount());
83-
map.put(timingType + TIMING_TYPE_START_TIME_SUFFIX, this.timings[timingType.ordinal()].getEarliestTimerStartTime());
74+
Map<String, Long> map = new TreeMap<>();
75+
for (Map.Entry<String, ProfileMetric> entry : metrics.entrySet()) {
76+
map.putAll(entry.getValue().toBreakdownMap());
8477
}
8578
return Collections.unmodifiableMap(map);
8679
}
8780

81+
public long toNodeTime() {
82+
long total = 0;
83+
for (Map.Entry<String, ProfileMetric> entry : metrics.entrySet()) {
84+
if (entry.getValue() instanceof Timer t) {
85+
total += t.getApproximateTiming();
86+
}
87+
}
88+
return total;
89+
}
90+
8891
/**
8992
* Fetch extra debugging information.
9093
*/
9194
public Map<String, Object> toDebugMap() {
9295
return emptyMap();
9396
}
9497

95-
public long toNodeTime() {
96-
long total = 0;
97-
for (T timingType : timingTypes) {
98-
total += timings[timingType.ordinal()].getApproximateTiming();
99-
}
100-
return total;
101-
}
10298
}

‎server/src/main/java/org/opensearch/search/profile/AbstractProfiler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
*
4040
* @opensearch.internal
4141
*/
42-
public class AbstractProfiler<PB extends AbstractProfileBreakdown<?>, E> {
42+
public abstract class AbstractProfiler<PB extends AbstractProfileBreakdown, E> {
4343

4444
protected final AbstractInternalProfileTree<PB, E> profileTree;
4545

‎server/src/main/java/org/opensearch/search/profile/ContextualProfileBreakdown.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,29 @@
1111
import org.apache.lucene.index.LeafReaderContext;
1212
import org.apache.lucene.search.Collector;
1313

14+
import java.util.Collection;
1415
import java.util.List;
1516
import java.util.Map;
17+
import java.util.function.Supplier;
1618

1719
/**
1820
* Provide contextual profile breakdowns which are associated with freestyle context. Used when concurrent
1921
* search over segments is activated and each collector needs own non-shareable profile breakdown instance.
2022
*
2123
* @opensearch.internal
2224
*/
23-
public abstract class ContextualProfileBreakdown<T extends Enum<T>> extends AbstractProfileBreakdown<T> {
24-
public ContextualProfileBreakdown(Class<T> clazz) {
25-
super(clazz);
25+
public abstract class ContextualProfileBreakdown extends AbstractProfileBreakdown {
26+
27+
public ContextualProfileBreakdown(Collection<Supplier<ProfileMetric>> metrics) {
28+
super(metrics);
2629
}
2730

2831
/**
2932
* Return (or create) contextual profile breakdown instance
3033
* @param context freestyle context
3134
* @return contextual profile breakdown instance
3235
*/
33-
public abstract AbstractProfileBreakdown<T> context(Object context);
36+
public abstract AbstractProfileBreakdown context(Object context);
3437

3538
public void associateCollectorToLeaves(Collector collector, LeafReaderContext leaf) {}
3639

‎server/src/main/java/org/opensearch/search/profile/ProfileMetric.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.search.profile;
10+
11+
import org.opensearch.common.annotation.ExperimentalApi;
12+
13+
import java.util.Map;
14+
15+
/**
16+
* A metric for profiling.
17+
*/
18+
@ExperimentalApi
19+
public abstract class ProfileMetric {
20+
21+
private final String name;
22+
23+
public ProfileMetric(String name) {
24+
this.name = name;
25+
}
26+
27+
/**
28+
*
29+
* @return name of the metric
30+
*/
31+
public String getName() {
32+
return name;
33+
}
34+
35+
/**
36+
*
37+
* @return map representation of breakdown
38+
*/
39+
abstract public Map<String, Long> toBreakdownMap();
40+
}

‎server/src/main/java/org/opensearch/search/profile/ProfileMetricUtil.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.search.profile;
10+
11+
import org.opensearch.search.profile.aggregation.AggregationTimingType;
12+
import org.opensearch.search.profile.query.QueryTimingType;
13+
14+
import java.util.ArrayList;
15+
import java.util.Collection;
16+
import java.util.function.Supplier;
17+
18+
/**
19+
* Utility class to provide profile metrics to breakdowns.
20+
*/
21+
public class ProfileMetricUtil {
22+
23+
public static Collection<Supplier<ProfileMetric>> getDefaultQueryProfileMetrics() {
24+
Collection<Supplier<ProfileMetric>> metrics = new ArrayList<>();
25+
for (QueryTimingType type : QueryTimingType.values()) {
26+
metrics.add(() -> new Timer(type.toString()));
27+
}
28+
return metrics;
29+
}
30+
31+
public static Collection<Supplier<ProfileMetric>> getAggregationProfileMetrics() {
32+
Collection<Supplier<ProfileMetric>> metrics = new ArrayList<>();
33+
for (AggregationTimingType type : AggregationTimingType.values()) {
34+
metrics.add(() -> new Timer(type.toString()));
35+
}
36+
return metrics;
37+
}
38+
}

‎server/src/main/java/org/opensearch/search/profile/ProfileResult.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ static void removeStartTimeFields(Map<String, Long> modifiedBreakdown) {
271271
Iterator<Map.Entry<String, Long>> iterator = modifiedBreakdown.entrySet().iterator();
272272
while (iterator.hasNext()) {
273273
Map.Entry<String, Long> entry = iterator.next();
274-
if (entry.getKey().endsWith(AbstractProfileBreakdown.TIMING_TYPE_START_TIME_SUFFIX)) {
274+
if (entry.getKey().endsWith(Timer.TIMING_TYPE_START_TIME_SUFFIX)) {
275275
iterator.remove();
276276
}
277277
}

‎server/src/main/java/org/opensearch/search/profile/Profilers.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public QueryProfiler addQueryProfiler() {
7979

8080
/** Get the current profiler. */
8181
public QueryProfiler getCurrentQueryProfiler() {
82-
return queryProfilers.get(queryProfilers.size() - 1);
82+
return queryProfilers.getLast();
8383
}
8484

8585
/** Return the list of all created {@link QueryProfiler}s so far. */

‎server/src/main/java/org/opensearch/search/profile/Timer.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232

3333
package org.opensearch.search.profile;
3434

35+
import java.util.HashMap;
36+
import java.util.Map;
37+
3538
/** Helps measure how much time is spent running some methods.
3639
* The {@link #start()} and {@link #stop()} methods should typically be called
3740
* in a try/finally clause with {@link #start()} being called right before the
@@ -48,16 +51,19 @@
4851
*
4952
* @opensearch.internal
5053
*/
51-
public class Timer {
54+
public class Timer extends ProfileMetric {
55+
public static final String TIMING_TYPE_COUNT_SUFFIX = "_count";
56+
public static final String TIMING_TYPE_START_TIME_SUFFIX = "_start_time";
5257

5358
private boolean doTiming;
5459
private long timing, count, lastCount, start, earliestTimerStartTime;
5560

56-
public Timer() {
57-
this(0, 0, 0, 0, 0);
61+
public Timer(String name) {
62+
super(name);
5863
}
5964

60-
public Timer(long timing, long count, long lastCount, long start, long earliestTimerStartTime) {
65+
public Timer(long timing, long count, long lastCount, long start, long earliestTimerStartTime, String name) {
66+
super(name);
6167
this.timing = timing;
6268
this.count = count;
6369
this.lastCount = lastCount;
@@ -131,4 +137,13 @@ public final long getApproximateTiming() {
131137
}
132138
return timing;
133139
}
140+
141+
@Override
142+
public Map<String, Long> toBreakdownMap() {
143+
Map<String, Long> map = new HashMap<>();
144+
map.put(getName(), getApproximateTiming());
145+
map.put(getName() + TIMING_TYPE_COUNT_SUFFIX, getCount());
146+
map.put(getName() + TIMING_TYPE_START_TIME_SUFFIX, getEarliestTimerStartTime());
147+
return map;
148+
}
134149
}

‎server/src/main/java/org/opensearch/search/profile/aggregation/AggregationProfileBreakdown.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,13 @@
3434

3535
import org.opensearch.common.annotation.PublicApi;
3636
import org.opensearch.search.profile.AbstractProfileBreakdown;
37+
import org.opensearch.search.profile.ProfileMetric;
38+
import org.opensearch.search.profile.ProfileMetricUtil;
3739

40+
import java.util.Collection;
3841
import java.util.HashMap;
3942
import java.util.Map;
43+
import java.util.function.Supplier;
4044

4145
import static java.util.Collections.unmodifiableMap;
4246

@@ -46,11 +50,15 @@
4650
* @opensearch.api
4751
*/
4852
@PublicApi(since = "1.0.0")
49-
public class AggregationProfileBreakdown extends AbstractProfileBreakdown<AggregationTimingType> {
53+
public class AggregationProfileBreakdown extends AbstractProfileBreakdown {
5054
private final Map<String, Object> extra = new HashMap<>();
5155

5256
public AggregationProfileBreakdown() {
53-
super(AggregationTimingType.class);
57+
this(ProfileMetricUtil.getAggregationProfileMetrics());
58+
}
59+
60+
public AggregationProfileBreakdown(Collection<Supplier<ProfileMetric>> timers) {
61+
super(timers);
5462
}
5563

5664
/**
@@ -64,4 +72,5 @@ public void addDebugInfo(String key, Object value) {
6472
public Map<String, Object> toDebugMap() {
6573
return unmodifiableMap(extra);
6674
}
75+
6776
}

‎server/src/main/java/org/opensearch/search/profile/aggregation/ConcurrentAggregationProfiler.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313

1414
package org.opensearch.search.profile.aggregation;
1515

16-
import org.opensearch.search.profile.AbstractProfileBreakdown;
1716
import org.opensearch.search.profile.ProfileResult;
17+
import org.opensearch.search.profile.Timer;
1818

1919
import java.util.HashMap;
2020
import java.util.LinkedList;
@@ -31,7 +31,7 @@ public class ConcurrentAggregationProfiler extends AggregationProfiler {
3131
private static final String MAX_PREFIX = "max_";
3232
private static final String MIN_PREFIX = "min_";
3333
private static final String AVG_PREFIX = "avg_";
34-
private static final String START_TIME_KEY = AggregationTimingType.INITIALIZE + AbstractProfileBreakdown.TIMING_TYPE_START_TIME_SUFFIX;
34+
private static final String START_TIME_KEY = AggregationTimingType.INITIALIZE + Timer.TIMING_TYPE_START_TIME_SUFFIX;
3535
private static final String[] breakdownCountStatsTypes = { "build_leaf_collector_count", "collect_count" };
3636

3737
@Override
@@ -82,8 +82,7 @@ private List<ProfileResult> reduceProfileResultsTree(List<ProfileResult> profile
8282
// Profiled breakdown total time
8383
for (AggregationTimingType timingType : AggregationTimingType.values()) {
8484
String breakdownTimingType = timingType.toString();
85-
Long startTime = profileResult.getTimeBreakdown()
86-
.get(breakdownTimingType + AbstractProfileBreakdown.TIMING_TYPE_START_TIME_SUFFIX);
85+
Long startTime = profileResult.getTimeBreakdown().get(breakdownTimingType + Timer.TIMING_TYPE_START_TIME_SUFFIX);
8786
Long endTime = startTime + profileResult.getTimeBreakdown().get(breakdownTimingType);
8887
minSliceStartTimeMap.put(
8988
breakdownTimingType,
@@ -103,7 +102,7 @@ private List<ProfileResult> reduceProfileResultsTree(List<ProfileResult> profile
103102
// Profiled breakdown count
104103
for (AggregationTimingType timingType : AggregationTimingType.values()) {
105104
String breakdownType = timingType.toString();
106-
String breakdownTypeCount = breakdownType + AbstractProfileBreakdown.TIMING_TYPE_COUNT_SUFFIX;
105+
String breakdownTypeCount = breakdownType + Timer.TIMING_TYPE_COUNT_SUFFIX;
107106
breakdown.put(
108107
breakdownTypeCount,
109108
breakdown.getOrDefault(breakdownTypeCount, 0L) + profileResult.getTimeBreakdown().get(breakdownTypeCount)

0 commit comments

Comments
 (0)