vendethiel / GladiusEx

13 stars 19 forks source link

Lots of improvements #25

Closed ManneN1 closed 5 years ago

ManneN1 commented 5 years ago

Improved Interrupt Module:

Is more modular, interrupts have priority (doesn't automatically override other auras) and are automatically hidden after duration end (previously required new AURA_UNIT tick to be removed which could leave it there for some time).

Improved Efficiency of ClassIcon Module:

In accordance with Phanx comment in https://www.wowinterface.com/forums/showthread.php?t=51484 I've removed the local function inside ScanAuras() which takes a lot of time to create each time a UNIT_AURA event is received.

Minor GUI Changes:

slaren commented 5 years ago

I did some tests regarding overhead of using inner functions and what I found is that while there is some overhead creating and calling an inner function, at least in my computer it is negligible (in the order of 1.5 microsecond slower). However, using instead a call to a member function is even slower.

This is how I tested it, in case you want to test it yourself or you want to verify that my methodology is correct: https://gist.github.com/slaren/bfaa5365eff2c41fd4db1dfef49054fb In my computer, what I get is approx ~ f1: 2us / f2: 0.65us / f3: 3us

In any case, what I take from that is that the difference between the different versions is so small that it is just not worth changing it if doing so makes the code harder to understand. Thus, before changing any code to improve performance, you should at least verify that it will have a measurable impact.

vendethiel commented 5 years ago

@slaren was that tested with WoW’s lua?

@ManneN1 thanks! I know you’ve been working on that for quite a while. I know you had trinket, interrupt gui, and combat modules started too. This PR looks good.

vendethiel commented 5 years ago

I’m really not surprised a member call is slower though because of metatable lookups.

slaren commented 5 years ago

Yes, the test was ran in wow using the WowLua addon.

vendethiel commented 5 years ago

You're missing a test case however: the case with another unnested (local) function

slaren commented 5 years ago

Somehow that is even slower at 3.2us (the code is in the same gist).

ManneN1 commented 5 years ago

There could be some errors with this because the merge I did with the interrupt module (i.e. not my personal version) is not fully tested. Someone daring should test it out (a lot).

vendethiel commented 5 years ago

Sorry for the delay, I've been unsubbed since. I'll try to resub soon-ish and take a look.