[API Proposal]: ConditionalWeakTable<TKey,TValue>.Remove overload

[Sergio0694]

Background and motivation

ConditionalWeakTable<TKey, TValue> has a Remove method that internally also retrieves the removed item, if present, but does not surface it. There is no existing method on it that allows removing an item while also getting the removed item, if present. This makes it unnecessarily expensive to opportunistically remove items while also doing something with them (eg. removing them from some other data structure).

Eg. we hit this in the Windows Community Toolkit (CommunityToolkit/Windows#569):

if (_debounceInstances.TryGetValue(timer, out Action? action))
{
    _debounceInstances.Remove(timer); // Repeated lookup 🙁
    action.Invoke();
}

It would be nice to just be able to do this with a single API, which would be simpler to read and also more efficient:

if (_debounceInstances.Remove(timer, out Action? action))
{
    action.Invoke();
}

The new API would also make the whole operation atomic, which is also nice to have and might be necessary in some scenarios.

Implementing the new API is trivial, as ConditionalWeakTable<TKey, TValue> already has the necessary logic, just not exposed.

API Proposal

namespace System.Runtime.CompilerServices;

public sealed class ConditionalWeakTable<TKey, TValue>
{
    public bool Remove(TKey key);
+   public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value);
}

API Usage

Updating the example above to use the new API:

if (_debounceInstances.Remove(timer, out Action? action))
{
    action.Invoke();
}

Alternative Designs

Keep using TryGetValue + Remove, and waste an extra lookup. Also the whole thing is not atomic. And it's clunkier.

Risks

None, it's just a new convenience API with a well known signature also used on lots of other similar data structures.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

[Sergio0694]

@eiriktsarpalis small follow up to #89002, if you will. Would you be able to help get this to API review? 😄
Thank you!

[michael-hawker]

This would also match the existing patterns established by other collections like ConcurrentDictionary which we were using before migrating to ConditionalWeakTable, which is how Sergio discovered this gap when reviewing our change.

RE: https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.tryremove?view=net-9.0

[michael-hawker]

Was just thinking broader beyond this; in the future, should TryRemove be on the base IDictionary<TKey,TValue> where TryGetValue and Remove come from?

[Sergio0694]

Note that ConcurrentWeakTable<,> is intentionally not a dictionary (even though it's similar to one).
That should be in its own separate proposal (though it'd seem like a tough sell to me, as it'd be a new interface method).

[eiriktsarpalis]

Note that the existing Remove returns a boolean and is effectively already a "TryRemove" (it has the same signature and behavior as the TryRemove method in ConcurrentDictionary). So to me this feels like it should be an overload to Remove as opposed to a separate method group.

[Sergio0694]

Makes sense to me, updated! 🙂

[bartonjs]

Video

• Looks good as proposed
• We raised the question of should it be "TryRemove" instead of "Remove", but left it as "Remove" to be consistent with the same overload pair on Dictionary`2.

namespace System.Runtime.CompilerServices;

public sealed partial class ConditionalWeakTable<TKey, TValue>
{
    public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value);
}