Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Make IFeatureCollection have generic<T> accessor #522

Closed
benaadams opened this issue Dec 26, 2015 · 8 comments
Closed

Make IFeatureCollection have generic<T> accessor #522

benaadams opened this issue Dec 26, 2015 · 8 comments
Assignees
Milestone

Comments

@benaadams
Copy link
Contributor

Rather than have it as an Extension; so the type of checks in Frame.FastFeatureGet(Type key) becomes typeof checks of T in Frame.FastFeatureGet<T>()

At that point they also become compile time constants meaning 90% of the time the entire method will be Jitted away - as per the typeof checks in System.Numerics.Vectors

@benaadams
Copy link
Contributor Author

Also the runtime typeof rather than compile time typeof is an expensive beast

runtime-typeof

As its 84% of "own time" cost of the Get call on FeatureCollection is only this line

(TFeature)features[typeof(TFeature)]

Which would become no cast or runtime typeof for primary features and only the typeof lookup would be needed for extra features.

@martinbliss
Copy link

+1 for education, convenience, and extending GOOD patterns and practices.

@benaadams
Copy link
Contributor Author

Updated results and methods

https://gist.github.com/benaadams/c6c109982f50c6f69af2#gistcomment-1657847

Method AvrTime StdDev op/s improve
NoBranchingCast (baseline) 26.9415 ns 0.5009 ns 37,129,998.18 N/A
GetGenericExtensionCastInline 27.3106 ns 0.0365 ns 36,615,855.50 +441 %
NoBranchingAsCast (baseline) 27.7353 ns 0.0483 ns 36,055,296.46 N/A
GetGenericClassWrapped 50.6726 ns 0.8665 ns 19,740,174.53 +192 %
GetGenericExtensionCast 58.9959 ns 0.0298 ns 16,950,345.39 +150 %
GetGenericStructWrapped 63.5347 ns 0.0668 ns 15,739,445.84 +132 %
GetGenericCast 97.1922 ns 0.1434 ns 10,288,916.32 +52 %
GetGenericStructUnwrap 97.9715 ns 1.6985 ns 10,210,053.51 +51 %
GetGenericClassUnwrap 98.6284 ns 0.1372 ns 10,139,086.32 +50 %
GetTypeOfExtensionCast (current) 147.9895 ns 2.6713 ns 6,759,391.15 -
GetGenericAs 162.8011 ns 2.8732 ns 6,144,292.82 -9 %
GetGenericExtenstionAs 174.7144 ns 0.1204 ns 5,723,630.47 -15 %
GetTypeOfExtensionAs 221.3632 ns 6.3888 ns 4,520,834.53 -33 %

@benaadams
Copy link
Contributor Author

Actually, might be able to work round this with Frame

@halter73
Copy link
Member

@benaadams So where did you land on this? If we we change IFeatureCollection so we can pass TFeature as a generic parameter all the way down to FastFeatureGet in Kestrel where the if statements can look like this:

        if (typeof(TFeature) == typeof(global::Microsoft.AspNet.Http.Features.IHttpRequestFeature))

Will the JIT compiler perform branch elimination despite TFeatures all being reference types and the method being accessed through the IFeatureCollection interface? Do we do it anyway so we can avoid the creation of a runtime type object?

@benaadams
Copy link
Contributor Author

Still working on it; the jit does do the branch elimination if the method is extracted locally rather than a call (i.e. inlined) regardless of them being reference types.

However; it won't inline via an interface. Nor will it inline a virtual method (so subclassing won't work either). Can't use a value type with a generic as it makes the value type bit concrete and leaves the generic bit shared.

Will add going via an interface to the tests and see if generics are still faster. Also have an idea on how to get the branch elimination working still; but it would mean creating a specific valuetype wrapper for each "named" feature.

@benaadams
Copy link
Contributor Author

The changed interface implementation of FeatureCollection seems to cause the Jit to change inlining, and no longer inline the agressive inline method :-/

Method AvrTime StdDev op/s
GetConcreteStructWrapped 7.517 ns 0.1262 ns 133,054,091.49 2263.0%
NoBranchingCast (baseline) 29.243 ns 0.2302 ns 34,198,218.52 507.3%
NoBranchingAsCast (baseline) 32.200 ns 0.1380 ns 31,056,481.25 451.5%
INoBranchingCast (baseline) 39.227 ns 0.4047 ns 25,494,653.98 352.8%
IGetConcreteStructWrapped 55.137 ns 0.2007 ns 18,136,746.06 222.1%
GetGenericStructWrapped 76.540 ns 1.2237 ns 13,068,333.88 132.1%
GetGenericCast 97.884 ns 1.7258 ns 10,219,152.21 81.5%
GetGenericStructUnwrap 103.899 ns 1.9795 ns 9,627,983.16 71.0%
GetGenericExtensionCastInline 109.877 ns 1.8892 ns 9,103,677.66 61.7%
GetGenericExtensionCast 111.273 ns 0.3054 ns 8,986,912.43 59.6%
IGetGenericStructWrapped 123.614 ns 2.0920 ns 8,091,910.21 43.7%
IGetGenericCast 156.467 ns 0.3672 ns 6,391,129.96 13.5%
GetGenericAs 158.727 ns 2.6942 ns 6,301,874.13 11.9%
IGetGenericStructUnwrap 160.752 ns 2.8691 ns 6,222,654.29 10.5%
IGetGenericExtensionCastInline 164.497 ns 4.6456 ns 6,083,670.51 8.0%
IGetGenericExtensionCast 167.306 ns 4.3638 ns 5,980,989.43 6.2%
GetTypeOfExtensionCast 171.157 ns 2.9481 ns 5,844,228.66 3.8%
GetGenericExtenstionAs 174.937 ns 0.5404 ns 5,716,391.21 1.5%
IGetTypeOfExtensionCast (current) 178.272 ns 12.2992 ns 5,630,807.91 --
GetTypeOfExtensionAs 239.155 ns 2.6341 ns 4,181,875.96 -25.7%

Fastest now being IGetConcreteStructWrapped which causes a concrete implementation of the generic function rather than a shared method.

Generic's via the interface without extension (IGetGenericCast) are 13.5% faster. Wrapping in a non-generic struct via interface is 222.1% faster; think can get there via the generic interface method.

@benaadams
Copy link
Contributor Author

Hmm... no, as you'd need to lookup the concrete struct to use, which would put you back in the original situation :-/

Though using a GenericStruct as implementation in FastFeatureGet<TFeature> (IGetGenericStructWrapped) is 43.7% faster; so maybe that could work in the interim? (Must be some why to get to the other)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants