Add CollectionsMarshal.SetCount(list, count)

[layomia]

Fixes #55217.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[stephentoub]

Is there a use case for setting a value smaller than the current count? That's already possible with RemoveRange.

[MichalPetryka]

Having the API not support it would certainly be unexpected.

[stephentoub]

Would it? Trying to set Capacity to less than Count throws. How is this different? And what's the use case where you'd call this to shrink rather than grow? I'm not against it if it's useful, I'm trying to understand the use.

[MichalPetryka]

It'd be useful in cases where you don't know whether the count will be smaller or bigger than the list size.

[stephentoub]

What's an example of that?

[DaZombieKiller]

In which case I'm not sure why we're clearing. If this is about an advanced developer using this unsafe API to do something that requires them to know what they're doing, why are we forcing additional costs upon the operation when we have no idea what the intent is?

I agree. While I don't expect it to be even remotely close to the "common" use case for the API, I can think of scenarios where not having the clearing behavior for reference types could be potentially beneficial.

For example, using a List<T> as a temporary buffer for several operations that may resize it, and then at the end you can SetCount(Capacity) followed by Clear to ensure objects aren't held onto afterwards, rather than potentially having multiple instances of zeroing inside the actual operation.

But the counterpoint is that this makes the API prone to memory leaks when T is a reference type... I'd argue that's grounds to mark the API with [UnsafeCallersOnly] if that existed, but at present I think this is a question of "how unsafe is too unsafe for CollectionsMarshal".

I personally think that if it's appropriately documented, it should be fine to not have the clearing behavior (and as mentioned, it's expected that callers would understand how the API behaves before using it).

[stephentoub]

and then at the end you can SetCount(Capacity) followed by Clear to ensure objects aren't held onto afterwards

Sure. But that's just RemoveRange.

I'd argue that's grounds to mark the API with [UnsafeCallersOnly] if that existed

I expect pretty much everything on Unsafe, Marshal, MemoryMarshal, and CollectionsMarshal would be UnsafeCallersOnly if it ever exists.

[stephentoub]

In the name of making forward progress here, let's finish this PR with the behavior of supporting shrinking and clearing, and then we can have a follow-up conversation about whether to change the behavior prior to shipping. Along with this, it'd be good to structure the method in a way that optimizes for the expected case of calling this to increase the count.

[MichalPetryka]

This API should be optimized for the cases that do not require buffer resizing, with increasing being the main target. Operations that require a resize could potentially be moved into a separate non inlineable local method.

[stephentoub]

Right.

[ghost]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[MichalPetryka]

@layomia Is there any progress with this?

[MichalPetryka]

@eiriktsarpalis Since @layomia seems to be inactive, would it be fine for me to open a new PR in place of this one?

[layomia]

would it be fine for me to open a new PR in place of this one?

@MichalPetryka, go for it. Thanks.