Add timings to third-party session handlers (#1717)

* Add Timings for third-party session handlers.

These often cause WorldGuard to get blamed by timings.

* Add config option to turn off extra timings.

* Move relocation to the right build file.

* Move config to a map for potential future expansion.

* Calculate timing immediately per-factory.

Instead of on each handler creation.

config/checkstyle/import-control.xml

@@ -29,6 +29,7 @@
       <allow pkg="org.bstats.bukkit"/>
       <allow pkg="io.papermc.lib"/>
       <allow pkg="com.destroystokyo.paper"/>
+      <allow pkg="co.aikar.timings.lib" />
     </subpackage>
 
   </subpackage>

worldguard-bukkit/build.gradle.kts

@@ -19,6 +19,10 @@
         name = "bstats"
         url = uri("https://repo.codemc.org/repository/maven-public")
     }
+    maven {
+        name = "aikar-timings"
+        url = uri("http://repo.aikar.co/nexus/content/groups/aikar/")
+    }
 }
 
 dependencies {
@@ -30,6 +34,7 @@
     "implementation"("com.google.guava:guava:${Versions.GUAVA}")
     "implementation"("com.sk89q:commandbook:2.3") { isTransitive = false }
     "implementation"("org.bstats:bstats-bukkit:1.7")
+    "implementation"("co.aikar:minecraft-timings:1.0.4")
 }
 
 tasks.named<Upload>("install") {
@@ -71,6 +76,9 @@
         relocate ("io.papermc.lib", "com.sk89q.worldguard.bukkit.paperlib") {
             include(dependency("io.papermc:paperlib:1.0.4"))
         }
+        relocate ("co.aikar.timings.lib", "com.sk89q.worldguard.bukkit.timingslib") {
+            include(dependency("co.aikar:minecraft-timings:1.0.4"))
+        }
     }
 }
 

worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/BukkitConfigurationManager.java

@@ -37,6 +37,7 @@ public class BukkitConfigurationManager extends YamlConfigurationManager {
 
     private boolean hasCommandBookGodMode;
     boolean extraStats;
+    boolean timedSessionHandlers;
 
     /**
      * Construct the object.
@@ -56,6 +57,7 @@ public Collection<BukkitWorldConfiguration> getWorldConfigs() {
     public void load() {
         super.load();
         this.extraStats = getConfig().getBoolean("custom-metrics-charts", true);
+        this.timedSessionHandlers = getConfig().getBoolean("extra-timings.session-handlers", true);
     }
 
     @Override

worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/BukkitWorldGuardPlatform.java

@@ -67,7 +67,7 @@
 
 public class BukkitWorldGuardPlatform implements WorldGuardPlatform {
 
-    private SessionManager sessionManager;
+    private BukkitSessionManager sessionManager;
     private BukkitConfigurationManager configuration;
     private BukkitRegionContainer regionContainer;
     private BukkitDebugHandler debugHandler;
@@ -138,6 +138,7 @@ public void load() {
         sessionManager = new BukkitSessionManager();
         configuration = new BukkitConfigurationManager(WorldGuardPlugin.inst());
         configuration.load();
+        sessionManager.setUsingTimings(configuration.timedSessionHandlers);
         regionContainer = new BukkitRegionContainer(WorldGuardPlugin.inst());
         regionContainer.initialize();
         debugHandler = new BukkitDebugHandler(WorldGuardPlugin.inst());

worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/session/BukkitSessionManager.java

@@ -27,6 +27,7 @@
 import com.sk89q.worldguard.bukkit.event.player.ProcessPlayerEvent;
 import com.sk89q.worldguard.session.AbstractSessionManager;
 import com.sk89q.worldguard.session.Session;
+import com.sk89q.worldguard.session.handler.Handler;
 import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
@@ -40,6 +41,13 @@
  */
 public class BukkitSessionManager extends AbstractSessionManager implements Runnable, Listener {
 
+    private boolean useTimings;
+
+    @Override
+    protected Handler.Factory<? extends Handler> wrapForRegistration(Handler.Factory<? extends Handler> factory) {
+        return useTimings ? new TimedHandlerFactory(factory) : factory;
+    }
+
     /**
      * Re-initialize handlers and clear "last position," "last state," etc.
      * information for all players.
@@ -75,9 +83,17 @@ public void run() {
     public boolean hasBypass(LocalPlayer player, World world) {
         if (player instanceof BukkitPlayer) {
             if (((BukkitPlayer) player).getPlayer().hasMetadata("NPC")
-                    && WorldGuard.getInstance().getPlatform().getGlobalStateManager().get(world).fakePlayerBuildOverride)
+                && WorldGuard.getInstance().getPlatform().getGlobalStateManager().get(world).fakePlayerBuildOverride)
                 return true;
         }
         return super.hasBypass(player, world);
     }
+
+    public boolean isUsingTimings() {
+        return useTimings;
+    }
+
+    public void setUsingTimings(boolean useTimings) {
+        this.useTimings = useTimings;
+    }
 }

worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/session/TimedHandlerFactory.java

@@ -0,0 +1,121 @@
+/*
+ * WorldGuard, a suite of tools for Minecraft
+ * Copyright (C) sk89q <http://www.sk89q.com>
+ * Copyright (C) WorldGuard team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.sk89q.worldguard.bukkit.session;
+
+import co.aikar.timings.lib.MCTiming;
+import co.aikar.timings.lib.TimingManager;
+import com.sk89q.worldedit.util.Location;
+import com.sk89q.worldguard.LocalPlayer;
+import com.sk89q.worldguard.bukkit.WorldGuardPlugin;
+import com.sk89q.worldguard.protection.ApplicableRegionSet;
+import com.sk89q.worldguard.protection.regions.ProtectedRegion;
+import com.sk89q.worldguard.session.MoveType;
+import com.sk89q.worldguard.session.Session;
+import com.sk89q.worldguard.session.handler.Handler;
+import org.bukkit.Bukkit;
+import org.bukkit.plugin.Plugin;
+
+import java.security.CodeSource;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+class TimedHandlerFactory extends Handler.Factory<Handler> {
+
+    private static final TimingManager TIMINGS = TimingManager.of(WorldGuardPlugin.inst());
+    private static final MCTiming UNKNOWN_SOURCE = TIMINGS.of("Third-Party Session Handlers");
+    private static final Map<CodeSource, TimingManager> PLUGIN_SOURCES = new HashMap<>();
+
+    private final Handler.Factory<?> factory;
+    private final MCTiming timing;
+
+    TimedHandlerFactory(Handler.Factory<?> factory) {
+        this.factory = factory;
+        this.timing = makeTiming();
+    }
+
+    private MCTiming makeTiming() {
+        CodeSource codeSource = factory.getClass().getProtectionDomain().getCodeSource();
+        TimingManager owner = PLUGIN_SOURCES.computeIfAbsent(codeSource, source -> {
+            for (Plugin plugin : Bukkit.getPluginManager().getPlugins()) {
+                CodeSource pluginSource = plugin.getClass().getProtectionDomain().getCodeSource();
+                if (Objects.equals(pluginSource, source)) {
+                    return TimingManager.of(plugin);
+                }
+            }
+            return null;
+        });
+        String handlerName = factory.getClass().getEnclosingClass().getSimpleName();
+        return owner == null
+            ? TIMINGS.of(handlerName, UNKNOWN_SOURCE)
+            : owner.of(handlerName, owner.of("Session Handlers"));
+    }
+
+    @Override
+    public Handler create(Session session) {
+        return new TimedHandler(factory.create(session), session, timing);
+    }
+
+    static class TimedHandler extends Handler {
+        private final Handler handler;
+        private final MCTiming timing;
+
+        TimedHandler(Handler innerHandler, Session session, MCTiming timing) {
+            super(session);
+            this.handler = innerHandler;
+            this.timing = timing;
+        }
+
+        @Override
+        public void initialize(LocalPlayer player, Location current, ApplicableRegionSet set) {
+            try (MCTiming ignored = timing.startTiming()) {
+                super.initialize(player, current, set);
+            }
+        }
+
+        @Override
+        public boolean testMoveTo(LocalPlayer player, Location from, Location to, ApplicableRegionSet toSet, MoveType moveType) {
+            try (MCTiming ignored = timing.startTiming()) {
+                return super.testMoveTo(player, from, to, toSet, moveType);
+            }
+        }
+
+        @Override
+        public boolean onCrossBoundary(LocalPlayer player, Location from, Location to, ApplicableRegionSet toSet, Set<ProtectedRegion> entered, Set<ProtectedRegion> exited, MoveType moveType) {
+            try (MCTiming ignored = timing.startTiming()) {
+                return super.onCrossBoundary(player, from, to, toSet, entered, exited, moveType);
+            }
+        }
+
+        @Override
+        public void tick(LocalPlayer player, ApplicableRegionSet set) {
+            try (MCTiming ignored = timing.startTiming()) {
+                super.tick(player, set);
+            }
+        }
+
+        @Override
+        public Handler getWrappedHandler() {
+            return handler;
+        }
+    }
+
+}

worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java

@@ -44,8 +44,10 @@
 import javax.annotation.Nullable;
 import java.lang.ref.WeakReference;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.function.BiPredicate;
@@ -73,6 +75,8 @@ public abstract class AbstractSessionManager implements SessionManager {
                     createSession(key.playerRef.get())));
 
     private boolean hasCustom = false;
+    // <original handler, wrapped handler>
+    private Map<Handler.Factory<? extends Handler>, Handler.Factory<? extends Handler>> wrappedHandlers = new HashMap<>();
     private List<Handler.Factory<? extends Handler>> handlers = new LinkedList<>();
 
     private static final List<Handler.Factory<? extends Handler>> defaultHandlers = new LinkedList<>();
@@ -106,20 +110,27 @@ public boolean customHandlersRegistered() {
         return hasCustom;
     }
 
+    protected Handler.Factory<? extends Handler> wrapForRegistration(Handler.Factory<? extends Handler> factory) {
+        return factory;
+    }
+
     @Override
     public boolean registerHandler(Handler.Factory<? extends Handler> factory, @Nullable Handler.Factory<? extends Handler> after) {
         if (factory == null) return false;
         WorldGuard.logger.log(Level.INFO, "Registering session handler "
                 + factory.getClass().getEnclosingClass().getName());
         hasCustom = true;
+        Handler.Factory<? extends Handler> wrappedFactory = wrapForRegistration(factory);
         if (after == null) {
-            handlers.add(factory);
+            handlers.add(wrappedFactory);
         } else {
-            int index = handlers.indexOf(after);
+            Handler.Factory<? extends Handler> wrappedAfter = wrappedHandlers.get(after);
+            int index = handlers.indexOf(wrappedAfter != null ? wrappedAfter : after);
             if (index == -1) return false;
 
             handlers.add(index, factory); // shifts "after" right one, and everything after "after" right one
         }
+        wrappedHandlers.put(factory, wrappedFactory);
         return true;
     }
 
@@ -131,6 +142,7 @@ public boolean unregisterHandler(Handler.Factory<? extends Handler> factory) {
         } else {
             WorldGuard.logger.log(Level.INFO, "Unregistering session handler "
                     + factory.getClass().getEnclosingClass().getName());
+            factory = wrappedHandlers.remove(factory);
         }
         return handlers.remove(factory);
     }

worldguard-core/src/main/java/com/sk89q/worldguard/session/Session.java

@@ -34,10 +34,10 @@
 import com.sk89q.worldguard.session.handler.Handler;
 import com.sk89q.worldguard.util.Locations;
 
-import javax.annotation.Nullable;
 import java.util.HashMap;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import javax.annotation.Nullable;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -69,7 +69,7 @@ public Session(SessionManager manager) {
      * @param handler A new handler
      */
     public void register(Handler handler) {
-        handlers.put(handler.getClass(), handler);
+        handlers.put(handler.getWrappedHandler().getClass(), handler);
     }
 
     /**
@@ -91,7 +91,7 @@ public SessionManager getManager() {
     @Nullable
     @SuppressWarnings("unchecked")
     public <T extends Handler> T getHandler(Class<T> type) {
-        return (T) handlers.get(type);
+        return (T) handlers.get(type).getWrappedHandler();
     }
 
     /**

worldguard-core/src/main/java/com/sk89q/worldguard/session/handler/Handler.java

@@ -138,4 +138,11 @@ public State getInvincibility(LocalPlayer player) {
         return null;
     }
 
+    /**
+     * Get the handler wrapped by this handler object, if applicable, or just return this if no handler is wrapped.
+     * @return any wrapped handler, or this handler itself
+     */
+    public Handler getWrappedHandler() {
+        return this;
+    }
 }