Skip to content

Conversation

@mkuratczyk
Copy link
Contributor

If someone uses this function, the list of counters to return is likely very short, quite possibly it has 1 element. Rather than building a map of all counters and then dropping the irrelevant ones, only add the relevant counters in the first place.

Testing with RabbitMQ an rabbit_quorum_queue:open_files function, which only asks for 1 metric, the avery execution went down form 7ms to 1ms with this change.

If someone uses this function, the list of counters to return
is likely very short, quite possibly it has 1 element.
Rather than building a map of all counters and then dropping
the irrelevant ones, only add the relevant counters in the first place.

Testing with RabbitMQ an rabbit_quorum_queue:open_files function,
which only asks for 1 metric, the avery execution went down form 7ms
to 1ms with this change.
@michaelklishin
Copy link
Contributor

michaelklishin commented Nov 26, 2025

@mkuratczyk WDYT about going further and adding a separate function head for a single element list?

We have been doing similar things on some RabbitMQ hot code paths (routing) since… 2008 or so?

Maybe that would not be meaningful enough but also should be easy to try, e.g.

build_counters_map(CRef, FieldsSpec, [Name]) ->
  %% construct a single element map using `Name`

@mkuratczyk
Copy link
Contributor Author

Makes sense. Let me test that

@kjnilsson
Copy link
Contributor

@mkuratczyk WDYT about going further and adding a separate function head for a single element list?

We have been doing similar things on some RabbitMQ hot code paths (routing) since… 2008 or so?

Maybe that would be be meaningful enough but also should be easy to try, e.g.

build_counters_map(CRef, FieldsSpec, [Name]) ->
  %% construct a single element map using `Name`

given we still have to fold over the full list of fieldsspec I don't think this will yield meaningful value. If we really wanted to optimise we could make FieldsSpec a map but that would mean a lot more changes

Previous commit sped up single-item invocation from
7ms to 1ms. With this change, it goes down to 0.1ms.
@mkuratczyk
Copy link
Contributor Author

Turns out it does make a significant difference - initially from 7ms to 1ms. With the second commit from 1ms to 0.1ms. :)

Nice one @michaelklishin !

@michaelklishin michaelklishin added this to the 1.0.1 milestone Nov 26, 2025
@michaelklishin michaelklishin merged commit ae76606 into main Nov 26, 2025
3 checks passed
@michaelklishin
Copy link
Contributor

1.0.1 is already up on hex.pm.

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

Successfully merging this pull request may close these issues.

4 participants