-
Notifications
You must be signed in to change notification settings - Fork 188
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
Allow into/from inner Arc
conversions
#472
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: tyranron <[email protected]>
Signed-off-by: tyranron <[email protected]>
Thanks for the patch. I'm still slightly unconvinced on the I'd even say the It is my understanding that the correctness of this relies on |
This way, the people who really need this (like me), may not find it at all. To be honest, for my needs, adding
Not sure, I've understood you. You mean my use case, or the proposed API of
In my use case, I need to return to library users a type being essentially an |
I meant on the proposed API for the |
Ooops, I didn't notice that. Thought it was public as the type was Yup, that's unfortunate, as even for
Not yet. I'll craft the PR referring this one and write back. |
Overview
This PR adds:
into_arc_value()
andfrom_arc_value()
methods toGenericCounter
andGenericGauge
types. These methods areunsafe
as allow bypassing counter/gauge semantics.into_arc_core()
andfrom_arc_core()
methods toHistogram
type. These methods are safe, as theHistogramCore
public API doesn't allow bypassingHistogram
semantics in any way.#[repr(transparent)]
attribute toGenericCounter
,GenericGauge
andHistogram
, so they can be safely transmuted into its innerArc
and back.Motivation
While implementing the
metrics-prometheus
crate, I have the situation where themetrics
crate requires the returned metric beingArc
-wrapped (because returns to usersArc<dyn Trait>
essentially), and so, pushing me to introduce a completely redundant allocation on the hot path accessing the metric, becauseprometheus
metric types haveArc
ed semantics anyway.First, I've tried to approach this on the
metrics
crate's side, but had no luck, as its API returnsArc<dyn Trait>
to user code, so creating anArc
cannot be really bypassed here.Then, I've looked at
prometheus
metric type's structure and realized that I can unwrap it into its innerArc
, and then, in theTrait
implementation, wrap back into the original metric type. The whole thing doesn't leave the library boundary, so users won't be able to misuse the unwrappedArc
s anyhow.As the bonus, it will allow me to solve the synchronization issue from #471, but without exposing safe APIs to
prometheus
users which do not follow metric type's semantics.Notes
this: Self
argument instead of justself
ininto*
methods, mimicking theArc::into_raw()
, to additionally discourage library users from using those methods.Checklist
Tests are added(do we need ones here?)