UnlimitedHugs / RimworldHugsLib

A lightweight shared library for Rimworld modding.
Other
245 stars 59 forks source link

Switch ShortHash invocation to DynamicMethod #33

Closed DoctorVanGogh closed 7 years ago

DoctorVanGogh commented 7 years ago

I took a peek at the short hash invocation in HugsLib.

Current code just uses Reflection. This is by far the slowest method of code invocation. Switching this to a DynamicMethod should give orders of magnitude better performance (Quick google of available comparisons found http://www.ookii.org/Blog/reflection_vs_dynamic_code_generation).

Compilation goes through (didn't run any tests though), but I had an identical version to this code running perfectly fine in an unfinished mod (which I switched to HugsLib, where I found the simple reflection code).

UnlimitedHugs commented 7 years ago

To be fair, this method doesn't really need optimizing- it's generally called at startup, and only a few dozen times, if that. Pulling out big guns like Emit significantly increases complexity- so there needs to be a good reason for it. Still, since you asked, and I was updating anyway, I implemented another kind of optimization that's a bit easier on the eyes. Using Delegate.CreateDelegate will result in a comparable performance increase ('bout 50 times faster) without the extra complexity.

DoctorVanGogh commented 7 years ago

Technically you can never know how often this method is called - downstream mods might be creating thousands of defs/objects that need ShortHashes, but I'll admit that a typical use case will see invocations far below a hundred or so.

Shying away from DynamicMethods 'because it's the big guns' is a bit hypocritical though - from my understanding the underlying Harmony library does exactly that for it's detouring. And since HugsLibs dropped detours in favor of Harmony it kinda endorses that technique 😁 . Still, anything is better than plain reflection. If we were running .NET 4 (compiled) expressions would have been the way to go - with 3.5 DynamicMethods are the next best thing to go (performance wise). They also have the (currently unused) benefit of neatly side-stepping any potential .Net security policy issues. Since the newly created method is situated inside the declaring type it has access to any and all private fields (and methods). If Rimworld ever get's released on platforms with a more restricted execution policy (mobile???) this might be something to revisit.

And yes, if you don't know that a body of

ldarg.0
ldarg.1
call class Foo.Bar
ret

is just the generated IL code for

Foo.Bar(arg1, arg2)

then this might indeed be a bit unsightly.

So - glad you've optimized this code in the end.