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.

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

MetricRegistry API - Don't assure sorted property for returned maps and sets #627

Open
jbee opened this issue Dec 1, 2020 · 0 comments
Open

Comments

@jbee
Copy link
Contributor

jbee commented Dec 1, 2020

Currently the MetricRegistry API often returns a SortedSet and SortedMap.

While it is understandable from user (caller) point of view that a sorted set or map can be more convenient it is not needed is many situations and can easily be constructed from an "ordinary" Set or Map.
Implementations could still opt to actually return SortedSet or SortedMap and the eventual applied conversion to a SortedSet or SortedMap as done by the caller can be a NOOP like in this pseudo-code example:

<T> SortedSet<T> toSortedSet(Set<T> set) {
  return set instanceof SortedSet ? return (SortedSet<T>) set : new TreeSet<>(set));
}

Gist is: replacing SortedSet with mere Set in the MetricRegistry API does not remove any capability. It only comes with less convenience in the case we actually want to rely on the additional property that the set or map is indeed sorted.

On the other hand if we require the sets and maps returned by the MetricRegistry to be SortedSet and SortedMap the implementation cannot return read-only views onto its internal data organisation that are cheap to produce unless these are also sorted. This excludes for example sets of keys or values as returned by concurrent maps and concurrent map implementations which are a likely candidates to implement the registry.

Example

Implementations could use the existing API method

SortedMap<MetricID, Metric> getMetrics(MetricFilter filter);

to effectively offer a not yet existing lookup by name

Map<MetricID, Metric> getMetrics(String name);

by recognising the filter implementation being one that only wants to filter on the name in which case it can use a more efficient path returning a view. The reason this is not possible is that the internal map is not sorted as the main concern of the implementation are concurrent reads and inserts. So it would need to construct a new map from the view.

@jbee jbee changed the title MetricRegistry API - Use Set instead of SortedSet MetricRegistry API - Don't assure sorted property for returned maps and sets Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant