time: avoid traversing entries in the time wheel twice

[wathenjiang]

This is for reverting the cached_when in #6584

In the extend_expiration method, we will not necessarily re-move the timer entry, but its state can be modified.

So we use the cached_when to calculate which slot the timer entry is currently stored in.

[Darksonn]

What has changed since the original PR? Can you add a test for the previous bug? We probably won't merge this until we have #6717, but it still makes sense to add a dedicated test for the problem.

[wathenjiang]

What has changed since the original PR? Can you add a test for the previous bug? We probably won't merge this until we have #6717, but it still makes sense to add a dedicated test for the problem.

• The new added test is reset_timer_and_drop.
• The change from the original PR can be seen in cd70a24

[Darksonn]

The old code doesn't call this. Why is it necessary now?

[wathenjiang]

The reason I add occupied_bit_maintain now is that in the past, we would use take_entries to take all items from the linked list at once and then update the occupied bit. But now, we need to traverse the items one by one, which means we need to update the occupied bit after the traversal is completed. I think the difference here is that in the current version, the linked list is not necessarily empty, so I add this method.

[Darksonn]

Would it be accurate to say that this is an operation that must be called when we are done with list? Would it make sense for it to be part of the destructor of EntryWaitersList?

[Darksonn]

Does it need to be called before drop(lock) in if !waker_list.can_push() {?

[wathenjiang]

Calling occupied_bit_maintain before drop(lock) in if !waker_list.can_push() { makes sense to me. That is a good idea.

[Darksonn]

What about set_elapsed?

I would really like to see a loom test where a timer is inserted during the if !waker_list.can_push() { ... } block. It's hard for me to tell whether it's correct.

I think you can do the loom test along these lines:

1. Start a runtime with paused time.
2. Create and poll 32 timers with the timer.
3. Spawn a thread. The background thread will create a timer, register it, and wait for it to complete.
4. In parallel with that wait for the 32 timers in order.
5. Join the background thread.

Spawning the thread after registering the 32 timers should reduce the size of the loom test, since the region with more than 1 thread will be shorter. As long as the if !waker_list.can_push() { ... } block happens in the more-than-1-thread region of the test, it should work.

[Darksonn]

Can STATE_DEREGISTERED actually happen?

[wathenjiang]

No. I think it will not happen.