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

Implement PartialOrd and Ord for callsite::Identifier #3161

Open
Robo210 opened this issue Nov 27, 2024 · 0 comments
Open

Implement PartialOrd and Ord for callsite::Identifier #3161

Robo210 opened this issue Nov 27, 2024 · 0 comments

Comments

@Robo210
Copy link

Robo210 commented Nov 27, 2024

Feature Request

Crates

tracing-core

Motivation

The tracing-etw crate needs to associate extra metadata with each callsite. Currently this can be done with a hashmap, as the callsite identity implements Hash, but a hash map is not ideal due to the unnecessary overhead (the list of callsites never changes once populated, so we don't really need mutability and all the performance costs associated with it, etc.). A simple fixed-size, sorted array of callsite identities and the extra metadata will be leaner, if only there was a way to sort and compare the Identifiers without hashing them.

Proposal

Implement PartialOrd and Ord (and Eq) on callsite::Identifier. Since an Identifier is currently an opaque wrapper around a pointer, these can be implemented via core::ptr::[partial_]cmp, just like how PartialEq is currently implemented.

I can send a PR to do this if there are no objections to this idea.

Alternatives

Currently tracing-etw is using the hashed value of the callsite Identifier to compare and sort them. This means it has to have a global hasher and spend CPU cycles every time something is logged to compute a hash of what is really just a single pointer value, just so it can perform a lookup in its sorted array. This is an improvement over prior iterations, but still not ideal.

Prior iterations used DashMap<callsite::Identifier, metadata>, but this required the map to acquire a reader-lock every lookup and uses multiple heap allocations, neither of which are truly necessary in this scenario since the map is effectively completely immutable.

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