Remove fast-path closures from EventBasedLRUCache

[stephentoub]

No description provided.

[kimbell]

How does this avoid a closure?
var localCacheItem = cacheItem; // avoid closure on fast path or when !_maintainLRU

The localCacheItem is still outside the delegate, I thought that would have created a closure.

[stephentoub]

How does this avoid a closure? var localCacheItem = cacheItem; // avoid closure on fast path or when !_maintainLRU

The localCacheItem is still outside the delegate, I thought that would have created a closure.

It doesn't avoid the closure all-up, but it avoids it on the fast path. If you have the code:

if (d.TryGetValue(key, out var something))
{
    return something;
}

if (whatever)
{
    Capture(() => something);
}

that will result in the C# compiler generating code along the lines of:

<>__DisplayClass_42 display = new();
if (d.TryGetValue(key, out display.something))
{
    return display.something;
}

if (whatever)
{
    Capture(new Action(display.SomeMethod));
}

but if you instead do:

if (d.TryGetValue(key, out var something))
{
    return something;
}

if (whatever)
{
    var localSomething = something;
    Capture(() => localSomething);
}

the closure moves to the scope in which localSomething is declared:

if (d.TryGetValue(key, out var something))
{
    return something;
}

if (whatever)
{
    <>__DisplayClass_42 display = new();
    display.localSomething = something;
    Capture(new Action(display.SomeMethod));
}

[kimbell]

@stephentoub Thanks, that was very interesting. Learn something new every day.
Something else I'll try to remember when reviewing my code.

[keegan-caruso]

failing tests

[stephentoub]

failing tests

Thanks, I'll take a look. Which tests? I guess CI isn't running any?

[stephentoub]

Fixed. I'd accidentally written cacheItem in one place when it should have been newCacheItem.