Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Removed unnecessary synchronization in metrics registry #1575

Merged
merged 1 commit into from
Mar 24, 2020
Merged

Removed unnecessary synchronization in metrics registry #1575

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 42 additions & 16 deletions metrics/metrics/src/main/java/io/helidon/metrics/Registry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ public ConcurrentGauge concurrentGauge(Metadata metadata, Tag... tags) {
return getOrRegisterMetric(metadata, HelidonConcurrentGauge::create, HelidonConcurrentGauge.class, tags);
}

/**
* Removes a metric by name. Synchronized for atomic update of more than one internal map.
*
* @param name Name of the metric.
* @return Outcome of removal.
*/
@Override
public synchronized boolean remove(String name) {
final boolean result = allMetricIDsByName.get(name).stream()
Expand All @@ -249,6 +255,12 @@ public synchronized boolean remove(String name) {
return result;
}

/**
* Removes a metric by ID. Synchronized for atomic update of more than one internal map.
*
* @param metricID ID of metric.
* @return Outcome of removal.
*/
@Override
public synchronized boolean remove(MetricID metricID) {
final List<MetricID> likeNamedMetrics = allMetricIDsByName.get(metricID.getName());
Expand All @@ -262,7 +274,7 @@ public synchronized boolean remove(MetricID metricID) {
}

@Override
public synchronized void removeMatching(MetricFilter filter) {
public void removeMatching(MetricFilter filter) {
allMetrics.entrySet().stream()
.filter(entry -> filter.matches(entry.getKey(), entry.getValue()))
.map(entry -> remove(entry.getKey()))
Expand Down Expand Up @@ -353,7 +365,7 @@ public Map<MetricID, Metric> getMetrics() {
}

@Override
public synchronized Map<io.helidon.common.metrics.InternalBridge.MetricID, Metric> getBridgeMetrics(
public Map<io.helidon.common.metrics.InternalBridge.MetricID, Metric> getBridgeMetrics(
Predicate<? super Map.Entry<? extends io.helidon.common.metrics.InternalBridge.MetricID,
? extends Metric>> predicate) {

Expand Down Expand Up @@ -396,8 +408,7 @@ public SortedMap<io.helidon.common.metrics.InternalBridge.MetricID, Timer> getBr
return getBridgeMetrics(getTimers(), Timer.class);
}

private static synchronized
<T extends Metric> SortedMap<io.helidon.common.metrics.InternalBridge.MetricID, T> getBridgeMetrics(
private static <T extends Metric> SortedMap<io.helidon.common.metrics.InternalBridge.MetricID, T> getBridgeMetrics(
SortedMap<MetricID, T> metrics, Class<T> clazz) {
return metrics.entrySet().stream()
.map(Registry::toBridgeEntry)
Expand Down Expand Up @@ -493,7 +504,7 @@ static Metadata toMetadata(io.helidon.common.metrics.InternalBridge.Metadata met
return builder.build();
}

synchronized Optional<Map.Entry<MetricID, HelidonMetric>> getOptionalMetricEntry(String metricName) {
Optional<Map.Entry<MetricID, HelidonMetric>> getOptionalMetricEntry(String metricName) {
return getOptionalMetricWithIDsEntry(metricName).map(entry -> {
final MetricID metricID = entry.getValue().get(0);
return new AbstractMap.SimpleImmutableEntry<>(metricID,
Expand All @@ -505,10 +516,13 @@ <T extends HelidonMetric> Optional<T> getOptionalMetric(String metricName, Class
return getOptionalMetric(new MetricID(metricName, tags), clazz);
}

<T extends HelidonMetric> Optional<T> getOptionalMetric(Metadata metadata, Class<T> clazz, Tag... tags) {
return getOptionalMetric(new MetricID(metadata.getName(), tags), clazz);
}

/**
* Get internal map entry given a metric name. Synchronized for atomic access of more than
* one internal map.
*
* @param metricName The metric name.
* @return Optional map entry..
*/
synchronized Optional<Map.Entry<HelidonMetric, List<MetricID>>> getOptionalMetricWithIDsEntry(String metricName) {
final List<MetricID> metricIDs = allMetricIDsByName.get(metricName);
if (metricIDs == null || metricIDs.isEmpty()) {
Expand All @@ -520,9 +534,7 @@ synchronized Optional<Map.Entry<HelidonMetric, List<MetricID>>> getOptionalMetri

<T extends HelidonMetric> Optional<T> getOptionalMetric(MetricID metricID, Class<T> clazz) {
return Optional.ofNullable(allMetrics.get(metricID))
.map(metric -> {
return toType(metric, clazz);
});
.map(metric -> toType(metric, clazz));
}

Type registryType() {
Expand All @@ -534,7 +546,7 @@ List<MetricID> metricIDsForName(String metricName) {
}

static <T extends Metadata, U extends Metadata> boolean metadataMatches(T a, U b) {
if ((a == null && b == null) || (a == b)) {
if (a == b) {
return true;
}
if (a == null || b == null) {
Expand Down Expand Up @@ -582,6 +594,7 @@ private static boolean enforceConsistentMetadataType(Metadata existingMetadata,
* Returns an existing metric (if one is already registered with the name
* from the metadata plus the tags, and if the existing metadata is
* consistent with the new metadata) or a new metric, registered using the metadata and tags.
* Synchronized for atomic access of more than one internal map.
*
* @param <T> type of the metric
* @param newMetadata metadata describing the metric
Expand Down Expand Up @@ -619,6 +632,7 @@ private synchronized <T extends HelidonMetric> T getOrRegisterMetric(Metadata ne
* is already registered registers a new metric using the name and type. If
* metadata with the same name already exists it is used and checked for
* consistency with the metric type {@code T}.
* Synchronized for atomic access of more than one internal map.
*
* @param <T> type of the metric
* @param metricName name of the metric
Expand Down Expand Up @@ -646,7 +660,7 @@ private synchronized <T extends HelidonMetric> T getOrRegisterMetric(String metr
* or, if none, creating new metadata based on the metric's name and type,
* returning the metric itself. Throws an exception if the metric is already
* registered or if the metric and existing metadata are incompatible.
*
* Synchronized for atomic access of more than one internal map.
*
* @param <T> type of the metric
* @param metricName name of the metric
Expand All @@ -672,6 +686,7 @@ private synchronized <T extends Metric> T registerUniqueMetric(String metricName
* by the given metadata, returning the metric itself. Throws an exception
* if the metric is already registered or if incompatible metadata is
* already registered.
* Synchronized for atomic access of more than one internal map.
*
* @param <T> type of the metric
* @param metadata metadata describing the metric
Expand Down Expand Up @@ -720,6 +735,7 @@ private <T extends HelidonMetric, U extends HelidonMetric> U toType(T m1, Class<
* Returns an existing metadata instance with the requested name or, if there
* is none, registers the provided new metadata. Throws an exception if the
* provided new metadata is incompatible with any existing metadata
* Synchronized for multiple access of an internal map.
*
* @param metricName name of the metric
* @param newMetadata new metadata to register if none exists for this name
Expand All @@ -737,7 +753,7 @@ private synchronized Metadata getOrRegisterMetadata(String metricName, Metadata
* Returns an existing metadata instance with the requested name or, if there is none,
* registers the metadata supplied by the provided metadata factory. Throws an exception
* if the provided new metric type is incompatible with any previously-registered
* metadata.
* metadata. Synchronized for multiple access of an internal map.
*
* @param metricName name of the metric
* @param newMetricType metric type of the new metric being created
Expand All @@ -762,6 +778,16 @@ private Metadata registerMetadata(Metadata metadata) {
return metadata;
}

/**
* Register a metric using name and tags. Synchronized for atomic access of more than
* one internal map.
*
* @param metricName Name of metric.
* @param metric The metric instance.
* @param tags The metric tags.
* @param <T> Metric subtype.
* @return The metric instance.
*/
private synchronized <T extends HelidonMetric> T registerMetric(String metricName, T metric, Tag... tags) {
final MetricID metricID = new MetricID(metricName, tags);
allMetrics.put(metricID, metric);
Expand Down Expand Up @@ -846,7 +872,7 @@ private static <T extends HelidonMetric> MetricType toType(Class<T> clazz) {
* @param <V> Type of class.
* @return The sorted map.
*/
private synchronized <V> SortedMap<MetricID, V> getSortedMetrics(MetricFilter filter, Class<V> metricClass) {
private <V> SortedMap<MetricID, V> getSortedMetrics(MetricFilter filter, Class<V> metricClass) {
Map<MetricID, V> collected = allMetrics.entrySet()
.stream()
.filter(it -> metricClass.isAssignableFrom(it.getValue().getClass()))
Expand Down