From a76d462eae0ce73bde0f9e9307562e32de3ad425 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 24 Mar 2019 17:47:23 -0400 Subject: [PATCH] Timings Improvements use a better stack for managing timing tree relationships remove unnecessary synchronization (i forgot HANDLERS is only touched on main) this should hopefully resolve any data integrity concerns --- ...hots-for-Timings-Tile-Entity-reports.patch | 2 +- Spigot-API-Patches/Timings-v2.patch | 121 +++++++++--------- .../Use-ASM-for-event-executors.patch | 5 +- 3 files changed, 66 insertions(+), 62 deletions(-) diff --git a/Spigot-API-Patches/Don-t-use-snapshots-for-Timings-Tile-Entity-reports.patch b/Spigot-API-Patches/Don-t-use-snapshots-for-Timings-Tile-Entity-reports.patch index 2e5c4254d3..bbbbad46d4 100644 --- a/Spigot-API-Patches/Don-t-use-snapshots-for-Timings-Tile-Entity-reports.patch +++ b/Spigot-API-Patches/Don-t-use-snapshots-for-Timings-Tile-Entity-reports.patch @@ -5,7 +5,7 @@ Subject: [PATCH] Don't use snapshots for Timings Tile Entity reports diff --git a/src/main/java/co/aikar/timings/TimingHistory.java b/src/main/java/co/aikar/timings/TimingHistory.java -index 8726bb9a2..21b929f94 100644 +index 043695239..99815866b 100644 --- a/src/main/java/co/aikar/timings/TimingHistory.java +++ b/src/main/java/co/aikar/timings/TimingHistory.java @@ -0,0 +0,0 @@ public class TimingHistory { diff --git a/Spigot-API-Patches/Timings-v2.patch b/Spigot-API-Patches/Timings-v2.patch index 739af6e982..ebdcb37849 100644 --- a/Spigot-API-Patches/Timings-v2.patch +++ b/Spigot-API-Patches/Timings-v2.patch @@ -6,7 +6,7 @@ Subject: [PATCH] Timings v2 diff --git a/src/main/java/co/aikar/timings/FullServerTickHandler.java b/src/main/java/co/aikar/timings/FullServerTickHandler.java new file mode 100644 -index 000000000..f6d616732 +index 000000000..64531fcce --- /dev/null +++ b/src/main/java/co/aikar/timings/FullServerTickHandler.java @@ -0,0 +0,0 @@ @@ -65,8 +65,7 @@ index 000000000..f6d616732 + long start = System.nanoTime(); + TimingsManager.tick(); + long diff = System.nanoTime() - start; -+ CURRENT = TIMINGS_TICK; -+ TIMINGS_TICK.addDiff(diff); ++ TIMINGS_TICK.addDiff(diff, null); + // addDiff for TIMINGS_TICK incremented this, bring it back down to 1 per tick. + record.setCurTickCount(record.getCurTickCount()-1); + @@ -259,7 +258,7 @@ index 000000000..68f7866e1 +} diff --git a/src/main/java/co/aikar/timings/Timing.java b/src/main/java/co/aikar/timings/Timing.java new file mode 100644 -index 000000000..a562796ce +index 000000000..a21e5ead5 --- /dev/null +++ b/src/main/java/co/aikar/timings/Timing.java @@ -0,0 +0,0 @@ @@ -330,8 +329,9 @@ index 000000000..a562796ce + void stopTimingIfSync(); + + /** -+ * Stops timing and disregards current timing data. ++ * @deprecated Doesn't do anything - Removed + */ ++ @Deprecated + void abort(); + + /** @@ -475,7 +475,7 @@ index 000000000..a5d13a1e4 +} diff --git a/src/main/java/co/aikar/timings/TimingHandler.java b/src/main/java/co/aikar/timings/TimingHandler.java new file mode 100644 -index 000000000..6bf8cb688 +index 000000000..4e6994098 --- /dev/null +++ b/src/main/java/co/aikar/timings/TimingHandler.java @@ -0,0 +0,0 @@ @@ -506,16 +506,23 @@ index 000000000..6bf8cb688 + +import co.aikar.util.LoadingIntMap; +import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; ++ ++import java.util.ArrayDeque; ++import java.util.Deque; +import java.util.concurrent.atomic.AtomicInteger; ++import java.util.logging.Level; ++import java.util.logging.Logger; ++ +import org.bukkit.Bukkit; +import org.jetbrains.annotations.NotNull; + +class TimingHandler implements Timing { + + private static AtomicInteger idPool = new AtomicInteger(1); ++ static Deque TIMING_STACK = new ArrayDeque<>(); + final int id = idPool.getAndIncrement(); + -+ final String name; ++ final TimingIdentifier identifier; + private final boolean verbose; + + private final Int2ObjectOpenHashMap children = new LoadingIntMap<>(TimingData::new); @@ -528,17 +535,10 @@ index 000000000..6bf8cb688 + private boolean added; + private boolean timed; + private boolean enabled; -+ private TimingHandler parent; + + TimingHandler(@NotNull TimingIdentifier id) { -+ if (id.name.startsWith("##")) { -+ verbose = true; -+ this.name = id.name.substring(3); -+ } else { -+ this.name = id.name; -+ verbose = false; -+ } -+ ++ this.identifier = id; ++ this.verbose = id.name.startsWith("##"); + this.record = new TimingData(this.id); + this.groupHandler = id.groupHandler; + @@ -579,33 +579,34 @@ index 000000000..6bf8cb688 + public Timing startTiming() { + if (enabled && Bukkit.isPrimaryThread() && ++timingDepth == 1) { + start = System.nanoTime(); -+ parent = TimingsManager.CURRENT; -+ TimingsManager.CURRENT = this; ++ TIMING_STACK.addLast(this); + } + return this; + } + + public void stopTiming() { + if (enabled && timingDepth > 0 && Bukkit.isPrimaryThread() && --timingDepth == 0 && start != 0) { -+ addDiff(System.nanoTime() - start); ++ TimingHandler last = TIMING_STACK.removeLast(); ++ if (last != this) { ++ Logger.getGlobal().log(Level.SEVERE, "TIMING_STACK_CORRUPTION - Report this to Paper! ( " + this.identifier + ":" + last +")", new Throwable()); ++ TIMING_STACK.addLast(last); // Add it back ++ } ++ addDiff(System.nanoTime() - start, TIMING_STACK.peekLast()); ++ + start = 0; + } + } + + @Override -+ public void abort() { -+ if (enabled && timingDepth > 0) { -+ start = 0; -+ } ++ public final void abort() { ++ + } + -+ void addDiff(long diff) { -+ if (TimingsManager.CURRENT == this) { -+ TimingsManager.CURRENT = parent; -+ if (parent != null) { -+ parent.children.get(id).add(diff); -+ } ++ void addDiff(long diff, TimingHandler parent) { ++ if (parent != null) { ++ parent.children.get(id).add(diff); + } ++ + record.add(diff); + if (!added) { + added = true; @@ -613,7 +614,7 @@ index 000000000..6bf8cb688 + TimingsManager.HANDLERS.add(this); + } + if (groupHandler != null) { -+ groupHandler.addDiff(diff); ++ groupHandler.addDiff(diff, parent); + groupHandler.children.get(id).add(diff); + } + } @@ -681,7 +682,7 @@ index 000000000..6bf8cb688 +} diff --git a/src/main/java/co/aikar/timings/TimingHistory.java b/src/main/java/co/aikar/timings/TimingHistory.java new file mode 100644 -index 000000000..8726bb9a2 +index 000000000..043695239 --- /dev/null +++ b/src/main/java/co/aikar/timings/TimingHistory.java @@ -0,0 +0,0 @@ @@ -779,16 +780,13 @@ index 000000000..8726bb9a2 + } + this.totalTicks = ticks; + this.totalTime = FULL_SERVER_TICK.record.getTotalTime(); -+ synchronized (TimingsManager.HANDLERS) { -+ this.entries = new TimingHistoryEntry[TimingsManager.HANDLERS.size()]; ++ this.entries = new TimingHistoryEntry[TimingsManager.HANDLERS.size()]; + -+ int i = 0; -+ for (TimingHandler handler : TimingsManager.HANDLERS) { -+ entries[i++] = new TimingHistoryEntry(handler); -+ } ++ int i = 0; ++ for (TimingHandler handler : TimingsManager.HANDLERS) { ++ entries[i++] = new TimingHistoryEntry(handler); + } + -+ + // Information about all loaded chunks/entities + //noinspection unchecked + this.worlds = toObjectMapper(Bukkit.getWorlds(), new Function() { @@ -1108,7 +1106,7 @@ index 000000000..86d5ac6bd +} diff --git a/src/main/java/co/aikar/timings/TimingIdentifier.java b/src/main/java/co/aikar/timings/TimingIdentifier.java new file mode 100644 -index 000000000..4d140f344 +index 000000000..df142a89b --- /dev/null +++ b/src/main/java/co/aikar/timings/TimingIdentifier.java @@ -0,0 +0,0 @@ @@ -1197,6 +1195,11 @@ index 000000000..4d140f344 + return hashCode; + } + ++ @Override ++ public String toString() { ++ return "TimingIdentifier{id=" + group + ":" + name +'}'; ++ } ++ + static class TimingGroup { + + private static AtomicInteger idPool = new AtomicInteger(1); @@ -1652,7 +1655,7 @@ index 000000000..c0d8f2016 +} diff --git a/src/main/java/co/aikar/timings/TimingsExport.java b/src/main/java/co/aikar/timings/TimingsExport.java new file mode 100644 -index 000000000..4a69074a8 +index 000000000..65d312b02 --- /dev/null +++ b/src/main/java/co/aikar/timings/TimingsExport.java @@ -0,0 +0,0 @@ @@ -1820,9 +1823,14 @@ index 000000000..4a69074a8 + if (!id.isTimed() && !id.isSpecial()) { + continue; + } ++ ++ String name = id.identifier.name; ++ if (name.startsWith("##")) { ++ name = name.substring(3); ++ } + handlers.put(id.id, toArray( + group.id, -+ id.name ++ name + )); + } + } @@ -2008,7 +2016,7 @@ index 000000000..4a69074a8 +} diff --git a/src/main/java/co/aikar/timings/TimingsManager.java b/src/main/java/co/aikar/timings/TimingsManager.java new file mode 100644 -index 000000000..0109065d3 +index 000000000..ef824d701 --- /dev/null +++ b/src/main/java/co/aikar/timings/TimingsManager.java @@ -0,0 +0,0 @@ @@ -2064,11 +2072,10 @@ index 000000000..0109065d3 + public static List hiddenConfigs = new ArrayList(); + public static boolean privacy = false; + -+ static final List HANDLERS = Collections.synchronizedList(new ArrayList<>(1024)); ++ static final List HANDLERS = new ArrayList<>(1024); + static final List MINUTE_REPORTS = new ArrayList<>(64); + + static EvictingQueue HISTORY = EvictingQueue.create(12); -+ static TimingHandler CURRENT; + static long timingStart = 0; + static long historyStart = 0; + static boolean needsFullReset = false; @@ -2091,14 +2098,12 @@ index 000000000..0109065d3 + if (Timings.timingsEnabled) { + boolean violated = FULL_SERVER_TICK.isViolated(); + -+ synchronized (HANDLERS) { -+ for (TimingHandler handler : HANDLERS) { -+ if (handler.isSpecial()) { -+ // We manually call this -+ continue; -+ } -+ handler.processTick(violated); ++ for (TimingHandler handler : HANDLERS) { ++ if (handler.isSpecial()) { ++ // We manually call this ++ continue; + } ++ handler.processTick(violated); + } + + TimingHistory.playerTicks += Bukkit.getOnlinePlayers().size(); @@ -2135,10 +2140,8 @@ index 000000000..0109065d3 + } else { + // Soft resets only need to act on timings that have done something + // Handlers can only be modified on main thread. -+ synchronized (HANDLERS) { -+ for (TimingHandler timings : HANDLERS) { -+ timings.reset(false); -+ } ++ for (TimingHandler timings : HANDLERS) { ++ timings.reset(false); + } + } + @@ -2391,7 +2394,7 @@ index 000000000..80155072d +} diff --git a/src/main/java/co/aikar/util/JSONUtil.java b/src/main/java/co/aikar/util/JSONUtil.java new file mode 100644 -index 000000000..e43599b47 +index 000000000..190bf0598 --- /dev/null +++ b/src/main/java/co/aikar/util/JSONUtil.java @@ -0,0 +0,0 @@ @@ -2441,7 +2444,7 @@ index 000000000..e43599b47 + * @return Map + */ + @NotNull -+ public static Map createObject(@NotNull JSONPair... data) { ++ public static Map createObject(@NotNull JSONPair... data) { + return appendObjectData(new LinkedHashMap(), data); + } + @@ -2453,7 +2456,7 @@ index 000000000..e43599b47 + * @return Map + */ + @NotNull -+ public static Map appendObjectData(@NotNull Map parent, @NotNull JSONPair... data) { ++ public static Map appendObjectData(@NotNull Map parent, @NotNull JSONPair... data) { + for (JSONPair JSONPair : data) { + parent.put(JSONPair.key, JSONPair.val); + } @@ -3179,7 +3182,7 @@ index 000000000..f9a00aecc + } +} diff --git a/src/main/java/org/bukkit/command/Command.java b/src/main/java/org/bukkit/command/Command.java -index b354b617f..01ea1ca67 100644 +index dafc4ed93..c2c19ed42 100644 --- a/src/main/java/org/bukkit/command/Command.java +++ b/src/main/java/org/bukkit/command/Command.java @@ -0,0 +0,0 @@ public abstract class Command { diff --git a/Spigot-API-Patches/Use-ASM-for-event-executors.patch b/Spigot-API-Patches/Use-ASM-for-event-executors.patch index 4e10da1b8a..faf00eaf5d 100644 --- a/Spigot-API-Patches/Use-ASM-for-event-executors.patch +++ b/Spigot-API-Patches/Use-ASM-for-event-executors.patch @@ -29,7 +29,7 @@ index 524231ecc..082f2f0c5 100644 diff --git a/src/main/java/com/destroystokyo/paper/event/executor/MethodHandleEventExecutor.java b/src/main/java/com/destroystokyo/paper/event/executor/MethodHandleEventExecutor.java new file mode 100644 -index 000000000..72318d42d +index 000000000..5b28e9b1d --- /dev/null +++ b/src/main/java/com/destroystokyo/paper/event/executor/MethodHandleEventExecutor.java @@ -0,0 +0,0 @@ @@ -39,6 +39,7 @@ index 000000000..72318d42d +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Method; + ++import com.destroystokyo.paper.util.SneakyThrow; +import org.bukkit.event.Event; +import org.bukkit.event.EventException; +import org.bukkit.event.Listener; @@ -70,7 +71,7 @@ index 000000000..72318d42d + try { + handle.invoke(listener, event); + } catch (Throwable t) { -+ throw new EventException(t); ++ SneakyThrow.sneaky(t); + } + } +}