evil-morfar / RCLootCouncil2

RCLootCouncil - addon for World of Warcraft
https://rclootcouncil.com
GNU Lesser General Public License v3.0
19 stars 29 forks source link

Limit voting frame update frequency #124

Closed SafeteeWoW closed 6 years ago

SafeteeWoW commented 6 years ago
SafeteeWoW commented 6 years ago

Gonna makes a small change

SafeteeWoW commented 6 years ago

I have done this PR. I didn't realize there are so many Update(). If you think 0.2s is too long, so I change it so maximum 1 Update() per frame.

SafeteeWoW commented 6 years ago

0.2s seems reasonable because it equals to TOOLTIP_UPDATE_TIME, the frequency the GameTooltip runs.

evil-morfar commented 6 years ago

Are there any benefits in that approach? I would prefer using AceBucket for handling it, something like:

-- OnInitialize
self:RegisterBucketMessage("RCVFUpdate", 0.2, "Update") 
-- On Response received
self:SendMessage("RCVFUpdate")
SafeteeWoW commented 6 years ago

AceBucket should not be used when the interval is short. AceBucket use ScheduleTimer. so every 0.2s we create a ScheduleTimer. ScheduleTimer is an expensive call. Blizzard SharedXML function C_Timer.NewTimer has similar feature and writes "Note that C_Timer.NewTimer is significantly more expensive than C_Timer.After and should only be used if you actually need any of its additional functionality". All Blizzard code uses "OnUpdate" handler to limit the update frequency. Overall, AceBucket may save several line, but very poor performance, but performance improvement is my goal here.

evil-morfar commented 6 years ago

But ScheduleTimer uses C_Timer.After, and unlike C_Timer.NewTicker it doesn't use metatables created on every timer, which to my understanding is what makes it expensive. AceBucket is intended to be used when lots of events are happening (they use 1 sec in their example) and it will only create a timer when there actually is an event.

Wouldn't the current code continuously do an update every 0,2s until the voting frame is hidden? Nvm, I misread. It's of course only the OnUpdate function that runs continously.

SafeteeWoW commented 6 years ago

It's still much more expensive than onUpdate event. The data structure involved in the SchedulerTimer already makes it expensive. onUpdate event is not expensive. I just don't want to save 5-10 lines and get worse performance.