HonorSpy

HonorSpy

2M Downloads

Script ran too long

one-zero-zero-one-one-zero-one opened this issue ยท 21 comments

commented

Lua error on dying and ending BG's.
Honorspy doesn't seem to be updating correctly anymore and is failing to get database send or receive it from others.
Lua.txt

commented

Does that happens when honorspy is the only addon enabled ?

commented

Sadly yes.

Lua error with only honorspy on added to message.

Lua (solo honorspy).txt

commented

How many people are known to honorspy ?
Which server are you playing on and which faction ?

commented

Classic Era - Firemaw (Horde)

Pool of about 2500-2700 people end of week.

commented

To me it sounds like the amount of data you have is what makes the script run for too long when trying to send it out.
We didn't had the chance to reach those numbers in the past on ERA so I couldn't really anticipate that.

It would probably require to either optimize the process so it fits or to "spool" the dataset and send it in multiple, smallest chunks.

My wow account is inactive so I will not be able to do the fix myself but at least there is this path to follow if someone want's to dive in.

For some context, ealier on ERA we fixed some corruption in comms by adding checks on message lenght (04b2074)
I added encoding and compression using LibDeflate to reduce the amount of data sent over comms at the same time and avoid some rate limiting issue I've encountered.
The compression and encoding is what seems to take too long.
Removing it would however require a com version bump and we would lose some message integrity check at the same time (In addition to sending out more data again) so I would avoid that if possible and explore the optimization / spooling options

commented

I feel like having the function HonorSpy:broadcastPlayers() add players to a queue then having a timed function sending them separately each few seconds would do the trick.
Maybe the queue could even contains the 10 players chunks the function makes, then the timed function would process those already defined chunks

The blizzard retail thing woud explains why it's broken indeed ...

If you can help with the testing / debugging I can try to help with this @teelolws

commented

Blizzard made a change in Dragonflight that lowers our processing time per frame. I guess they finally pushed that out to the other versions of the game. Its ridiculously short now. I get the error all the time from various addons.

Fixing it won't be easy. Will require a rework of half the addon. Have to stagger the processing.

commented

Doesn't even need to be every few seconds. Use C_Timer, create a 0.1s ticker that processes any pending work. Store the work in a table, storing the function reference and arguments.

Just make sure none of the work that goes into the ticker needs more than about 0.07s to process it. If it does, you'll have to increase the ticker.

commented

Yeah agree, I wasn't set on the delay itself. I would be fine with a 0.1s ticker
If I understand well you want to globalize the queue ? I think we will only have this issue on broadcasts most likely

commented

Sorry no, I don't play classic anymore. All I can do is give advice.

commented

Would you be willing to assist with debug and testing ?

commented

I haven't worked on this addon in about 18 months, I'm not going to suggest where to split the workload.

This is how I stagger the processing in one of my other addons:
workhandler.zip

Rather than directly starting processing, I just add it to a queue with: addon:addWork(processingFunc, arg1, arg2, arg3)

commented

I fully understand that, thanks for you input :)

@one-zero-zero-one-one-zero-one what about you ?

commented

Yes ofcourse. I got zero coding skills though so can only help with testing/feedback.

commented

I can also donate you a sub if you prefer that :)

commented

You won't need coding skill!
Add me on discord to start with.
I would rather not be a reason for you to give more money to blizz if you don't mind :P

commented

One-zero is testing a beta version that spool the broadcasts at the moment

commented

The author of the issue is done ranking and won't be able to help testing anymore
I need some other people that actually play the game to help me release the fix

If you're experiencing the bug and it bothers you, get [the beta version] and let me know if it works for you ! :D

commented

Without any tester we will probably merge and release the fix and see how it behave on the large scale.
Let us know if something is wrong on your side with 1.9.4

commented

We could, I'm unsure if that's worth making an exception, sharing works quite well with spool for what I know.

commented

So apparently the limit is no more than 200ms processing time, but only limited while in instanced content - dungeons or BGs. Perhaps you could work with that - skip the spooling if the player isn't in a BG or dungeon or raid?