Reduce calls to Bukkit.isPrimaryThread(). Provide optimized methods.

Where it's known that it's the primary thread, that test should be
omitted.

A remaining problem is the Check class, where the convenience methods
all will lead to testing for Bukkit.isPrimaryThread(). This needs to be
done differently.

It'll be hard/impossible to work around, if we have to check permissions
and meta data. For permissions we could do some kind of bulk/context
dependent caching and updating policy and check via PlayerData, but meta
data needs the Bukkit API to state thread safety. Future design could
also do without knowledge of the thread, if permissions are cached and
exemption by meta data is turned off (or also cached, but this only
works if other plugins don't use it for temporary exemption), a lazy
approach could pass on an AlmostBoolean isPrimaryThread.

For now, at least some of the frequently run moving checks use the
optimized approach.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/blockbreak/BlockBreakListener.java

@@ -150,7 +150,9 @@ else if (MovingUtil.hasScheduledPlayerSetBack(player)) {
         }
 
         // Destroying liquid blocks.
-        if (!cancelled && BlockProperties.isLiquid(block.getType()) && !player.hasPermission(Permissions.BLOCKBREAK_BREAK_LIQUID) && !NCPExemptionManager.isExempted(player, CheckType.BLOCKBREAK_BREAK)){
+        if (!cancelled && BlockProperties.isLiquid(block.getType()) 
+                && !player.hasPermission(Permissions.BLOCKBREAK_BREAK_LIQUID) 
+                && !NCPExemptionManager.isExempted(player, CheckType.BLOCKBREAK_BREAK, true)){
             cancelled = true;
         }
 

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/MovingListener.java

@@ -693,7 +693,9 @@ else if (time < data.timeSprinting) {
             data.adjustWalkSpeed(player.getWalkSpeed(), tick, cc.speedGrace);
             thisMove.flyCheck = CheckType.MOVING_SURVIVALFLY;
         }
-        else if (cc.creativeFlyCheck && !NCPExemptionManager.isExempted(player, CheckType.MOVING_CREATIVEFLY) && !player.hasPermission(Permissions.MOVING_CREATIVEFLY)) {
+        else if (cc.creativeFlyCheck 
+                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_CREATIVEFLY, true) 
+                && !player.hasPermission(Permissions.MOVING_CREATIVEFLY)) {
             checkCf = true;
             checkSf = false;
             data.adjustFlySpeed(player.getFlySpeed(), tick, cc.speedGrace);
@@ -799,7 +801,7 @@ && checkPastStateBounceAscend(player, pFrom, pTo, thisMove, lastMove, tick, data
         // TODO: Redesign to set set backs later (queue + invalidate).
         boolean mightSkipNoFall = false; // If to skip nofall check (mainly on violation of other checks).
         if (newTo == null && cc.passableCheck && player.getGameMode() != BridgeMisc.GAME_MODE_SPECTATOR 
-                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_PASSABLE) 
+                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_PASSABLE, true) 
                 && !player.hasPermission(Permissions.MOVING_PASSABLE)) {
             // Passable is checked first to get the original set back locations from the other checks, if needed. 
             newTo = passable.check(player, pFrom, pTo, data, cc, tick, useBlockChangeTracker);
@@ -883,7 +885,7 @@ else if (checkCf) {
 
         // Morepackets.
         if (cc.morePacketsCheck && (newTo == null || data.isMorePacketsSetBackOldest())
-                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_MOREPACKETS) 
+                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_MOREPACKETS, true) 
                 && !player.hasPermission(Permissions.MOVING_MOREPACKETS)) {
             /* (Always check morepackets, if there is a chance that setting/overriding newTo is appropriate,
             to avoid packet speeding using micro-violations.) */

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/player/NoFall.java

@@ -330,7 +330,8 @@ public void checkDamage(final Player player, final MovingData data, final double
      * @return
      */
     public boolean isEnabled(final Player player , final MovingConfig cc) {
-        return cc.noFallCheck && !NCPExemptionManager.isExempted(player, CheckType.MOVING_NOFALL) && !player.hasPermission(Permissions.MOVING_NOFALL);
+        return cc.noFallCheck && !NCPExemptionManager.isExempted(player, CheckType.MOVING_NOFALL, true) 
+                && !player.hasPermission(Permissions.MOVING_NOFALL);
     }
 
 }

NCPCore/src/main/java/fr/neatmonster/nocheatplus/checks/moving/util/MovingUtil.java

@@ -63,7 +63,8 @@ public class MovingUtil {
     private static final Location useLoc = new Location(null, 0, 0, 0);
 
     /**
-     * Check if the player is to be checked by the survivalfly check.
+     * Check if the player is to be checked by the survivalfly check.<br>
+     * Primary thread only.
      * 
      * @param player
      * @param fromLoc
@@ -78,7 +79,7 @@ public static final boolean shouldCheckSurvivalFly(final Player player, final Pl
         return  cc.survivalFlyCheck && gameMode != BridgeMisc.GAME_MODE_SPECTATOR
                 && (cc.ignoreCreative || gameMode != GameMode.CREATIVE) && !player.isFlying() 
                 && (cc.ignoreAllowFlight || !player.getAllowFlight())
-                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_SURVIVALFLY)
+                && !NCPExemptionManager.isExempted(player, CheckType.MOVING_SURVIVALFLY, true)
                 && (Double.isInfinite(Bridge1_9.getLevitationAmplifier(player)) || fromLocation.isInLiquid())
                 && (!Bridge1_9.isGlidingWithElytra(player) || fromLocation.isOnGroundOrResetCond())
                 && !player.hasPermission(Permissions.MOVING_SURVIVALFLY);

NCPCore/src/main/java/fr/neatmonster/nocheatplus/hooks/NCPExemptionManager.java

@@ -151,6 +151,19 @@ public static final boolean isExempted(final UUID id, final CheckType checkType)
         return exempted.get(checkType).contains(id);
     }
 
+    /**
+     * Check for exemption, including meta data. Convenience method, testing for
+     * primary thread.
+     * 
+     * @see #isExempted(Player, CheckType, boolean)
+     * @param player
+     * @param checkType
+     * @return
+     */
+    public static final boolean isExempted(final Player player, final CheckType checkType) {
+        return isExempted(player, checkType,  Bukkit.isPrimaryThread());
+    }
+
     /**
      * Check if a player is exempted from a check right now. This also checks
      * for exemption by meta data, iff it's called from within execution of the
@@ -161,11 +174,16 @@ public static final boolean isExempted(final UUID id, final CheckType checkType)
      * @param checkType
      *            This can be individual check types, as well as a check group
      *            like MOVING or ALL.
+     * @param isPrimaryThread
+     *            If set to true, this has to be the primary server thread, as
+     *            returned by Bukkit.isPrimaryThread(). If set to false,
+     *            meta data can't be checked!
      * @return If the player is exempted from the check right now.
      */
-    public static final boolean isExempted(final Player player, final CheckType checkType) {
+    public static final boolean isExempted(final Player player, final CheckType checkType,
+            final boolean isPrimaryThread) {
         return isExempted(player.getUniqueId(), checkType) 
-                || settings.isExemptedBySettings(player, Bukkit.isPrimaryThread());
+                || settings.isExemptedBySettings(player, isPrimaryThread);
     }
 
     /**

NCPCore/src/main/java/fr/neatmonster/nocheatplus/utilities/CheckUtils.java

@@ -129,6 +129,8 @@ else if (!cc.isEnabled(checkType)) {
     /**
      * Check for exemption by permissions, API access, possibly other. Meant
      * thread-safe.
+     * 
+     * @see #hasBypass(CheckType, Player, ICheckData, boolean)
      *
      * @param checkType
      *            the check type
@@ -141,12 +143,35 @@ else if (!cc.isEnabled(checkType)) {
      */
     public static boolean hasBypass(final CheckType checkType, final Player player, final ICheckData data) {
         // TODO: Checking for the thread might be a temporary measure.
+        return hasBypass(checkType, player, data, Bukkit.isPrimaryThread());
+    }
+
+    /**
+     * Check for exemption by permissions, API access, possibly other. Meant
+     * thread-safe.
+     *
+     * @param checkType
+     *            the check type
+     * @param player
+     *            the player
+     * @param data
+     *            If data is null, the data factory will be used for the given
+     *            check type.
+     * @param isPrimaryThread
+     *            If set to true, this must be the primary server thread as
+     *            returned by Bukkit.isPrimaryThread().
+     * @return true, if successful
+     */
+    public static boolean hasBypass(final CheckType checkType, final Player player, final ICheckData data,
+            final boolean isPrimaryThread) {
+        // TODO: Checking for the thread might be a temporary measure.
         final String permission =  checkType.getPermission();
-        if (Bukkit.isPrimaryThread()) {
+        if (isPrimaryThread) {
             if (permission != null && player.hasPermission(permission)) {
                 return true;
             }
-        } else if (permission != null) {
+        }
+        else if (permission != null) {
             if (data == null) {
                 if (checkType.hasCachedPermission(player, permission)) {
                     return true;
@@ -162,7 +187,7 @@ else if (data.hasCachedPermission(permission)) {
         }
         // TODO: ExemptionManager relies on the initial definition for which type can be checked off main thread.
         // TODO: Maybe a solution: force sync into primary thread a) each time b) once with lazy force set to use copy on write [for the player or global?]. 
-        return NCPExemptionManager.isExempted(player, checkType);
+        return NCPExemptionManager.isExempted(player, checkType, isPrimaryThread);
     }
 
     /**