LibUnitID is a framework-independent library that provides consolidated change event callbacks and identity caching for all of the event-trackable UnitIDs.
The goal is for no other library or addon to have to worry about event registration for the basic (event-trackable) UnitIDs: "target", "focustarget", "party3", "raidpet22target", etc. Any time any of these UnitIDs change their identity, LibUnitID provides a callback. This includes "cascade" changes: for example, UNIT_PET("party3") indicates that "partypet3" has changed, which probably also means that "partypet3target" has changed. LibUnitID detects this automatically and notifies any callbacks that are registered for any affected UnitID.
In addition to (and as a side effect of) tracking UnitID identities and issuing callbacks, LibUnitID caches the GUID associated with each UnitID. Querying this information from LibUnitID's cache takes slightly less than half the time as calling the built-in UnitGUID() API function, while maintaining this cache takes slightly more than twice as long per UnitID as a single UnitGUID() call. This means that if UnitGUID() is called for the same UnitID more than ~5 times per actual identity change, the caching mechanism yields a net performance gain. While tracking every supported UnitID in a 5-person instance run, informal testing has shown upwards of 100x as many UnitGUID() calls as cache updates -- well beyond the break-even point.
While tracking every supported UnitID in a 5-person instance run, informal testing has shown upwards of 100x as many UnitGUID() calls as cache updates -- well beyond the break-even point.
Can you provide details about this informal testing? What addons were used/tested?
In a 5-person instance run, unless a pet dies and resurrects very often, there should be almost no unitIDs that change apart from the targets of each person which can be tracked directly via UNIT_TARGET.
Can you provide details about this informal testing? What addons were used/tested?
In a 5-person instance run, unless a pet dies and resurrects very often, there should be almost no unitIDs that change apart from the targets of each person which can be tracked directly via UNIT_TARGET.
This is true -- actual cache updates (even in a 25 person raid) are fairly low, and cache tests (where an event indicated a possible change, but there wasn't an actual change) aren't that much higher. And yet, UnitGUID() calls tend to exceed both of these by at least an order of magnitude.
I've refined the logging and analysis tools in the standalone version of the library (rev.9) to provide more specific and precise data. I'll post more formal test results shortly.
In the mean time, you're welcome to try to replicate my testing and post your own results. Just install the standalone library and start the analysis like so:
/libunitid log on
/libunitid monitor on
These will set logging hooks on the native UnitGUID() function, and make LibUnitID monitor every unitid it supports, so that it has to do the maximum work to maintain its cache. In real usage, the maintenance work would probably be less, since every single supported unitid might not be tracaked. With logging and monitoring active, play for awhile, do a run, whatever. Then:
/libunitid time
/libunitid report
The first will lag for a moment while it computes timing data for various types of calls (you can repeat this to refine the timing over more loops), and the second will analyze that timing data and the logging data to report usage and efficiency.
I don't recommend having the standalone library enabled unless you intend to do such tests, however, as the logging hooks will degrade performance a bit even if logging is disabled (since the hook still has to relay to the logged function).
In the end, though, LibUnitID isn't about performance as much as convenience. The performance gain is a nice bonus, but the point is to save authors from re-inventing all the logic of monitoring when UnitIDs change or keeping track of what they point to, especially if they need to maintain their own cached data per unit.
I still don't see much benefit from this library if addons are only calling UnitGUID() when they need to on the events that it could change.
But that assumption is not correct. Please test it. Install the standalone library, run the monitoring, do an instance, and look at the results. Compare the cachable API UnitGUID() calls to the library's cache tests and updates.
Furthermore, as I've told you specifically at least half a dozen times in IRC already, the primary purpose of this library is not performance, but convenience. The performance gain is only a minor bonus, although a measurable one.
If you don't find it convenient, great, don't use it; some other folks might be interested.
Fine then if its convenience only then I will have to say this does not belong on WoWAce and will ask one of the others to move the project to CurseForge for you.
Fine then if its convenience only then I will have to say this does not belong on WoWAce and will ask one of the others to move the project to CurseForge for you.
I see a great many projects and libraries on WoWAce whose primary purpose is not performance. I assume all (good) libraries consider performance, but it seems to me that many (good) libraries exist for the purpose of saving author effort and re-using code. This one is no different.
And I found your nasty PM rather insulting.
If you're going to respond to my private conversations in public, it is only fair to do so in context. The message I sent you was:
I understand that you're not interested in using LibUnitID. That's fine, but please try to keep an open mind.
I don't go posting in your threads about the uselessness of your work just because I don't personally want to use it. Your apparent insistence on finding any possible objection is frankly a little irritating, especially since you seem to simply repeat the same initial "not useful" conclusion without addressing or even showing any signs of considering my explanations.
It seems to me like you're just spreading FUD.
I didn't think it was nasty; I thought that was fairly civil and respectful, given the situation. I thought it would be more appropriate to address this in private with you, and if you agree, I'll happily remove this post. But please don't take pot shots in public based on comments I made in private, without offering the context. I find that rather insulting.
And the WowAce of now is not the WowAce of the past that did not have any approval on new libraries. Almost every single one of the veteran developers will agree that many of the things of the past were bad and wrong, including libraries which soul purpose was to cater to lazy authors that couldn't do simple things right on their own.
And it is my duty to insist on finding any possible objection regardless if I'll ever use it or not. And still my initial conclusion of 'not useful' stands which is why I've repeated it. Show some hard evidence other than that your UnitGUID function call is faster than the API one. I can tell by looking at your code that you do not know what you are doing so I am being critical about it. I have looked at your standalone code and can tell you right now that it does not do what you think it does, it makes almost no sense at all.
Arrow doesn't know how to spell constructive :)
Also, he only speaks for himself usually.
But he is right about evaluating new libraries more closely then addons.
However, back on topic.
One thing that puzzles me, you say its more for convenience then speed, but what could be more convenient then just calling UnitGUID directly? :)
The convenience factor is biggest when you need to react to changes in second- or third-tier unitids. What I mean by that is, "party3" only changes on PARTY_MEMBERS_CHANGED, easy enough to monitor. "party3target" might change on UNIT_TARGET(party3), but it might also change on PARTY_MEMBERS_CHANGED if "party3" changes, in which case you might not get UNIT_TARGET(party3). "partypet3target" might change on UNIT_TARGET(partypet3), or UNIT_PET(party3), or PARTY_MEMBERS_CHANGED. Handling the multiple events that all might signal a change in your one unitid is work to code; calling "lib:RegisterUnit('partypet3target', mycallback)" is less work.
The way addons seem to handle this now is, on an event like P_M_C they'll re-process every party member, and their target, and their pet, and their pet's target, just in case any of them might have changed. But they do this even if the party member didn't actually change, and therefore the derived unitids didn't change either. Multiply that by several addons that need to track unitid identities (unit frames, threat meters, damage meters, etc), and multiply again for raid-based unitids instead of just party, and the effect becomes measurable.
My initial motivation for this library was aura caching, because UnitAura() seems to suffer from the same calling-into-C penalty. In that context, the cost incurred on unnecessary unitid-identity-change processing isn't just the one UnitGUID(), but potentially dozens of UnitAura() calls. Any other addon or library that maintains non-trivial per-guid data will have the same issue.
So, the argument for this library is both potential convenience and potential performance, depending on your situation. I readily grant that it is not suitable for every purpose, but I do believe it's suitable for some purposes. If you folks really think it's worthless, fine, delete it. I developed it for my own need and offered that work to the community, so in my view you'll only be depriving the community of a resource, but so be it.
I've responded to you in private Arrow; I don't think that conversation concerns the community and I don't want to further derail this thread.
Arrow doesn't know how to spell constructive :)
Also, he only speaks for himself usually.
I think bullying isn't OK and mods should try to keep the place safe especially for lesser experienced authors. That has nothing to do if one speaks or a group speaks.
But he is right about evaluating new libraries more closely then addons.
What does that mean? And why is arrow allowed to basically intimidate other authors with that notion?
However, back on topic.
Feel free to move my questions to a new thread, this thread should really just go on.
As for convenience: He has all the relevant events canned. I'm pretty sure that's the convenience he's talking about. Any addon that needs select Unit-change based GUID updates can change from registering multiple events to registering one callback that gives exactly the functionality that ultimately is the goal.
Seriously this author gets a rough ride here. He provides benchmarking in his library for everyone to try. He does make a case for a minor performance gain and a case for code reuse and a case for access convenience. Noone has come out and showed that his claims are factually wrong, instead he is repeatedly asked to justify himself with a not clearly defined benchmark in the backdrop.
His statements are oversimplified into a "it's for convenience" and OP simply doesn't know that there are prevailing cultures that only accepts utility and has a preference for KISS and that he would have to argue differently to stand a ground. Certainly code reuse, while naively a fair point doesn't have a lot of value with some people here and convenience even less. But this is preference and coding philosophy rather than hard fact of what is desirable.
The case that he should make stronger is performance gain. I have not verified this because his argument is credible.
Furthermore this lib does encode the right practice when to grab UnitGUIDs (in fact only once when a relevant event is fired) and in this case is as good as the best practice addon will achieve. In fact the author's claim is correct that if multiple addons want to get on event UnitGUID updates it will cut down on the actual API calls, hence optimize. In practice I would expect the following addons to do that kind of thing (threatmeter, raid frames, damage meter, boss mod). Hence you get an optimization there.
Some coding style/practices that could be improved, but this is certainly no reason to dismiss the lib idea. Overall the lib is well documented and may appear larger than it is due to included testing function as well.
I personally do not see much use for OPs lib because the performance gain is likely too small for me, but that doesn't mean that such a lib shouldn't exist.
As with any lib, authors have to exercise judgements whether they want to use them or not anyway.
Edit: Well author gave his reasons. If this indeed gets deleted/moved I'd really like to hear the rational and I would like to hear why that's appropriate for a community that I thought was meant "for developers", which I think should be about learning, exchanging, sharing and friendly debates.
Edit2: Fixed typos.
What does that mean? And why is arrow allowed to basically intimidate other authors with that notion?
He isn't.
To the meaning of that policy, we evaluate new libraries usually on a small set of factors, including, but not limited to:
- Quality of code
We are not too hard about this. But we still look at the code if the dev at least understands his own code, and doesn't produce extreme spaghetti code. However we don't just delete projects that do not fit this criteria instantly, we encourage authors to improve it and offer suggestions.
- Documentation
We do require luadoc or some equivalent for all new libraries, as well as a proper description how/why it is used/useful.
- (relative) Usefulness to the community
We don't want another AlarLib that is only good for one addon. If you can explain the use of your library, and care enough to do so, you usually qualify already.
On libraries, we also usually have more then one guy looking at it, before anything is done, so that we try to stay objective. We should definitely post the guidelines for addons and libraries in an article somewhere. I'll talk with Kaelten again about writing something.
Back to the topic:
Okay i see now, at first i didn't exactly get the whole "Update Notification" dealy and thought you were only going to use it to cache UnitGUID calls.
I can see those notification callbacks being useful for some addons, instead of polling UnitGUID all over.
From what i see in the code, the cache updating seems to cover all cases too, and the idea behind it is sound.
I don't appreciate new authors being berated however. It is important that we, as mods strive to be impartial and practical.
Something that is hard to do at times.
I value Arrowmaster's contributions to the community, but at times he has a hard time remembering to have some tact. It's perfectly possible to speak plainly and be even keeled. It's not easy and I know at times I've either been too indirect or too blunt as well.
There is sound logic for this mod, and even if it does not get much in the way of usage there is no reason not to congratulate the author on his thought process, the idea in general, and the level of documentation he put into it. I'm shamed to admit that in many cases we've failed to document our own projects as well.
It's welcomed here, as is he. New authors have always been welcomed here and always will be. My apologies to taleden for the poor reception.
As for acceptance of new libraries, I'll work with nev and we'll work out something spelled out. But yes, we don't want harmful or obscene libraries hosted on wowace. But this is neither, and again I apologize for the issues.
These were generated with rev.13 of !LibUnitID, which includes built-in performance analysis functions. To replicate the test, install the standalone library addon and type:
/libunitid monitor on -- make the library do maximum upkeep work
/libunitid log on -- make the library log its work, and also log the native UnitGUID()
-- do some stuff
/libunitid time -- generate timing data for your machine (repeat to refine)
/libunitid report all -- show call log results, including the uncachable unitids that were used
/libunitid analyze -- calculate time spent on various functions, overall efficiency, and potential efficiency
The analysis has two components: call logging and per-call timing measurements.
When logging is enabled, the core library wraps a number of its own internal functions with relays that increment a counter and then tail-call the original function. These counters are kept separately for cache tests (when an event indicates that a UnitID might have changed identity, but it actually didn't) and cache updates (when the identity actually changed, and the cache was updated and callbacks issued), because those operations have different performance impacts.
Calls to the built-in UnitGUID() function are intercepted by replacing the global function with a similar wrapper. This is only reliable if the replacement is done before any other addon loads, since most addons will do "local UnitGUID = UnitGUID", and then we can't intercept it. For these tests, I manually added an OptionalDeps line to the .toc of every one of my addons.
Timing data is generated by calling the function in question in a "for i=1,n" loop for progressively larger "n"s, until the total time is at least 0.2 seconds and we can consider it representative. An empty timing loop is run to measure the loop overhead, which is subtracted from the other timing tests.
Using these call logs and per-call timing information, we can measure the performance impact of each operation over a period of time.
In this batch, we ran a slightly short-manned heroic raid, first to Sartharion and then to Naxxramas Spider and Plague wings.
3 addons loaded (or at least processed their .toc file) before !LibUnitID, and might have bypassed the logging function:
BigWigs, !BugGrabber, LibSharedMedia-3.0
42 addons loaded after !LibUnitID, and should be included in the log:
Nanoseconds per operation:
377.995ns per API UnitGUID()
634.915ns per Cache Change Test
923.687ns per Cache Identity Update
137.860ns per Cached GUID Query
Operations in the last 1726.065s:
712659 Cachable API UnitGUID()s
16893 Uncachable API UnitGUID()s
7338 Cache Change Tests
3952 Cache Identity Updates
0 Cached GUID Queries
Analysis:
275.767s spent querying the API's UnitGUID() (712659 cachable, 16893 uncachable)
8.309s spent maintaining the library's cache (7338 tests, 3952 updates)
0.000us saved by querying the cache instead of the API (0 queries)
Overall loss is -0.481% (8.309s extra spent over the last 1726.065s)
171.135s more could have been saved if cachable UnitGUID() calls had used the cache
Base-case gain is 9.433% (162.825s could have been saved over the last 1726.065s)
Nanoseconds per operation:
363.314ns per API UnitGUID()
632.823ns per Cache Change Test
928.462ns per Cache Identity Update
140.822ns per Cached GUID Query
Operations in the last 3345.072s:
1332149 Cachable API UnitGUID()s
30718 Uncachable API UnitGUID()s
12780 Cache Change Tests
7627 Cache Identity Updates
0 Cached GUID Queries
Uncachable UnitGUID() queries:
mouseovertarget - 7467
player - 8936
targettargettarget - 6848
mouseover - 7467
Analysis:
495.149s spent querying the API's UnitGUID() (1332149 cachable, 30718 uncachable)
15.169s spent maintaining the library's cache (12780 tests, 7627 updates)
0.000us saved by querying the cache instead of the API (0 queries)
Overall loss is -0.453% (15.169s extra spent over the last 3345.072s)
296.393s more could have been saved if cachable UnitGUID() calls had used the cache
Best-case gain is 8.404% (281.224s could have been saved over the last 3345.072s)
Nanoseconds per operation:
359.017 per API UnitGUID()
629.860 per Cache Change Test
927.788 per Cache Identity Update
139.481 per Cached GUID Query
Operations in the last 2990.767s:
937967 Cachable API UnitGUID()s
27367 Uncachable API UnitGUID()s
7106 Cache Change Tests
6076 Cache Identity Updates
0 Cached GUID Queries
Uncachable UnitGUID() queries:
mouseovertarget - 6463
player - 8211
targettargettarget - 6230
mouseover - 6463
Analysis:
346.571s spent querying the API's UnitGUID() (937967 cachable, 27367 uncachable)
10.113s spent maintaining the library's cache (7106 tests, 6076 updates)
0.000us saved saved by querying the cache instead of the API (0 queries)
Overall loss is -0.338% (10.113s extra spent over the last 2990.767s)
205.917s more could have been saved if cachable UnitGUID() calls had used the cache
Best-case gain is 6.547% (195.804s could have been saved over the last 2990.767s)
These numbers indicate an even higher potential performance gain than my earlier estimates. I think this may be due to a bug that made me think my utilities were loading first, when in fact they weren't, so my earlier call log results were probably too low.
I was surprised at the number of calls to UnitGUID("player"), which I would have thought most addons would cache on their own, since it shouldn't change. It shows up here as "uncachable" because the library doesn't bother with it; given these log results, I may add that in the future.
As always, please post any questions. I've tried to make the analysis as accurate as I can, but if I've made a mistake, please let me know.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
The goal is for no other library or addon to have to worry about event registration for the basic (event-trackable) UnitIDs: "target", "focustarget", "party3", "raidpet22target", etc. Any time any of these UnitIDs change their identity, LibUnitID provides a callback. This includes "cascade" changes: for example, UNIT_PET("party3") indicates that "partypet3" has changed, which probably also means that "partypet3target" has changed. LibUnitID detects this automatically and notifies any callbacks that are registered for any affected UnitID.
In addition to (and as a side effect of) tracking UnitID identities and issuing callbacks, LibUnitID caches the GUID associated with each UnitID. Querying this information from LibUnitID's cache takes slightly less than half the time as calling the built-in UnitGUID() API function, while maintaining this cache takes slightly more than twice as long per UnitID as a single UnitGUID() call. This means that if UnitGUID() is called for the same UnitID more than ~5 times per actual identity change, the caching mechanism yields a net performance gain. While tracking every supported UnitID in a 5-person instance run, informal testing has shown upwards of 100x as many UnitGUID() calls as cache updates -- well beyond the break-even point.
Project Home: http://www.wowace.com/projects/libunitid-1-0
Getting Started: http://www.wowace.com/projects/libunitid-1-0/pages/getting-started
API: http://www.wowace.com/projects/libunitid-1-0/pages/api
Please leave any feedback in this thread, or file tickets on the project page.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumCan you provide details about this informal testing? What addons were used/tested?
In a 5-person instance run, unless a pet dies and resurrects very often, there should be almost no unitIDs that change apart from the targets of each person which can be tracked directly via UNIT_TARGET.
This is true -- actual cache updates (even in a 25 person raid) are fairly low, and cache tests (where an event indicated a possible change, but there wasn't an actual change) aren't that much higher. And yet, UnitGUID() calls tend to exceed both of these by at least an order of magnitude.
I've refined the logging and analysis tools in the standalone version of the library (rev.9) to provide more specific and precise data. I'll post more formal test results shortly.
In the mean time, you're welcome to try to replicate my testing and post your own results. Just install the standalone library and start the analysis like so:
These will set logging hooks on the native UnitGUID() function, and make LibUnitID monitor every unitid it supports, so that it has to do the maximum work to maintain its cache. In real usage, the maintenance work would probably be less, since every single supported unitid might not be tracaked. With logging and monitoring active, play for awhile, do a run, whatever. Then:
The first will lag for a moment while it computes timing data for various types of calls (you can repeat this to refine the timing over more loops), and the second will analyze that timing data and the logging data to report usage and efficiency.
I don't recommend having the standalone library enabled unless you intend to do such tests, however, as the logging hooks will degrade performance a bit even if logging is disabled (since the hook still has to relay to the logged function).
In the end, though, LibUnitID isn't about performance as much as convenience. The performance gain is a nice bonus, but the point is to save authors from re-inventing all the logic of monitoring when UnitIDs change or keeping track of what they point to, especially if they need to maintain their own cached data per unit.
But that assumption is not correct. Please test it. Install the standalone library, run the monitoring, do an instance, and look at the results. Compare the cachable API UnitGUID() calls to the library's cache tests and updates.
Furthermore, as I've told you specifically at least half a dozen times in IRC already, the primary purpose of this library is not performance, but convenience. The performance gain is only a minor bonus, although a measurable one.
If you don't find it convenient, great, don't use it; some other folks might be interested.
And I found your nasty PM rather insulting.
I see a great many projects and libraries on WoWAce whose primary purpose is not performance. I assume all (good) libraries consider performance, but it seems to me that many (good) libraries exist for the purpose of saving author effort and re-using code. This one is no different.
If you're going to respond to my private conversations in public, it is only fair to do so in context. The message I sent you was:
I didn't think it was nasty; I thought that was fairly civil and respectful, given the situation. I thought it would be more appropriate to address this in private with you, and if you agree, I'll happily remove this post. But please don't take pot shots in public based on comments I made in private, without offering the context. I find that rather insulting.
And it is my duty to insist on finding any possible objection regardless if I'll ever use it or not. And still my initial conclusion of 'not useful' stands which is why I've repeated it. Show some hard evidence other than that your UnitGUID function call is faster than the API one. I can tell by looking at your code that you do not know what you are doing so I am being critical about it. I have looked at your standalone code and can tell you right now that it does not do what you think it does, it makes almost no sense at all.
And what happened to constructive feedback rather than talking down other devs?
-
View User Profile
-
View Posts
-
Send Message
Forum ModeratorAlso, he only speaks for himself usually.
But he is right about evaluating new libraries more closely then addons.
However, back on topic.
One thing that puzzles me, you say its more for convenience then speed, but what could be more convenient then just calling UnitGUID directly? :)
The convenience factor is biggest when you need to react to changes in second- or third-tier unitids. What I mean by that is, "party3" only changes on PARTY_MEMBERS_CHANGED, easy enough to monitor. "party3target" might change on UNIT_TARGET(party3), but it might also change on PARTY_MEMBERS_CHANGED if "party3" changes, in which case you might not get UNIT_TARGET(party3). "partypet3target" might change on UNIT_TARGET(partypet3), or UNIT_PET(party3), or PARTY_MEMBERS_CHANGED. Handling the multiple events that all might signal a change in your one unitid is work to code; calling "lib:RegisterUnit('partypet3target', mycallback)" is less work.
The way addons seem to handle this now is, on an event like P_M_C they'll re-process every party member, and their target, and their pet, and their pet's target, just in case any of them might have changed. But they do this even if the party member didn't actually change, and therefore the derived unitids didn't change either. Multiply that by several addons that need to track unitid identities (unit frames, threat meters, damage meters, etc), and multiply again for raid-based unitids instead of just party, and the effect becomes measurable.
My initial motivation for this library was aura caching, because UnitAura() seems to suffer from the same calling-into-C penalty. In that context, the cost incurred on unnecessary unitid-identity-change processing isn't just the one UnitGUID(), but potentially dozens of UnitAura() calls. Any other addon or library that maintains non-trivial per-guid data will have the same issue.
So, the argument for this library is both potential convenience and potential performance, depending on your situation. I readily grant that it is not suitable for every purpose, but I do believe it's suitable for some purposes. If you folks really think it's worthless, fine, delete it. I developed it for my own need and offered that work to the community, so in my view you'll only be depriving the community of a resource, but so be it.
I've responded to you in private Arrow; I don't think that conversation concerns the community and I don't want to further derail this thread.
I think bullying isn't OK and mods should try to keep the place safe especially for lesser experienced authors. That has nothing to do if one speaks or a group speaks.
What does that mean? And why is arrow allowed to basically intimidate other authors with that notion?
Feel free to move my questions to a new thread, this thread should really just go on.
As for convenience: He has all the relevant events canned. I'm pretty sure that's the convenience he's talking about. Any addon that needs select Unit-change based GUID updates can change from registering multiple events to registering one callback that gives exactly the functionality that ultimately is the goal.
Seriously this author gets a rough ride here. He provides benchmarking in his library for everyone to try. He does make a case for a minor performance gain and a case for code reuse and a case for access convenience. Noone has come out and showed that his claims are factually wrong, instead he is repeatedly asked to justify himself with a not clearly defined benchmark in the backdrop.
His statements are oversimplified into a "it's for convenience" and OP simply doesn't know that there are prevailing cultures that only accepts utility and has a preference for KISS and that he would have to argue differently to stand a ground. Certainly code reuse, while naively a fair point doesn't have a lot of value with some people here and convenience even less. But this is preference and coding philosophy rather than hard fact of what is desirable.
The case that he should make stronger is performance gain. I have not verified this because his argument is credible.
Furthermore this lib does encode the right practice when to grab UnitGUIDs (in fact only once when a relevant event is fired) and in this case is as good as the best practice addon will achieve. In fact the author's claim is correct that if multiple addons want to get on event UnitGUID updates it will cut down on the actual API calls, hence optimize. In practice I would expect the following addons to do that kind of thing (threatmeter, raid frames, damage meter, boss mod). Hence you get an optimization there.
Some coding style/practices that could be improved, but this is certainly no reason to dismiss the lib idea. Overall the lib is well documented and may appear larger than it is due to included testing function as well.
I personally do not see much use for OPs lib because the performance gain is likely too small for me, but that doesn't mean that such a lib shouldn't exist.
As with any lib, authors have to exercise judgements whether they want to use them or not anyway.
Edit: Well author gave his reasons. If this indeed gets deleted/moved I'd really like to hear the rational and I would like to hear why that's appropriate for a community that I thought was meant "for developers", which I think should be about learning, exchanging, sharing and friendly debates.
Edit2: Fixed typos.
Because I'm not afraid to be blunt when needed.
-
View User Profile
-
View Posts
-
Send Message
Curse PremiumThere is a difference between being blunt and being rude Arrowmaster.
-
View User Profile
-
View Posts
-
Send Message
Forum ModeratorHe isn't.
To the meaning of that policy, we evaluate new libraries usually on a small set of factors, including, but not limited to:
- Quality of code
Back to the topic:
Okay i see now, at first i didn't exactly get the whole "Update Notification" dealy and thought you were only going to use it to cache UnitGUID calls.
I can see those notification callbacks being useful for some addons, instead of polling UnitGUID all over.
From what i see in the code, the cache updating seems to cover all cases too, and the idea behind it is sound.
I'll see about getting it approved
No its just a difference in how you choose to interpret it.
Something that is hard to do at times.
I value Arrowmaster's contributions to the community, but at times he has a hard time remembering to have some tact. It's perfectly possible to speak plainly and be even keeled. It's not easy and I know at times I've either been too indirect or too blunt as well.
There is sound logic for this mod, and even if it does not get much in the way of usage there is no reason not to congratulate the author on his thought process, the idea in general, and the level of documentation he put into it. I'm shamed to admit that in many cases we've failed to document our own projects as well.
It's welcomed here, as is he. New authors have always been welcomed here and always will be. My apologies to taleden for the poor reception.
As for acceptance of new libraries, I'll work with nev and we'll work out something spelled out. But yes, we don't want harmful or obscene libraries hosted on wowace. But this is neither, and again I apologize for the issues.
These were generated with rev.13 of !LibUnitID, which includes built-in performance analysis functions. To replicate the test, install the standalone library addon and type:
The analysis has two components: call logging and per-call timing measurements.
When logging is enabled, the core library wraps a number of its own internal functions with relays that increment a counter and then tail-call the original function. These counters are kept separately for cache tests (when an event indicates that a UnitID might have changed identity, but it actually didn't) and cache updates (when the identity actually changed, and the cache was updated and callbacks issued), because those operations have different performance impacts.
Calls to the built-in UnitGUID() function are intercepted by replacing the global function with a similar wrapper. This is only reliable if the replacement is done before any other addon loads, since most addons will do "local UnitGUID = UnitGUID", and then we can't intercept it. For these tests, I manually added an OptionalDeps line to the .toc of every one of my addons.
Timing data is generated by calling the function in question in a "for i=1,n" loop for progressively larger "n"s, until the total time is at least 0.2 seconds and we can consider it representative. An empty timing loop is run to measure the loop overhead, which is subtracted from the other timing tests.
Using these call logs and per-call timing information, we can measure the performance impact of each operation over a period of time.
In this batch, we ran a slightly short-manned heroic raid, first to Sartharion and then to Naxxramas Spider and Plague wings.
3 addons loaded (or at least processed their .toc file) before !LibUnitID, and might have bypassed the logging function:
42 addons loaded after !LibUnitID, and should be included in the log:
Obsidian Sanctum (Heroic) -- 23 people, 29 minutes
Naxxramas (Heroic) Spider Wing -- 21 people, 56 minutes
Naxxramas (Heroic) Plague Wing -- 20 people, 50 minutes
These numbers indicate an even higher potential performance gain than my earlier estimates. I think this may be due to a bug that made me think my utilities were loading first, when in fact they weren't, so my earlier call log results were probably too low.
I was surprised at the number of calls to UnitGUID("player"), which I would have thought most addons would cache on their own, since it shouldn't change. It shows up here as "uncachable" because the library doesn't bother with it; given these log results, I may add that in the future.
As always, please post any questions. I've tried to make the analysis as accurate as I can, but if I've made a mistake, please let me know.