Add config toggle for onInventoryMoveItem Event
LadyCailinBot opened this issue ยท 20 comments
WORLDGUARD-3520 - Reported by phanaticd
In worlds with a lot of hoppers this event can become quite expensive, and if server owners only use worldguard for a spawn region for example, all the checking just becomes unnecessary. That is why I propose a config toggle is added for checking this event, so it can be disabled if not needed for performance purposes.
Comment by Techcable
@sk89q
WorldGuard is now the bottleneck on phanaticd's server.
The easiest way to handle this is like @PhanaticD (but configurable), but it is possible to optimize the hashmap lookup and some of the underlying calls if you refuse :/
Comment by 00000000-0000-0000-0000-00000000
Config option that would allow skipping IMIE...
Comment by wizjany
that doesn't answer the question.
as noted by phanaticD and techable, you can comment out the handler if it so affects you.
however, techable specifically is talking about optimizing the lookups.
Comment by PseudoKnight
Techcable's suggestion for optimization aside, a config option would be handy for the majority of users. (but I guess we've been over this)
Comment by wizjany
yes, we're beating a dead horse in that regard. however if techcable actually has code optimizations in mind i'd love to hear them.
Comment by axed
Example on why this is indeed needed: http://top.mvdw-software.be/?id=11808787
Comment by wizjany
I love when people link timings reports where they are getting 20 tps and the server is taking 4x as long as the plugins to handle tile ticks, but it's still the plugins that are doing something wrong.
Not saying that we can't add this, just saying that your data is absolute bullshit.
Comment by axed
@wizjany Jesuz man the guy explains why an option might help us to improve performance and I provide some timings during night time from my server as an example and this how you react? No the plugins isn't doing anything wrong but like he says adding such a setting might help him, me and possibly other people. So salty...
Comment by PseudoKnight
Of the most frequent events WorldGuard handles, this is the only notable one that doesn't have a config option (the next most frequent without a config option is vehicle move events). So I think it's a fair request. This event is pretty intensive on its own, but reducing the overhead can help on large servers.
wizjany's issue with this was probably mostly to do with it listed as a bug. There's no "problem" here, per se. In fact, WG's handling of this event is pretty fast, I think. It takes just 5 microseconds on average. The real problem is sometimes there can be TONS of hoppers and large servers need to eek out as much as they can.
Comment by wizjany
my "issue" is only that people try to give timings reports as some sort of evidence but anyone with half a brain can see that they are counter-evidence to the poster's cause. like i said, that report only shows that the server has too many hoppers but that worldguard is actually doing a perfectly fine job of handling it. also, timings reports in general, especially that top page format thing, are a terrible indicator of performance issues and even worse at finding the cause.
Comment by PseudoKnight
Unfortunately there aren't many good alternatives for indicating performance issues other than spigot timings reports. (at least without their own downsides) Strangely, one of the best ones I discovered a couple years ago was actually in NoLagg of all things. I just wish it was separated from the rest of that terrible plugin. Warmroast can sometimes be nice to find particular classes that are taking too long, but it's often difficult to parse due to so much going on.
As you said, that report shows that WG is doing a fine job, but that it's still taking over 4% of the server time because there's so many hoppers. So in a way, it effectively communicated to you what was happening.
Comment by PseudoKnight
I should mention that while that page is slick (and the player pings is interesting), the classic spigot timings report is so much better for evaluating performance.
Comment by wizjany
warmroast would be a lot better. heck, WG has warmroast-lite inbuilt. it is literally as simple as one command to get a cpu sampling and pastebin it all at once.
that "top" thing is absolutely the worst, from presentation to formatting. classic timings is definitely better.
Comment by phanaticd
@wizjany jfc wizjany.. hope you are not being serious I only asked one time 10 days after the ticket was created
Comment by 00000000-0000-0000-0000-00000000
I can confirm this too, WG is causing ~5% of load just for IMIEs on my server (medium load, ~2000 hoppers in loaded chunks):
WorldGuard v6.1.1-SNAPSHOT.1619-d320356Total: 331.895 s Pct: 4.70%
3.27% 4.21% 230.34 s 2.10 ms 465 50,863.7k EventAbstractionListener::onInventoryMoveItem(InventoryMoveItemEvent)
and that's AFTER canceling ~50% IMIEs without that it could easy be 10+%. With only 2 regions (global and spawn).
It's almost like "high frequency flags" ... IMO everyone with 2k+ hoppers (with stuff in them) has those problems...