Yup that's what is lacking in your test: function call (scope changes probably meaning some heap manipulations ?) passing arguments (probably PUSH/POP not the registries fastcall way*) doing stuff then restoring previous environnement... That's what make up to the 4th event check still being faster.
And for readability it's a matter of taste IMO. I like having events in one place, but like i said im used to old WM_* message loops :P.
Quote from Jerry »
Do you think you will get the same results for a fight with, say, Al'ar and a fight with Void Reaver ? For which fight are you going to "optimize" your code ? What about performance in the other fight ?
Wanna go crazy and construct your OnEvent using loadstring to always have the best order (given the loadstring is made outside of combat or the compile time is paid back) ? That's getting funny but out of proportion :)
Oh and sorry for being off-topic :P
EDIT: One thing is clear: not separating COMBAT_LOG_EVENT_UNFILTERED (CLEU) from other events AND using table lookup / function call is a guaranteed loser against if/then/else with CLEU as first, or prove me false :P
Looking at these results, since we know the table look up is generally always going to be faster, does the addition of the function call (to perform the actual logic you want on said event) needed by the table lookup negate any of that performance? I'm not sure of a way to use the look up table without using a function call, that isn't some mish-mash of if/else and table look up. The if/else alone can be done with or without a function call, so just wondering if it gains anything in the cases where you don't make a function call compared to the table lookup that has to use it.
Just wanting to be sure of the increase before starting to tweak code =)
Did some more tests to probe that point. Short answer is that yes if chain gains some on table lookup due to the call overhead if you actually inline your event handling in the if chain. But for sufficiently complex addons table/function calls is still the way to go.
The function loop I used is this:
function fun(a)
local b=0
for j=1,10 do
b=b+j
end
end
The table lookup + function call clocks in at 6.66
The if loop with inlined code clocks in at 6.51 at the 8th if, later is slower earlier is faster. Best case (first if) is 5.09. Worst case time (last if) is 14.39 for a 50 if chain.
I.e. from this test the function call handling is considerable, roughly worth 5-6 chained ifs.
Not sure if one can easily transport this to WoW, but if the same scaling holds in-game, you'd want to handle simple parsing (less than 8ish events to look at) by if chains with inlined code, and if you need to look at more than that with tables/function calls.
Also with 8 if's an event frequency analysis may in fact help if chains. I.e. if indeed only few (8 or less event) dominate what you see as incoming events chained ifs may be the way to go for any addon. For very large parsers that may not be very readable code though.
And yes I know that this isn't addressing the callback question. It seems to me that there isn't any good argument yet that callbacks will gain anything in any situation.
Small addons should just register the unfiltered combat log event and have few ifs to check for the select few events they care about. An interrupt alerter, or killing blow announcer, or any other thing that just needs one or very few events (crit yeller etc) is as simple as this:
function CombatLogEvent(_,timestamp, eventtype, srcGUID, srcName, srcFlags, dstGUID, dstName, dstFlags, ...)
if eventtype == "SPELL_INTERRUPT" then
-- Do stuff to announce the interrupt
end
end
RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED","CombatLogEvent") -- Assuming Ace3
But maybe Xin's tests will show when just having N addons do this tips in performance against the callback method for sufficiently large N, if it ever does.
So understanding this correctly (prolly not),
For about 10 or less events needed, one should implement the Parsing with an If/then/Else chain.
For more than 10 events one should use a table method execution.
The use of CBH or x/pcall()'ing is naturaly more expensive and in all cases should only be used from a external Lib application, otherwise a standard "t[k](t,...)" call is fine.
Orionshock: Almost correct. Here's the trickiness of it, there are 2 levels of it. Let's assume that
1. We are using AceEvent-3.0 as the main event handler.
When we register for COMBAT_LOG_EVENT_UNFILTERED in N addons, there is just 1 frame listening for this event in AceEvent-3.0. This one frame catches the event, and callbacks these N addons. Now here's the issue, in Ace2, this is simply an iteration over the N addons that registered for the event and pcall() them.
The issue with using pcall() was that it provided no stack trace beyond the point where pcall() was called, and this was a disaster in trying to track addon errors. An example of this was the AceComm-2.0 str_byte() error months ago which I took days and weeks to debug.
In Ace3, this is circumvented with using a wrapped xpcall() that involves using a custom errorhandler. In this approach, Ace3 uses a table lookup and 4 function calls and 2 assignments in order to do what pcall() alone did, but with the capability of generating a full stack trace should any event occur.
Because of this, I am not convinced that using AceEvent-3.0 is more efficient than having every addon simply creating their own addon frame and registering their own events to avoid the expensive wrapped xpcall().
---------------
Now the Combat Log event is just the exact same thing one level down. Now we're dealing with the same event, but it has subevents such as SPELL_INTERRUPT, etc etc. Let's assume that we create this combat log event lib and
2. We are using CBH as the CombatLog subevent callback handler.
Now lets say I am writing Recount. From a damage meter standpoint, I need every event to do my calculations. Now if I need every event, I will of course register for COMBAT_LOG_EVENT_UNFILTERED. If I need every (or most) events, then why should i even bother with using a CombatLogParser-2.4 library that just adds xpcall() overheads? I KNOW i need every subevent, using the library just adds unnecessary overhead of xpcall(). I can use my own table lookup to call the appropriate functions for each subevent, which in this case is preferable to if-then-else chains.
Let's say I am writing an addon that only needs to use SPELL_INTERRUPT, a specific subevent. If I use my own COMBAT_LOG_EVENT_UNFILTERED, then I have to check for the subevent M times for M events. If N addons use their own COMBAT_LOG_EVENT_UNFILTERED, then there are going to be N*M if-checks. It is obvious here that if these N addons registered to a central CombatLogParser-2.4, the central parser only needs to do M tablelookups and callbacks the appropriate addon listening to it in a wrapped xpcall().
The question is, when does "N*M if-checks" become better than "M tablelookups + N wrapped xpcall()"? The answer becomes a matter of how often these events occur, when the overhead of all these extra xpcalls() isn't worth just implementing your own tablelookup without the xpcall().
At the end of it, I believe that each addon author should just be responsible for his/her own addon and avoid using libraries where possible. There are minor gains from using a central library, but these gains are very minimal, and in fact negate any advantage if other addon authors decide to use said library when they need every event and incur unnecessary overhead just because it offers easier coding and maintainability for the parselib to callback your function instead of implementing your own solution.
-----------------------------
Without doing any real tests in the WoW environment, it is nearly impossible to tell which is better in a partial situation. The trickiness of it is compounded by trying to measure CPU performance.
Were there any tests that proved in the first place that having 1 central frame receiving events (and doing 20 pcalls) was better than having 20 frames listening for their own events? Of course, I'm assuming here that each of these 20 things are doing meaningful stuff, and not a useless if-check that fails and returns immediately.
Things like this, is what I mean by over-libifying things. People nowadays are creating libs for every small thing unnecessarily. I do understand that AceEvent's API does offer a standard for inter-addon communication, as well as slightly neater code.
I'm curious why you couldn't do the iteration of the event registy instide the xpcall? Sure you'd have to keep track which handlers have been called for a given event, so in case of an error you can rerun the iteration code which would make the iteration code "idempotent" (I hate that word btw)
example:
function :HandleEvent(event) <-- call all the event handlers
local called_handlers = {} <-- local table so that we can handle recursive events
while not xpcall( IterateRegistry , err, event, called_handlers) do end
end
function IterateRegistry(event, called_handlers)
for _, v in pairs(registry)
called_handlers[v] = true <-- mark this handler as having been called
v(...) <-- Call it
end
end
I'm curious why you couldn't do the iteration of the event registy instide the xpcall? Sure you'd have to keep track which handlers have been called for a given event, so in case of an error you can rerun the iteration code which would make the iteration code "idempotent" (I hate that word btw)
If you use this approach, then there is no performance benefit to using AceEvent at all if I have to check and keep track of which events have already run - bearing in mind that events can trigger events (meaning every event needs its own table to keep track of this status). Might as well use my own frame. Avoid the pcall() and xpcall() totally. A frame is only a few hundred bytes anyway.
An example of an event triggering an event is, event X occurs, in the event handler, you do a SetMapToCurrentZone(). Calling that function triggers WORLD_MAP_UPDATE by the wow client, and everything associated with that event and their event handlers complete first, before returning to execute the next line after SetMapToCurrentZone().
If you use this approach, then there is no performance benefit to using AceEvent at all if I have to check and keep track of which events have already run - bearing in mind that events can trigger events (meaning every event needs its own table to keep track of this status). Might as well use my own frame. Avoid the pcall() and xpcall() totally. A frame is only a few hundred bytes anyway.
An example of an event triggering an event is, event X occurs, in the event handler, you do a SetMapToCurrentZone(). Calling that function triggers WORLD_MAP_UPDATE by the wow client, and everything associated with that event and their event handlers complete first, before returning to execute the next line after SetMapToCurrentZone().
I didnt think about recursion - good point, ill edit it
You have some good points. On balance though, I'm glad the xpcall is there.
I think the gist was that 20 addons running private OnEvent and doing up to 20 if's each would be 400 ifs + 20 OnEvent calls for every event - even if it is not handled at all by any of the addons. The AceEvent cost occurs only on events which are actually handled.
I think also that the OnEvent calls have a higher than normal cost since the old arg1 arg2 need to be set up every time OnEvent is called (I don't know if this is true - just speculating)
if ace3 creates such an overhead just to make it easier to debug errors wouldn't it make sense to create some kind of debug mode ?
so as long as everything goes fine it runs in a light mode with smallest overhead possible with the possibility to enable some kind of debug mode in case of errors ?
i believe the lvl of debug introduced was necessary due to the fact that the masses where reporting errors and that the uselessness of the previous method would create more headache than solve. and that idea is off topic. :P
i believe the lvl of debug introduced was necessary due to the fact that the masses where reporting errors and that the uselessness of the previous method would create more headache than solve. and that idea is off topic. :P
I agree. The overhead isn't all THAT much ulitmately. And any real software system contains debug code in it that will allow in-the-field troubleshooting. This code isnt even really debug code - its more about robustness than anything else.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
And for readability it's a matter of taste IMO. I like having events in one place, but like i said im used to old WM_* message loops :P.
Wanna go crazy and construct your OnEvent using loadstring to always have the best order (given the loadstring is made outside of combat or the compile time is paid back) ? That's getting funny but out of proportion :)
Oh and sorry for being off-topic :P
EDIT: One thing is clear: not separating COMBAT_LOG_EVENT_UNFILTERED (CLEU) from other events AND using table lookup / function call is a guaranteed loser against if/then/else with CLEU as first, or prove me false :P
http://www.lua.org/pil/6.3.html
Tail calls does not remove the cost of CALL, but may remove the associated RET cost.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumAs long as the combat sub-event handlers are your code, you don't need to use pcall/xpcall. The code in the first post will do that just fine.
Did some more tests to probe that point. Short answer is that yes if chain gains some on table lookup due to the call overhead if you actually inline your event handling in the if chain. But for sufficiently complex addons table/function calls is still the way to go.
The function loop I used is this:
The table lookup + function call clocks in at 6.66
The if loop with inlined code clocks in at 6.51 at the 8th if, later is slower earlier is faster. Best case (first if) is 5.09. Worst case time (last if) is 14.39 for a 50 if chain.
I.e. from this test the function call handling is considerable, roughly worth 5-6 chained ifs.
Not sure if one can easily transport this to WoW, but if the same scaling holds in-game, you'd want to handle simple parsing (less than 8ish events to look at) by if chains with inlined code, and if you need to look at more than that with tables/function calls.
Also with 8 if's an event frequency analysis may in fact help if chains. I.e. if indeed only few (8 or less event) dominate what you see as incoming events chained ifs may be the way to go for any addon. For very large parsers that may not be very readable code though.
And yes I know that this isn't addressing the callback question. It seems to me that there isn't any good argument yet that callbacks will gain anything in any situation.
Small addons should just register the unfiltered combat log event and have few ifs to check for the select few events they care about. An interrupt alerter, or killing blow announcer, or any other thing that just needs one or very few events (crit yeller etc) is as simple as this:
function CombatLogEvent(_,timestamp, eventtype, srcGUID, srcName, srcFlags, dstGUID, dstName, dstFlags, ...) if eventtype == "SPELL_INTERRUPT" then -- Do stuff to announce the interrupt end end RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED","CombatLogEvent") -- Assuming Ace3But maybe Xin's tests will show when just having N addons do this tips in performance against the callback method for sufficiently large N, if it ever does.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumSo understanding this correctly (prolly not),
For about 10 or less events needed, one should implement the Parsing with an If/then/Else chain.
For more than 10 events one should use a table method execution.
The use of CBH or x/pcall()'ing is naturaly more expensive and in all cases should only be used from a external Lib application, otherwise a standard "t[k](t,...)" call is fine.
Correct?
-
View User Profile
-
View Posts
-
Send Message
Curse Premium1. We are using AceEvent-3.0 as the main event handler.
When we register for COMBAT_LOG_EVENT_UNFILTERED in N addons, there is just 1 frame listening for this event in AceEvent-3.0. This one frame catches the event, and callbacks these N addons. Now here's the issue, in Ace2, this is simply an iteration over the N addons that registered for the event and pcall() them.
The issue with using pcall() was that it provided no stack trace beyond the point where pcall() was called, and this was a disaster in trying to track addon errors. An example of this was the AceComm-2.0 str_byte() error months ago which I took days and weeks to debug.
In Ace3, this is circumvented with using a wrapped xpcall() that involves using a custom errorhandler. In this approach, Ace3 uses a table lookup and 4 function calls and 2 assignments in order to do what pcall() alone did, but with the capability of generating a full stack trace should any event occur.
Because of this, I am not convinced that using AceEvent-3.0 is more efficient than having every addon simply creating their own addon frame and registering their own events to avoid the expensive wrapped xpcall().
---------------
Now the Combat Log event is just the exact same thing one level down. Now we're dealing with the same event, but it has subevents such as SPELL_INTERRUPT, etc etc. Let's assume that we create this combat log event lib and
2. We are using CBH as the CombatLog subevent callback handler.
Now lets say I am writing Recount. From a damage meter standpoint, I need every event to do my calculations. Now if I need every event, I will of course register for COMBAT_LOG_EVENT_UNFILTERED. If I need every (or most) events, then why should i even bother with using a CombatLogParser-2.4 library that just adds xpcall() overheads? I KNOW i need every subevent, using the library just adds unnecessary overhead of xpcall(). I can use my own table lookup to call the appropriate functions for each subevent, which in this case is preferable to if-then-else chains.
Let's say I am writing an addon that only needs to use SPELL_INTERRUPT, a specific subevent. If I use my own COMBAT_LOG_EVENT_UNFILTERED, then I have to check for the subevent M times for M events. If N addons use their own COMBAT_LOG_EVENT_UNFILTERED, then there are going to be N*M if-checks. It is obvious here that if these N addons registered to a central CombatLogParser-2.4, the central parser only needs to do M tablelookups and callbacks the appropriate addon listening to it in a wrapped xpcall().
The question is, when does "N*M if-checks" become better than "M tablelookups + N wrapped xpcall()"? The answer becomes a matter of how often these events occur, when the overhead of all these extra xpcalls() isn't worth just implementing your own tablelookup without the xpcall().
At the end of it, I believe that each addon author should just be responsible for his/her own addon and avoid using libraries where possible. There are minor gains from using a central library, but these gains are very minimal, and in fact negate any advantage if other addon authors decide to use said library when they need every event and incur unnecessary overhead just because it offers easier coding and maintainability for the parselib to callback your function instead of implementing your own solution.
-----------------------------
Without doing any real tests in the WoW environment, it is nearly impossible to tell which is better in a partial situation. The trickiness of it is compounded by trying to measure CPU performance.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumWere there any tests that proved in the first place that having 1 central frame receiving events (and doing 20 pcalls) was better than having 20 frames listening for their own events? Of course, I'm assuming here that each of these 20 things are doing meaningful stuff, and not a useless if-check that fails and returns immediately.
Things like this, is what I mean by over-libifying things. People nowadays are creating libs for every small thing unnecessarily. I do understand that AceEvent's API does offer a standard for inter-addon communication, as well as slightly neater code.
-
View User Profile
-
View Posts
-
Send Message
Curse Premiumexample:
function :HandleEvent(event) <-- call all the event handlers
local called_handlers = {} <-- local table so that we can handle recursive events
while not xpcall( IterateRegistry , err, event, called_handlers) do end
end
function IterateRegistry(event, called_handlers)
for _, v in pairs(registry)
called_handlers[v] = true <-- mark this handler as having been called
v(...) <-- Call it
end
end
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumIf you use this approach, then there is no performance benefit to using AceEvent at all if I have to check and keep track of which events have already run - bearing in mind that events can trigger events (meaning every event needs its own table to keep track of this status). Might as well use my own frame. Avoid the pcall() and xpcall() totally. A frame is only a few hundred bytes anyway.
An example of an event triggering an event is, event X occurs, in the event handler, you do a SetMapToCurrentZone(). Calling that function triggers WORLD_MAP_UPDATE by the wow client, and everything associated with that event and their event handlers complete first, before returning to execute the next line after SetMapToCurrentZone().
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumI didnt think about recursion - good point, ill edit it
You have some good points. On balance though, I'm glad the xpcall is there.
I think the gist was that 20 addons running private OnEvent and doing up to 20 if's each would be 400 ifs + 20 OnEvent calls for every event - even if it is not handled at all by any of the addons. The AceEvent cost occurs only on events which are actually handled.
I think also that the OnEvent calls have a higher than normal cost since the old arg1 arg2 need to be set up every time OnEvent is called (I don't know if this is true - just speculating)
so as long as everything goes fine it runs in a light mode with smallest overhead possible with the possibility to enable some kind of debug mode in case of errors ?
-
View User Profile
-
View Posts
-
Send Message
Curse Premium-
View User Profile
-
View Posts
-
Send Message
Curse PremiumI agree. The overhead isn't all THAT much ulitmately. And any real software system contains debug code in it that will allow in-the-field troubleshooting. This code isnt even really debug code - its more about robustness than anything else.