This added some complexity... But seemed bad taking max item count of tasks wide and waiting on them....

TL;DR

For an in‑process async cache, spawning a Task for every item is all overhead and no benefit.
Using the BatchAsync helper with a simple for loop:

• Cuts allocations from n Tasks (e.g. 50 k) to a single rented buffer.

• Eliminates thread‑pool thrash and lock contention on MemoryCache.

• Achieves essentially the same write throughput because each SetAsync finishes immediately.

Why the batched, serial approach wins

When not to stay serial

Add a semaphore‑throttled fan‑out inside the slice only if:

• SetAsync starts doing real I/O (remote Redis, file writes, HTTP).

• Each call performs heavy CPU work (e.g., compression, JSON serialisation).

Set Parallelism = Environment.ProcessorCount to keep memory bounded while still overlapping latency.

// Simple, memory‑lean batching (good for local cache)
await work.BatchAsync(1024, async slice =>
{
    var span = slice.Span;
    for (int i = 0; i < span.Length; i++)
    {
        var pair = span[i];
        await SetAsync(pair.Key, pair.Value, expiresIn)
              .ConfigureAwait(false);
    }
});

You’ll stay fast for 5‑item inputs and 50 000‑item inputs—without flooding the GC or the thread‑pool.

So now we have a duplicate method that was doing the exact same thing as the old one? I'm confused why we need both of these.

See previous comment. This one is called from the maintenance worker which happens in the background as values are possibly changing. Notice this one only updates it if the entry is marked as expired. Previously it wasn't working how we thought it was where the passed in expires was expired but the cache entry wasn't was leading to it not being removed.

I wouldn't even do this. Just warn like you are and process all of them and let SetAsync do it's job of maintaining max items limit.

It's more complicated to move this into setAsync because this is in SetAll and easier to truncate. If you move it in, it has to do a lot more lock contention and work, it's easier to just say replace all these with the max count.

This is what I'm saying to remove because I don't think it's correct to just drop some of the items you passed in and told it to set.

You are changing the behavior at a minimum where previously it would've kept the last X items as it passed them all into SetAsync. I just think in general, this isn't a real-world thing that we should be changing code for.

I can take the last x items instead of the first x items. But I think it's realistic that it's going to happen in some capacity especially if you set an in memory cache really low. Our tests were doing this..

Yes, that is test code. It's pretty useless to set a really small max items limit in the real world. So the chances that you'd pass in more items to SetAll than the max items is just not going to be a real world thing.

I don't see why it's not a bad thing to prevent any amount of work, why would you want to take on any extra locking or batching?

To me, it just seems like an extremely unlikely scenario that we are adding code for and I don't like that. That's all I'm saying. It's not the end of the world to have the code if it's still doing the correct thing, but you could definitely argue that the correct thing is to set all of the cache entries like the caller of the method would expect to happen. Right now you already changed the behavior by not using the last X items, you are also affecting things like various cache counts.

Just updated it to take the last limit of max items (last in wins).

Yeah only thing we are missing is probably incrementing some counters like _writes but we have a warning logged out..