vtable caching

[damageboy]

When generating managed wrappers for pure virtual functions the generated code ALWAYS "calculated" the pointer for the slot and generates a delegate wrapper (using Marshal.GetDelegateForFunctionPointer) for every single call... (in CSharpSources.cs)

Unless I'm completely out of my depth, this is a rather expensive call that could/should be cached when the wrapper object is new'd up...

Is there a reason for this weird behavior?

Would a pull request adding a caching layer be welcomed?

If so, should the existing behavior be made optional? default?

I would appreciate any sort of feedback, esp. a one explaining to me if/how I'm not grasping the situation...

[ddobrev]

@damageboy the fields for thunks and v-tables are static and are checked for null so they are only initialised once per type, not per object. In case I misunderstood, please reopen this issue.

[damageboy]

I'm probably doing a very pool job of explaining myself, so I'll try with an example:

For this abbreviated header:

class MDAPI_API DFITCMdApi
{
  public:
        virtual int ReqUserLogin(struct DFITCUserLoginField * pReqUserLoginField) = 0;
};//end of MDAPI_API

I get a generated .cs that looks like this for the same method:

            public override int ReqUserLogin(global::Xspeed.DFITCUserLoginField pReqUserLoginField)
            {
                var __slot = *(void**) ((IntPtr) __OriginalVTables[0] + 6 * 8);
                var ___ReqUserLogin_0Delegate = (global::Xspeed.Delegates.Func_int_IntPtr_IntPtr) Marshal.GetDelegateForFunctionPointer(new IntPtr(__slot), typeof(global::Xspeed.Delegates.Func_int_IntPtr_IntPtr));
                var ____arg0 = pReqUserLoginField.__Instance;
                var __arg0 = new global::System.IntPtr(&____arg0);
                var __ret = ___ReqUserLogin_0Delegate((__Instance + __PointerAdjustment), __arg0);
                return __ret;
            }

As you can see there is a lot of (what I think is) redundancy here: as the __slot and ___ReqUserLogin_0Delegate variables (they are NOT members / static / cached) are calculated from scratch for every call...

I might be messing up somewhere else, but the generated code seems pretty clear cut to me.

I don't think I can reopen the issue if you closed it, so hopefully you get this comment :)

[ddobrev]

I understand now, you mean caching across multiple calls of the same method. You cannot cache beyond this because the corresponding function pointers are located at different offsets. You also cannot cache using static fields because __OriginalVTables is an instance field. This means that each instance would grow by 4 or 8 bytes per virtual method in exchange for the supposed benefit in performance. So I have to ask you, have you measured the speed in each case and can you therefore reliably claim that caching the delegates is significantly faster?

[damageboy]

Thanks for replying so promptly...

That's exactly the sort of "improvement" I was planning to discuss about..

I just want to know before I start measuring and bench-marking if I'm missing some obvious options.TurnOnVTableCachingYouDimWit sort of solution...

I'll start by benchmarking some hand tuned code for this to see how it goes...

Additionally, in my mind the caching doesn't have to add that many instance fields to each instance, but rather prepare a shadow-vtable-delegate array of delegates once per vtable pointer value at runtime.
So in theory, if all delegates are prepared we simply replace the current vtable pointer with the shadow (or at worst add another single pointer per instance...)

There's definitely more tricky code around this the way I envision it, but except for much more complex accounting code it should provide very little additional performance/memory trade-off...

But as you said, first thing's first, some micro-benchmarking is in order...

Again thanks for the time, I would appreciate if you keep this issue open while I go and work on some benchmarking code...

[tritao]

Hey @damageboy.

We're very welcome to see improvements done to this area, I don't think we've ever benchmarked this code and there's probably easy performance gains to be realized. I like your idea of keeping a shadow array with the cached delegates. 👍