Fix FoliaTickReporter never resolving getTickReport5s, bypassing the TPS cache#333
Open
Leleawa wants to merge 1 commit into
Open
Fix FoliaTickReporter never resolving getTickReport5s, bypassing the TPS cache#333Leleawa wants to merge 1 commit into
Leleawa wants to merge 1 commit into
Conversation
MethodHandles.Lookup#findVirtual matches on the full MethodType, return type included. The lookup passed Object.class as the return type, but RegionScheduleHandle#getTickReport5s returns TickData.TickReportData, so it always threw NoSuchMethodException and the handle stayed null. Every getTPS()/getMSPT() call therefore fell through to the Server fallback, which bypasses the Caffeine cache entirely. Server#getTPS() rebuilds the 1m/5m/15m tick reports on each call, sorting the full tick history (~18k samples for the 15m window). On a profiled Folia server this accounted for ~40% of all non-idle region thread time, nearly all of it in Arrays.sort, driven mostly by the regionalactivity Pathfinding listener calling shouldCancelBecauseLagging() on every EntityPathfindEvent. Add ReflectionUtil#findMethodAnyReturn, which unreflects a Method looked up by name and parameter types only, and use it here. The cached path now resolves the 5s report as intended. Also move the null check into the cache loaders, so the Server fallback can no longer be reached on an uncached path if the lookup ever breaks again in a future Folia release. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FoliaTickReporterlooks upRegionScheduleHandle#getTickReport5swithObject.classas the return type:ReflectionUtil.findMethodresolves this throughMethodHandles.Lookup#findVirtual, which matches on the fullMethodType, return type included. The real signature is:TickReportData != Object, so the lookup throwsNoSuchMethodExceptionandgetTickReport5sis alwaysnullon every Folia server.Minimal reproduction of the lookup semantics:
Impact
With the handle null, all four public methods take the
Serverfallback branch:That path bypasses the Caffeine cache entirely.
CraftServer#getTPS()rebuilds the 1m/5m/15m tick reports on every call, and each report sorts the full tick history — roughly 18k samples for the 15m window.Because
regionalactivity'sPathfindinglistener callsshouldCancelBecauseLagging()on everyEntityPathfindEvent, this runs constantly.Spark profile from an affected Folia server (12 region threads, 92% parked, so percentages below are of the ~27.9s of actual non-idle work):
For scale, inside
PathNavigation.createPath()(9552ms total), 9388ms — 98% — was this listener rather than actual pathfinding.Fix
ReflectionUtil#findMethodAnyReturn, which unreflects aMethodlooked up by name and parameter types only.findMethod's existing contract is untouched, so no other call sites are affected.FoliaTickReporter, so the cached 5s-window path works as originally intended.getTickReport5s == nullcheck into the two cache loaders. The old structure meant a failed lookup necessarily implied an uncached, expensive path; now theServerfallback is rate-limited by the cache no matter what, should the lookup ever break again in a future Folia release. This also removes 12 lines.Testing
./gradlew buildpasses. TheClassDowngradererrors in the log are pre-existing — a baseline build on unmodifiedmasterproduces the identical count (382).AnarchyExploitFixes-Foliajar bytecode that the constructor now callsfindMethodAnyReturn, and thatgetTPS()has no path toserver.getTPS()outside the cache loader.