Consider changing `Registry::register/unregister` to remove the `Box` in the argument

[nagisa]

Currently register and unregister methods of Registry take Box<dyn Collector>, however in practice this means that most calls of this method will end up looking like this:

let metric = /* a concrete type of some sort */;
registry.register(Box::new(metric.clone()));

This feels unsatisfactory in multiple ways, for example I cannot sleep well knowing there’s an unnecessary double indirection in my code base (Box to Arc to the actual metric, could instead be just Arc<dyn Collector>). But also the API is not too convenient and fails to hide implementation details. It could instead be like this

fn register<C: Collector>(&self, collector: C) { /* do the boxing inside */ };
// use as `registry.register(metric.clone());`

fn unregister<C: Collector>(&self, collector: &C) { /* does this even need a box???  */ };
// use as `registry.unregister(&metric);`

and no longer force users to deal with the manual Boxing.

[breezewish]

If I remember correctly, the Box lives for a reason that inside register() the collector need to be put inside a Vec, which has to be Vec<Box<dyn Collector>>. Thus, for a &C interface, a Box inside register is still needed.

[breezewish]

However I agree that manual boxing is not friendly.

[mxinden]

The Box is needed to store multiple Collectors of different type via dynamic dispatching in the RegistryCore:

https://github.com/tikv/rust-prometheus/blob/227a1ce83bda0fd62115e4e914fc1b4d4cfc4907/src/registry.rs#L16

/* do the boxing inside */

While reducing the noise of a register invocation it would as well hide the heap allocation.

/* does this even need a box??? */

I guess not as unregister only needs the collector ids.

I would guess register and unregister are not in the hot path of most users applications. Thus I would be careful doing any changes to these APIs without benchmarks proving that not dynamically dispatching actually has an impact.