Skip to content
Permalink
Browse files

Try to avoid `synchronized` in `Sensor#record`, which is a hot path

Originally, the `synchronized` section was introduced by quota logic,
and it is in the critical path for every record invocation.
Considering quota config is only required in every few sensors, it
doesn't make sense to slow down the hot path, and this code change
adds a check before entering the `synchronized` section to remove
the contention for regular metrics.
  • Loading branch information
gaojieliu committed Nov 21, 2019
1 parent c28a2f8 commit 2220ffd407ebc4211fb77851c9cc256f3674f2c2
Showing with 38 additions and 5 deletions.
  1. +38 −5 src/main/java/io/tehuti/metrics/Sensor.java
@@ -39,6 +39,7 @@
private final List<TehutiMetric> metrics;
private final MetricConfig config;
private final Time time;
private boolean hasQuotaConfig;

private final Map<Stat, MetricConfig> statConfigs;

@@ -53,6 +54,27 @@
this.time = time;
this.statConfigs = new HashMap<Stat, MetricConfig>();
checkForest(new HashSet<Sensor>());

this.hasQuotaConfig = false;
checkWhetherQuotaConfigPresent();
}

/**
* This function is used to check whether the attached metrics have quota config or not.
* We only need to trigger this function when there are new metric additions.
*/
private void checkWhetherQuotaConfigPresent() {
if (hasQuotaConfig) {
return;
}
for (TehutiMetric metric : metrics) {
MetricConfig metricConfig = metric.config();
if (metricConfig != null && metricConfig.quota() != null) {
hasQuotaConfig = true;
return;

}
}
}

/** Validate that this sensor doesn't end up referencing itself */
@@ -96,15 +118,23 @@ public void record(double value) {
* bound
*/
public void record(double value, long timeMs) {
synchronized (this) {
// Check quota before recording usage if needed.
checkQuotas(timeMs, true, value);
// increment all the stats
if (hasQuotaConfig) {
// Try to avoid synchronized section as much as possible
synchronized (this) {
// Check quota before recording usage if needed.
checkQuotas(timeMs, true, value);
// increment all the stats
for (int i = 0; i < this.stats.size(); i++) {
Stat stat = this.stats.get(i);
stat.record(statConfigs.get(stat), value, timeMs);
}
checkQuotas(timeMs, false, value);
}
} else {
for (int i = 0; i < this.stats.size(); i++) {
Stat stat = this.stats.get(i);
stat.record(statConfigs.get(stat), value, timeMs);
}
checkQuotas(timeMs, false, value);
}
for (int i = 0; i < parents.length; i++)
parents[i].record(value, timeMs);
@@ -166,6 +196,7 @@ private void checkQuotas(long timeMs, boolean preCheck, double requestedValue) {
this.metrics.add(metric);
addedMetrics.put(metric.name(), metric);
}
checkWhetherQuotaConfigPresent();
return addedMetrics;
}

@@ -213,6 +244,8 @@ public synchronized Metric add(String name, String description, MeasurableStat s
this.metrics.add(metric);
this.stats.add(stat);
this.statConfigs.put(stat, statConfig);
checkWhetherQuotaConfigPresent();

return metric;
}

0 comments on commit 2220ffd

Please sign in to comment.
You can’t perform that action at this time.