Reduce Revision check calls via interface

[benaadams]

Can cache the first lookup

[rynowak]

😻

[rynowak]

Not to derail too much, but do we have a feel for how much benefit we're really getting from using this pattern instead of just calling Get all the time?

[benaadams]

a feel for how much benefit we're really getting from using this pattern instead of just calling Get all the time?

If you only access a feature once per request not much; but if you access it more than once it should be significant e.g. the plaintext benchmark accesses httpContext.Response 4 times

However instead of doing

if (httpContext.Request.Path.StartsWithSegments(_path, StringComparison.Ordinal))
{
    httpContext.Response.StatusCode = 200;
    httpContext.Response.ContentType = "text/plain";
    httpContext.Response.Headers["Content-Length"] = "13";
    return httpContext.Response.Body.WriteAsync(_helloWorldPayload, 0, _helloWorldPayload.Length);
}

It could do

if (httpContext.Request.Path.StartsWithSegments(_path, StringComparison.Ordinal))
{
    var response = httpContext.Response;
    response.StatusCode = 200;
    response.ContentType = "text/plain";
    response.Headers["Content-Length"] = "13";
    return response.Body.WriteAsync(_helloWorldPayload, 0, _helloWorldPayload.Length);
}

To access it only once; but users probably won't do that

[benaadams]

@rynowak aspnet/Benchmarks#84 :)

[benaadams]

My mistake, it still goes via the feature anyway even if the Reponse is cached; however the first access on StatusCode is very high

With the Get on the frame being 41% of that + casting etc

Then the lookups for the other features are very low

[rynowak]

Maybe we could make the cost of the extra lookups even lower? 😀 #617

The hope is that the cost of abstraction is low and that doing optimizations like using the feature directly aren't super beneficial.

[benaadams]

I was trying to see if it could be a compile/jit time lookup though it would only work with generics not typeof dev...benaadams:strong-typed-features

[rynowak]

We could also check cached == null here, but this might do enough already. And if we're worried about inlinability being more thorough might be bad (code size)

[benaadams]

If cached == null it couldn't update the revision count as it may have changed and other cached features may be invalid; which would mean on second use it would have to clear the cache.

[Tratcher]

This makes some assumptions about the implementation of Collection.Revision and the initial value. E.g. WebListener's Revision count starts at 0 so this code will keep executing.

[benaadams]

Resolved, but 3 nested ifs :(

[benaadams]

Hmm, but this will go wrong if revision has changed

[benaadams]

k, now is fixed

[davidfowl]

This code could do with some comments. If we had fxcop it would probably complain about the cyclomatic complexity

[Tratcher]

You'd be surprised how high that complicity limit is.

[benaadams]

Better?

[benaadams]

OSX is grumpy on restore

[benaadams]

Looks to be faster for the same number of requests

Pre

Post

[Tratcher]

[Tratcher]

Rebased and merged.