Rework MarkerAPIImpl to async, to prevent huge lag
TheLaoming opened this issue ยท 16 comments
Hey,
Basically dynmap is not usable on bigger production servers with 80+ players. It killing the server even more than Slimefun.
Even with external web server + MariaDB configured in, dynmap is able to literally kill that server like a nothing. All that due to markers writting into main thread.
Here's proof: (this timings is with parametr only-ticks-over 300 which is already insane value...)
I think async saving/writting markers can save over 90% of dynmap lag overall. Every modern plugin using async, so...
Or atleast anything else what prevent this huge lag. Unfortunatelly from this moment I get rid of it until it get an improvement, I can't use it on my main server.
Thanks for your time.
FYI: the imgur link is wrong:
What you see: https://i.imgur.com/j9UyRhg.png
What you get: https://github.com/webbukkit/dynmap/issues/url
The Markdown code: [https://i.imgur.com/j9UyRhg.png](https://github.com/webbukkit/dynmap/issues/url)
FYI: I added an image instead of a link.
So back to this issue, hope mikeprimm will add this in the near future and notice this topic. I think it's really important feature request, since I know about few big servers using pl3map, squaremap etc. instead of dynmap mainly due to this issue and performance benefits.
YIKES - never would have guessed that the sanitizeHTML implementation in the OWASP library was that horrifying inefficient! Thanks for flagging this.
Is the sanitizing even needed during writing? It feels like the wrong place to be doing that...
It was a way to deal with migrating existing files that were not 'safely' encoded (it was possible with anyone with the right to create markers to do script injection attacks).
As for it being 'unusable on servers with more than 80+ users', I beg to differ (as I run it on a server routinely with 100-200 users, as do literally hundreds of other servers). Now, if you have mods using the marker API to write custom markers constantly, AND those plugins are incorrectly coded such that they are continuously updating persistent markers (which are stored in the files, versus being just memory-based, which is appropriate behavior for markers that are defined externally), I could see this being a concern.
I'd be interested to know what marker-driving plugins are involved in this use case.
Hm, interesting and good point too. I think it's plugin Residence displaying claimed areas. You can check right here (I'm not using this map anymore): magic.gamerealms.cz. Just enable Residences and you will see. It's really a lot of areas there. I don't know about anything else interacting with dynmap, especially with markers.
It is indeed creating persistent markers!
Cool - well, that is another discussion, but I think what I'm working on should do a lot of good :)
Cool - thanks! I'm just trying to be accurate on bounding things here - I VERY MUCH appreciate the discovery, as I really wasn't aware of the poor performance on the sanitizeHTML function in OWASP - but I just wanted to be sure we can better characterize the where-and-when on this showing :)
I'm actually doing a change right now that will both optimize the sanitizeHTML use, as well as shifting it to only be on load of the marker file (when needed), and when marker labels and descriptions are specifically being set. I'll ping here when I've got a SNAPSHOT with the changes - I'd very much appreciate a look on what it does for you, if that is possible.
OK - dev builds with updates in https://dynmap.us/builds/dynmap (look for 3.5-SNAPSHOT appropriate to platform).
also this feels like duplicate of #3846
AND those plugins are incorrectly coded such that they are continuously updating persistent markers (which are stored in the files, versus being just memory-based, which is appropriate behavior for markers that are defined externally),
So if you create non persistent markers with the api there will be no html sanitize? That should be noted in the api docs because some people might use user input without sanitizing those markers.
Didn't say that - but having high frequency updating of persistent markers, which causes frequent rewrites of markers.yml for no good reason, is every bit as bad for performance.
Anyway, I will let you know if this update really improve performance, i'm sure and believe it will. Meanwhile I use this chance that you're right now active and reading this post @mikeprimm, by any chance can you take a look also to #3728 ? It's been probably forgotten and i'm facing this issue since 1.18.
@TheLaoming did you get around to testing this feature after the update? if you feel like it is fixed, please close the issue