Skip to content

Improved Meter.Id#getTags() performance #6182

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

Merged
merged 2 commits into from
Jun 4, 2025
+19 −5
Merged

Improved Meter.Id#getTags() performance #6182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3fa190
Improved Meter.Id#getTags() performance
etki Apr 26, 2025
6019ffb
Use size of Tags to size List in getTags
shakuzen Jun 3, 2025
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
15 changes: 10 additions & 5 deletions micrometer-core/src/main/java/io/micrometer/core/instrument/Meter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ public Id withTags(Iterable<Tag> tags) {
/**
* Generate a new id replacing all tags with new ones.
* @param tags The tags to add.
* @return A new id with the only the provided tags. The source id remains
* unchanged.
* @return A new id with only the provided tags. The source id remains unchanged.
* @since 1.1.0
*/
public Id replaceTags(Iterable<Tag> tags) {
Expand Down Expand Up @@ -282,9 +281,15 @@ public String getName() {
* @return A set of dimensions that allows you to break down the name.
*/
public List<Tag> getTags() {
List<Tag> tags = new ArrayList<>();
this.tags.forEach(tags::add);
return Collections.unmodifiableList(tags);
if (this.tags == Tags.empty()) {
Copy link
Contributor Author

@etki etki Apr 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to explicitly check for zero length, as it's possible to construct another instance with zero tags, but we don't have that API.

Copy link
Member

@shakuzen shakuzen May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth adding API for size/length to Tags. Given this is in the same package, we can make it package private. We could consider separately whether to make it public API if there's good justification for it.

Copy link
Member

@shakuzen shakuzen Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's possible to construct another instance with zero tags

I'd also note it's not possible with public API, so as long as we don't inadvertently do it, we shouldn't need to worry about that case.

return Collections.emptyList();
}

List<Tag> list = new ArrayList<>(this.tags.size());
for (Tag tag : this.tags) {
list.add(tag);
}
return Collections.unmodifiableList(list);
}

public Iterable<Tag> getTagsAsIterable() {
Expand Down
9 changes: 9 additions & 0 deletions micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,15 @@ public Tags and(@Nullable Iterable<? extends Tag> tags) {
return merge(Tags.of(tags));
}

/**
* Non-public (for now) method to get the size of this, which can be useful in sizing
* a collection where these elements will be copied.
* @return number of unique {@link Tag} instances in this
*/
int size() {
return length;
}

@Override
public Iterator<Tag> iterator() {
return new ArrayIterator();
Expand Down