Guard: implement Clone

[bonzini]

The guard that is returned by load() generally behaves as an Arc<>; however, unlike Arc<> it does not implement Clone on itself. Usually it is enough to clone the underlying Arc<>, but not if you have a trait like

trait GiveMeASmartPointer {
    type Result = Clone + Deref<Target = SomeType>;

    fn give_it_to_me(&self) -> Self::Result;
}

To support this, implement Clone on Guard itself, cloning the inner Arc into a new, unprotected guard.

[vorner]

Hello.

Thanks for the pull request, I'm happy someone finds the library useful enough to actually send it :-).

Nevertheless, I'm not really sure the Clone implementation makes sense API-wise. It is definitely correct, but I'm afraid this might hide some performance implications or act in surprising ways. After all, the RwLockReadGuard, which is kind of the parallel of the Guard type here, doesn't implement Clone either.

However, I see you have a use case for the functionality. I'll try to think about it for a day or two. Alternatively, I'll think of some other way how to accomplish what you do in the other pull request. Would that be OK?

[bonzini]

I'm afraid this might hide some performance implications or act in surprising ways

Yes, performance-wise it's not great. That said, anyone using Arc is probably aware that clone can have performance implications.

However, I see you have a use case for the functionality. I'll try to think about it for a day or two. Alternatively, I'll think of some other way how to accomplish what you do in the other pull request. Would that be OK?

Yes, of course.

[vorner]

Yes, performance-wise it's not great. That said, anyone using Arc is probably aware that clone can have performance implications.

That's the problem. By cloning a Guard, that is advertised as lightweight, it is not clear an Arc is being cloned too. In theory, it wouldn't have to be, it would be possible to allocate another slot in the debt storage, for example. It's just is not worth the complexity.

I'm thinking about adding a from_arc or from_inner or something like that to Guard. Then you would not clone the guard, but would do something like that:

guard: Guard::from_arc(Arc::clone(*self.guard))

Would that be acceptable? I think such API would make it clearer what happens behind the scenes and one can't really clone it by accident.

[bonzini]

It's just is not worth the complexity.

Well, for my use case it might be worthwhile if it means no atomics in the fast path.

Would that be acceptable? I

Yes, I would just implement clone in my crate's own wrapper for arc_swap::Guard.

[bonzini]

I tried making it create a Protection::debt guard. I started with some refactoring so that I can have a clone_fallible/clone pair not entirely unlike load_fallible/load.

[bonzini]

Looking more at src/debt.rs there is anyway a compare-and-swap in Debt::new; we need it as a sort of hazard pointer when loading the ArcSwap, but not when cloning. Therefore, it may actually not be that much faster to get a new debt compared to just cloning. I'll open a new pull request for from_inner.