Grief Prevention

Grief Prevention

1M Downloads

Claim.allowContainers Drop TPS

SrMonsterYT opened this issue ยท 10 comments

commented

Observed Behavior

lag is happening on my server and the only plugin that's doing it is this one, I've been using it for years since 1.12.2

lag server: https://prnt.sc/qNBYb1EFTTjh

Expected Behavior

no lag

Reproduction steps

I don't know how it happens

Stack trace or error log

no error

Server version

git-Pufferfish-15 (MC: 1.20.1)

GriefPrevention version

GriefPrevention  16.18.1-48-g6884bdf

Configuration

https://mclo.gs/owclRgQ

Plugin list

No response

Running without GriefPrevention

  • I attempted running the server without GriefPrevention installed.
  • The problem does not occur when GriefPrevention is removed from the server.

Running with only GriefPrevention

  • I attempted running only GriefPrevention on the server.
  • The issue still occurs when GriefPrevention is the only plugin running.

Running on a fresh, clean server installation

  • I attempted testing for the issue on a new server.
  • The issue still occurs on a new server.

Using unmodified client

  • I attempted testing for the issue with the vanilla client.
  • The issue still occurs when using the vanilla client.

We appreciate you taking the time to fill out a bug report!

  • I searched for similar issues before submitting this bug report.
commented

I don't think GP is causing the lag, but can't really tell you anything without more information. A spark report is the best way to confirm if its GP or not.

If you're able to, download the Spark plugin. Once the server starts to lag run a spark profiler for a couple of minutes, then share the link here. This will let us confirm if its GP or not.

Command

/spark profiler start --timeout 120

Make sure you run the command only when the server is lagging, won't be helpful if its not

https://spark.lucko.me/krvm0xoPyb

commented

I don't think GP is causing the lag, but can't really tell you anything without more information. A spark report is the best way to confirm if its GP or not.

If you're able to, download the Spark plugin. Once the server starts to lag run a spark profiler for a couple of minutes, then share the link here. This will let us confirm if its GP or not.

Command

/spark profiler start --timeout 120

Make sure you run the command only when the server is lagging, won't be helpful if its not

commented

Knew I'd seen this issue before, it's the same root cause as #1415. I've opened Slimefun/dough#248 as a temporary fix.

Quoting myself from the other issue:

A cache would also be simple to implement

If anyone else wants to have a stab at it (it's easy, it'd make a good first issue!) it's this line.

commented

It did an offlineplayer lookup. Not sure if the cache is being bypassed or something here - usually GP loads all recently-played players. Or so it did. Maybe that has been changed recently. That being said, this could point to other potential issues. Not sure how offlineplayers are retrieved by pufferfish, or if there are extra hits to disk, which would then depend on if it's a SSD and i/o load and such.

commented

It did an offlineplayer lookup. Not sure if the cache is being bypassed or something here - usually GP loads all recently-played players. Or so it did. Maybe that has been changed recently. That being said, this could point to other potential issues. Not sure how offlineplayers are retrieved by pufferfish, or if there are extra hits to disk, which would then depend on if it's a SSD and i/o load and such.

I've updated to the latest version, but players are able to protect /spawn, even if it's protected with worldguard, so I had to go back

commented

It did an offlineplayer lookup. Not sure if the cache is being bypassed or something here - usually GP loads all recently-played players. Or so it did. Maybe that has been changed recently. That being said, this could point to other potential issues. Not sure how offlineplayers are retrieved by pufferfish, or if there are extra hits to disk, which would then depend on if it's a SSD and i/o load and such.

This is OfflinePlayer#getName, not the lookup by name. GP does cache lookups by name I believe, but it does not cache names for UUIDs. This was also one of the reasons the denial message is provided via Supplier now, so addons can check permission without causing offline players' data to be read from disk.

https://spark.lucko.me/krvm0xoPyb

Looks like SlimeFun is using the deprecated Claim#allowContainers instead of Claim#checkPermission(Player, ClaimPermission, Event) in their protection library. If they change that you should see huge improvements across the board - it looks like the vast majority of their container permission checks are actually their special blocks trying to schedule push/pull, which would not result in player feedback.
Reversed caller order shows them as the source of the vast majority of GP calls.
image

/e: https://github.com/baked-libs/dough/blob/1108163a4971627d4730e913b14c733d36c60f2f/dough-protection/src/main/java/io/github/bakedlibs/dough/protection/modules/GriefPreventionProtectionModule.java

commented

Ah, I didn't drill down far enough to see what was calling it, thanks for looking into it further.

commented

Ah, I didn't drill down far enough to see what was calling it, thanks for looking into it further.

do you have a solution for this?

I even tried to update to the latest version, but people can protect land protected by worldguard with /spawn, so I went back to the old version

commented

do you have a solution for this?

Jikoo made a PR, tho for only master branch rn. I'll likely accept, seems it doesn't add much complexity (though I do have questions about the google imports - I guess the dependencies for those are in maven central and requires no dependency to be listed?)

I even tried to update to the latest version, but people can protect land protected by worldguard with /spawn, so I went back to the old version

I recommend using /adminclaim to protect land you don't want claimed. As an fyi, all builds from appveyor are development builds, and not to be considered "latest."

commented

Jikoo made a PR, tho for only master branch rn. I'll likely accept, seems it doesn't add much complexity (though I do have questions about the google imports - I guess the dependencies for those are in maven central and requires no dependency to be listed?)

Transitive dependency from spigot-api:

$ mvn dependency:tree
[INFO] Scanning for projects...
[INFO]
[INFO] ----------------< com.griefprevention:GriefPrevention >-----------------
[INFO] Building GriefPrevention 16.18.2-SNAPSHOT
[INFO]   from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- dependency:3.6.1:tree (default-cli) @ GriefPrevention ---
[INFO] com.griefprevention:GriefPrevention:jar:16.18.2-SNAPSHOT
[INFO] +- org.spigotmc:spigot-api:jar:1.20.1-R0.1-SNAPSHOT:provided
[INFO] |  +- com.google.guava:guava:jar:31.1-jre:provided
[INFO] |  |  +- com.google.guava:failureaccess:jar:1.0.1:provided
[INFO] |  |  +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:provided
[INFO] |  |  +- com.google.code.findbugs:jsr305:jar:3.0.2:provided
[INFO] |  |  +- org.checkerframework:checker-qual:jar:3.12.0:provided
[INFO] |  |  +- com.google.errorprone:error_prone_annotations:jar:2.11.0:provided
[INFO] |  |  \- com.google.j2objc:j2objc-annotations:jar:1.3:provided
[INFO] |  +- com.google.code.gson:gson:jar:2.10:provided
[INFO] |  +- org.joml:joml:jar:1.10.5:provided
[INFO] |  +- net.md-5:bungeecord-chat:jar:1.20-R0.1:provided
[INFO] |  \- org.yaml:snakeyaml:jar:2.0:provided
[INFO] +- com.sk89q.worldguard:worldguard-bukkit:jar:7.0.4-SNAPSHOT:provided
[INFO] |  +- com.sk89q.worldedit:worldedit-bukkit:jar:7.2.0-SNAPSHOT:provided
[INFO] |  +- com.sk89q.worldguard:worldguard-core:jar:7.0.4-SNAPSHOT:provided
[INFO] |  |  +- com.sk89q.worldguard.worldguard-libs:core:jar:7.0.4-SNAPSHOT:provided
[INFO] |  |  +- com.sk89q.worldedit:worldedit-core:jar:7.2.0-SNAPSHOT:provided
[INFO] |  |  |  +- com.sk89q.worldedit.worldedit-libs:core:jar:7.2.0-SNAPSHOT:provided
[INFO] |  |  |  +- de.schlichtherle:truezip:jar:6.8.3:provided
[INFO] |  |  |  +- net.java.truevfs:truevfs-profile-default_2.13:jar:0.12.1:provided
[INFO] |  |  |  |  +- org.scala-lang:scala-library:jar:2.13.1:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-http:jar:0.12.1:provided
[INFO] |  |  |  |  |  +- net.java.truevfs:truevfs-kernel-spec:jar:0.12.1:provided
[INFO] |  |  |  |  |  |  +- net.java.truecommons:truecommons-annotations:jar:2.5.0:provided
[INFO] |  |  |  |  |  |  |  \- com.google.code.findbugs:annotations:jar:3.0.0:provided
[INFO] |  |  |  |  |  |  +- net.java.truecommons:truecommons-cio:jar:2.5.0:provided
[INFO] |  |  |  |  |  |  +- net.java.truecommons:truecommons-io:jar:2.5.0:provided
[INFO] |  |  |  |  |  |  \- net.java.truecommons:truecommons-services:jar:2.5.0:provided
[INFO] |  |  |  |  |  |     +- net.java.truecommons:truecommons-logging:jar:2.5.0:provided
[INFO] |  |  |  |  |  |     \- javax.inject:javax.inject:jar:1:provided
[INFO] |  |  |  |  |  +- org.apache.httpcomponents:httpclient:jar:4.5.10:provided
[INFO] |  |  |  |  |  |  +- org.apache.httpcomponents:httpcore:jar:4.4.12:provided
[INFO] |  |  |  |  |  |  \- commons-codec:commons-codec:jar:1.11:provided
[INFO] |  |  |  |  |  \- org.slf4j:jcl-over-slf4j:jar:1.7.28:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-odf:jar:0.12.1:provided
[INFO] |  |  |  |  |  \- net.java.truevfs:truevfs-comp-zipdriver:jar:0.12.1:provided
[INFO] |  |  |  |  |     \- net.java.truevfs:truevfs-comp-zip:jar:0.12.1:provided
[INFO] |  |  |  |  |        \- org.bouncycastle:bcprov-jdk15on:jar:1.63:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-tar:jar:0.12.1:provided
[INFO] |  |  |  |  |  \- net.java.truevfs:truevfs-comp-tardriver:jar:0.12.1:provided
[INFO] |  |  |  |  |     \- org.apache.commons:commons-compress:jar:1.19:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-tar-bzip2:jar:0.12.1:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-tar-gzip:jar:0.12.1:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-tar-xz:jar:0.12.1:provided
[INFO] |  |  |  |  |  \- org.tukaani:xz:jar:1.8:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-driver-zip-raes:jar:0.12.1:provided
[INFO] |  |  |  |  +- net.java.truevfs:truevfs-profile-base_2.13:jar:0.12.1:provided
[INFO] |  |  |  |  |  +- net.java.truevfs:truevfs-access-swing:jar:0.12.1:provided
[INFO] |  |  |  |  |  |  \- net.java.truevfs:truevfs-access:jar:0.12.1:provided
[INFO] |  |  |  |  |  |     \- net.java.truevfs:truevfs-driver-file:jar:0.12.1:provided
[INFO] |  |  |  |  |  +- net.java.truevfs:truevfs-driver-jar:jar:0.12.1:provided
[INFO] |  |  |  |  |  +- net.java.truevfs:truevfs-driver-zip:jar:0.12.1:provided
[INFO] |  |  |  |  |  |  \- net.java.truevfs:truevfs-comp-ibm437:jar:0.12.1:provided
[INFO] |  |  |  |  |  +- net.java.truevfs:truevfs-kernel-impl_2.13:jar:0.12.1:provided
[INFO] |  |  |  |  |  +- net.java.truecommons:truecommons-key-console:jar:2.5.0:provided
[INFO] |  |  |  |  |  +- net.java.truecommons:truecommons-key-default:jar:2.5.0:provided
[INFO] |  |  |  |  |  \- net.java.truecommons:truecommons-key-swing:jar:2.5.0:provided
[INFO] |  |  |  |  \- net.java.truecommons:truecommons-key-macosx:jar:2.5.0:provided
[INFO] |  |  |  |     +- net.java.truecommons:truecommons-key-spec:jar:2.5.0:provided
[INFO] |  |  |  |     |  \- net.java.truecommons:truecommons-shed:jar:2.5.0:provided
[INFO] |  |  |  |     \- net.java.dev.jna:jna:jar:4.1.0:provided
[INFO] |  |  |  +- org.mozilla:rhino-runtime:jar:1.7.12:provided
[INFO] |  |  |  +- org.slf4j:slf4j-api:jar:1.7.26:provided
[INFO] |  |  |  +- it.unimi.dsi:fastutil:jar:8.2.1:provided
[INFO] |  |  |  \- org.antlr:antlr4-runtime:jar:4.7.2:provided
[INFO] |  |  \- org.flywaydb:flyway-core:jar:3.0:provided
[INFO] |  \- io.papermc:paperlib:jar:1.0.4:provided
[INFO] +- net.milkbowl.vault:VaultAPI:jar:1.7:provided
[INFO] +- org.jetbrains:annotations:jar:23.0.0:provided
[INFO] +- org.junit.jupiter:junit-jupiter:jar:5.9.3:test
[INFO] |  +- org.junit.jupiter:junit-jupiter-api:jar:5.9.3:test
[INFO] |  |  +- org.opentest4j:opentest4j:jar:1.2.0:test
[INFO] |  |  +- org.junit.platform:junit-platform-commons:jar:1.9.3:test
[INFO] |  |  \- org.apiguardian:apiguardian-api:jar:1.1.2:test
[INFO] |  +- org.junit.jupiter:junit-jupiter-params:jar:5.9.3:test
[INFO] |  \- org.junit.jupiter:junit-jupiter-engine:jar:5.9.3:test
[INFO] |     \- org.junit.platform:junit-platform-engine:jar:1.9.3:test
[INFO] \- org.mockito:mockito-junit-jupiter:jar:5.4.0:test
[INFO]    \- org.mockito:mockito-core:jar:5.4.0:test
[INFO]       +- net.bytebuddy:byte-buddy:jar:1.14.5:test
[INFO]       +- net.bytebuddy:byte-buddy-agent:jar:1.14.5:test
[INFO]       \- org.objenesis:objenesis:jar:3.3:test
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.921 s
[INFO] Finished at: 2024-02-13T20:27:53+01:00
[INFO] ------------------------------------------------------------------------