Optimize upgrade method of alloc::sync::Weak

[wyang5]

This is my first time contributing to Rust, so apologies if I’ve broken any rules or tagged the wrong people.

Background

The upgrade method on Weak is currently implemented using a CAS loop, to ensure that the increment to the strong count cannot occur if it is zero. This quickly becomes inefficient if other threads attempt to upgrade the Weak, clone or drop the Arc, or otherwise update the strong count. The CAS will fail every time the strong count changes, even if it’s nowhere near zero. In theory, it may never succeed.

Consider a basic doubly linked list with strong next pointers and weak prev pointers. Given N nodes, M threads, and just 1 update to the strong count per thread, a simple backward iteration could end up being N*M operations in the worst case.

Solution

We can utilize a “sticky counter” mechanism, which I encountered in section 4.3 of this paper. This eliminates the CAS loop and replaces it with an “increment-if-not-zero” mechanism. I propose a modified version of the counter described in the paper, which only steals a single high bit (indicating whether the count is zero). That actually fits nicely with the current MAX_REFCOUNT, as the weak count is already effectively using its high bit as a spinlock flag. The algorithm has no additional overhead; all increments and decrements will succeed or fail in a single atomic operation (until the counter reaches zero). This would make the list iteration example O(N) in all cases.

Considerations

• The authors of the paper also used a second “help bit” in their formulation, to allow a single decrement operation to “take credit” for bringing the counter to zero in some edge cases. I could not think of a reason why we would need that, so I omitted it. If it turns out that I made a mistake, then 2 bits would be required, and that may or may not be a dealbreaker.
• The correct memory orderings need to be hammered out - right now I have many of them set to SeqCst as a proof of concept.
• If we go forward with this, I’m guessing we’ll want a Crater test at some point to make sure that this doesn’t nuke everything that uses Arc / Weak.

Edit 1:

• The "help bit" is needed, so I added it. However, the second bit being used actually isn't a concern after all w.r.t MAX_REFCOUNT since it isn't used until the counter reaches zero.

Edit 2:
Closing the PR - it turns out that the counter cannot be used in this context (it only works if there also exists a memory reclamation mechanism, which is not the case with Arc / Weak).

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue