From 2fb51842947c6ac690cc0a08b645a7b45daceac0 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Tue, 9 Nov 2021 17:18:01 -0800 Subject: [PATCH 1/4] fix(GameScheduler): enable metrics before scheduler creation --- .../engine/core/subsystem/common/MonitoringSubsystem.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java index 5b118a72f3d..ee571354efc 100644 --- a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java @@ -77,6 +77,7 @@ public Duration step() { }, Clock.SYSTEM); Metrics.addRegistry(jmxMeterRegistry); + // If we want to make global metrics available to our custom view, // we add our custom registry to the global composite: // From 8e57d792f715071cff381fd0b06d27946ccdec1a Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sat, 13 Nov 2021 14:44:36 -0800 Subject: [PATCH 2/4] doc(MonitoringSubsystem): document micrometer-related methods --- .../engine/core/subsystem/common/MonitoringSubsystem.java | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java index ee571354efc..5b118a72f3d 100644 --- a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java @@ -77,7 +77,6 @@ public Duration step() { }, Clock.SYSTEM); Metrics.addRegistry(jmxMeterRegistry); - // If we want to make global metrics available to our custom view, // we add our custom registry to the global composite: // From e34cd75a161e75fb17249224d722393634e311d1 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Sun, 14 Nov 2021 22:10:52 -0800 Subject: [PATCH 3/4] feat(MonitoringSubsystem): provide Micrometer gauges for rendering.world --- .../core/subsystem/common/GaugeMapEntry.java | 20 ++++ .../core/subsystem/common/GaugeSpec.java | 82 +++++++++++++++++ .../subsystem/common/MonitoringSubsystem.java | 92 +++++++++++++++---- .../engine/rendering/world/Meters.java | 52 +++++++++++ .../rendering/world/RenderableWorldImpl.java | 8 +- .../rendering/world/WorldRendererImpl.java | 8 +- 6 files changed, 235 insertions(+), 27 deletions(-) create mode 100644 engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeMapEntry.java create mode 100644 engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeSpec.java create mode 100644 engine/src/main/java/org/terasology/engine/rendering/world/Meters.java diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeMapEntry.java b/engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeMapEntry.java new file mode 100644 index 00000000000..cdd295e07ad --- /dev/null +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeMapEntry.java @@ -0,0 +1,20 @@ +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.engine.core.subsystem.common; + +import java.util.Set; + +/** + * Describes the set of gauges that may apply to a particular interface. + */ +public class GaugeMapEntry { + public final Class iface; + public final Set> gaugeSpecs; + + @SafeVarargs + public GaugeMapEntry(Class iface, GaugeSpec... gaugeSpecs) { + this.iface = iface; + this.gaugeSpecs = Set.of(gaugeSpecs); + } +} diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeSpec.java b/engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeSpec.java new file mode 100644 index 00000000000..758724343a1 --- /dev/null +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/common/GaugeSpec.java @@ -0,0 +1,82 @@ +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.engine.core.subsystem.common; + +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.binder.MeterBinder; + +import java.util.function.ToDoubleFunction; + +import static com.google.common.base.Preconditions.checkArgument; + +/** + * The information that defines a Gauge. + *

+ * The Micrometer API doesn't let you define a Gauge without connecting it to + * some MeterRegistry. This class provides an immutable record of all* + * the properties of a Gauge, facilitating a more data-driven approach. + *

+ * * All the ones we use so far, anyway. + * + * @param the type this gauge reads from + */ +public class GaugeSpec { + public final String name; + public final String description; + public final ToDoubleFunction valueFunction; + public final String baseUnit; + + protected final Class subjectType; + + public GaugeSpec(String name, String description, ToDoubleFunction valueFunction) { + this(name, description, valueFunction, null); + } + + /** @see Gauge.Builder */ + public GaugeSpec(String name, String description, ToDoubleFunction valueFunction, String baseUnit) { + this.name = name; + this.description = description; + this.valueFunction = valueFunction; + this.baseUnit = baseUnit; + this.subjectType = getSubjectClass(); + } + + public Gauge register(MeterRegistry registry, T subject) { + return Gauge.builder(name, subject, valueFunction) + .description(description) + .baseUnit(baseUnit) + .register(registry); + } + + /** + * Creates a MeterBinder for this gauge. + *

+ * This allows us to make things with the same interface as the meters + * provided by {@link io.micrometer.core.instrument.binder}. + * + * @param subject passed to this gauge's {@link #valueFunction} + * @return call to bind this gauge to a MeterRegistry + */ + public MeterBinder binder(T subject) { + return registry -> register(registry, subject); + } + + public MeterBinder binderAfterCasting(U subject) { + checkArgument(isInstanceOfType(subject)); + T safeSubject = subjectType.cast(subject); + return binder(safeSubject); + } + + public boolean isInstanceOfType(Object object) { + return subjectType.isInstance(object); + } + + @SafeVarargs + private Class getSubjectClass(T...t) { + // Thank you https://stackoverflow.com/a/40917725 for this amazing kludge + //noinspection unchecked + return (Class) t.getClass().getComponentType(); + } +} diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java index 5b118a72f3d..2120544cfeb 100644 --- a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java @@ -6,6 +6,7 @@ import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.binder.MeterBinder; import io.micrometer.core.instrument.binder.jvm.ClassLoaderMetrics; import io.micrometer.core.instrument.binder.jvm.JvmGcMetrics; import io.micrometer.core.instrument.binder.jvm.JvmMemoryMetrics; @@ -13,19 +14,30 @@ import io.micrometer.core.instrument.binder.system.ProcessorMetrics; import io.micrometer.jmx.JmxConfig; import io.micrometer.jmx.JmxMeterRegistry; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.terasology.engine.config.SystemConfig; import org.terasology.engine.context.Context; import org.terasology.engine.core.GameEngine; import org.terasology.engine.core.Time; import org.terasology.engine.core.subsystem.EngineSubsystem; import org.terasology.engine.monitoring.gui.AdvancedMonitor; +import reactor.core.publisher.Flux; +import reactor.function.TupleUtils; +import reactor.util.function.Tuple2; +import reactor.util.function.Tuples; import java.time.Duration; +import java.util.List; +import java.util.Set; public class MonitoringSubsystem implements EngineSubsystem { public static final Duration JMX_INTERVAL = Duration.ofSeconds(5); + private static final Logger logger = LoggerFactory.getLogger(MonitoringSubsystem.class); + + protected MeterRegistry meterRegistry; private AdvancedMonitor advancedMonitor; @Override @@ -39,29 +51,23 @@ public void initialise(GameEngine engine, Context rootContext) { advancedMonitor = new AdvancedMonitor(); advancedMonitor.setVisible(true); } + meterRegistry = initMeterRegistries(); + } - initMicrometerMetrics(rootContext.get(Time.class)); + @Override + public void postInitialise(Context context) { + initMeters(context); } /** * Initialize Micrometer metrics and publishers. * * @see org.terasology.engine.core.GameScheduler GameScheduler for global Reactor metrics - * @param time provides statistics - *

- * Note {@link org.terasology.engine.core.EngineTime EngineTime} - * does not serve the same role as a {@link Clock micrometer Clock}. - * (Not yet. Maybe it should?) */ - private void initMicrometerMetrics(Time time) { + protected MeterRegistry initMeterRegistries() { // Register metrics with the built-in global composite registry. // This makes them available to any agent(s) we add to it. - MeterRegistry meterRegistry = Metrics.globalRegistry; - - Gauge.builder("terasology.fps", time::getFps) - .description("framerate") - .baseUnit("Hz") - .register(meterRegistry); + MeterRegistry registry = Metrics.globalRegistry; // Publish the global metrics registry on a JMX server. MeterRegistry jmxMeterRegistry = new JmxMeterRegistry(new JmxConfig() { @@ -77,6 +83,7 @@ public Duration step() { }, Clock.SYSTEM); Metrics.addRegistry(jmxMeterRegistry); + return registry; // If we want to make global metrics available to our custom view, // we add our custom registry to the global composite: // @@ -84,7 +91,50 @@ public Duration step() { // // If we want to see JVM metrics there as well: // - // initAllJvmMetrics(DebugOverlay.meterRegistry); + // allJvmMetrics().forEach(m -> m.bindTo(DebugOverlay.meterRegistry)); + } + + /** Initialize meters for all the things in this Context. */ + protected void initMeters(Context context) { + // We can build meters individually like this: + var time = context.get(Time.class); + Gauge.builder("terasology.fps", time::getFps) + .description("framerate") + .baseUnit("Hz") + .register(meterRegistry); + + // But we'd like the code for meters to live closer to the implementation + // of the thing they're monitoring. + // + // Somewhere we get a list of all the things that provide meters. + // Maybe hardcoded, maybe a registry of some kind? Modules will want + // to contribute as well. + var meterMaps = List.of( + org.terasology.engine.rendering.world.Meters.GAUGE_MAP + ); + + meterMaps.forEach(gaugeMap -> registerForContext(context, gaugeMap)); + } + + protected void registerForContext(Context context, Iterable gaugeMap) { + Flux.fromIterable(gaugeMap) + .map(entry -> Tuples.of(context.get(entry.iface), entry.gaugeSpecs)) + .filter(TupleUtils.predicate((subject, specs) -> subject != null)) + .doOnDiscard(Tuple2.class, TupleUtils.consumer((iface, gaugeSpecs) -> + logger.debug("Not building gauges for {}, none was in {}", iface, context))) + .subscribe(TupleUtils.consumer(this::registerAll)); + } + + protected void registerAll(T subject, Set> gaugeSpecs) { + Flux.fromIterable(gaugeSpecs) + .filter(spec -> spec.isInstanceOfType(subject)) + // Make sure the gauge is right for the specific type. + .map(spec -> spec.binderAfterCasting(subject)) + .subscribe(this::registerMeter); + } + + public void registerMeter(MeterBinder meterBinder) { + meterBinder.bindTo(meterRegistry); } /** @@ -95,12 +145,14 @@ public Duration step() { * have a different agent you want them published through. */ @SuppressWarnings("unused") - void initAllJvmMetrics(MeterRegistry registry) { - new ClassLoaderMetrics().bindTo(registry); - new JvmMemoryMetrics().bindTo(registry); - new JvmGcMetrics().bindTo(registry); - new JvmThreadMetrics().bindTo(registry); - new ProcessorMetrics().bindTo(registry); + List allJvmMetrics() { + return List.of( + new ClassLoaderMetrics(), + new JvmMemoryMetrics(), + new JvmGcMetrics(), + new JvmThreadMetrics(), + new ProcessorMetrics() + ); } @Override diff --git a/engine/src/main/java/org/terasology/engine/rendering/world/Meters.java b/engine/src/main/java/org/terasology/engine/rendering/world/Meters.java new file mode 100644 index 00000000000..9f3e4309d74 --- /dev/null +++ b/engine/src/main/java/org/terasology/engine/rendering/world/Meters.java @@ -0,0 +1,52 @@ +// Copyright 2021 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.engine.rendering.world; + +import io.micrometer.core.instrument.binder.BaseUnits; +import org.terasology.engine.core.subsystem.common.GaugeMapEntry; +import org.terasology.engine.core.subsystem.common.GaugeSpec; + +import java.util.List; + +public final class Meters { + public static final String PREFIX = Meters.class.getPackageName(); + + public static final List GAUGE_MAP = List.of( + new GaugeMapEntry(WorldRenderer.class, + new GaugeSpec( + PREFIX + ".emptyMeshChunks", + "Empty Mesh Chunks", + wri -> wri.statChunkMeshEmpty, + BaseUnits.OBJECTS), + new GaugeSpec( + PREFIX + ".unreadyChunks", + "Unready Chunks", + wri -> wri.statChunkNotReady, + BaseUnits.OBJECTS), + new GaugeSpec( + PREFIX + ".triangles", + "Rendered Triangles", + wri -> wri.statRenderedTriangles, + BaseUnits.OBJECTS) + ), + new GaugeMapEntry(RenderableWorld.class, + new GaugeSpec( + PREFIX + ".visibleChunks", + "Visible Chunks", + rwi -> rwi.statVisibleChunks, + BaseUnits.OBJECTS), + new GaugeSpec( + PREFIX + ".dirtyChunks", + "Dirty Chunks", + rwi -> rwi.statDirtyChunks, + BaseUnits.OBJECTS), + new GaugeSpec( + PREFIX + ".dirtyChunks", + "Ignored Phases", + rwi -> rwi.statIgnoredPhases) + ) + ); + + private Meters() { } +} diff --git a/engine/src/main/java/org/terasology/engine/rendering/world/RenderableWorldImpl.java b/engine/src/main/java/org/terasology/engine/rendering/world/RenderableWorldImpl.java index 446eaa74847..10e3994c66e 100644 --- a/engine/src/main/java/org/terasology/engine/rendering/world/RenderableWorldImpl.java +++ b/engine/src/main/java/org/terasology/engine/rendering/world/RenderableWorldImpl.java @@ -50,6 +50,10 @@ class RenderableWorldImpl implements RenderableWorld { ViewDistance.MEGA.getChunkDistance().x() * ViewDistance.MEGA.getChunkDistance().y() * ViewDistance.MEGA.getChunkDistance().z(); private static final Vector3fc CHUNK_CENTER_OFFSET = new Vector3f(Chunks.CHUNK_SIZE).div(2); + int statDirtyChunks; + int statVisibleChunks; + int statIgnoredPhases; + private final int maxChunksForShadows = TeraMath.clamp(CoreRegistry.get(Config.class).getRendering().getMaxChunksUsedForShadowMapping(), 64, 1024); @@ -71,10 +75,6 @@ class RenderableWorldImpl implements RenderableWorld { private final Config config = CoreRegistry.get(Config.class); private final RenderingConfig renderingConfig = config.getRendering(); - private int statDirtyChunks; - private int statVisibleChunks; - private int statIgnoredPhases; - RenderableWorldImpl(Context context, Camera playerCamera) { diff --git a/engine/src/main/java/org/terasology/engine/rendering/world/WorldRendererImpl.java b/engine/src/main/java/org/terasology/engine/rendering/world/WorldRendererImpl.java index b9ea118c7d0..5f17550cad7 100644 --- a/engine/src/main/java/org/terasology/engine/rendering/world/WorldRendererImpl.java +++ b/engine/src/main/java/org/terasology/engine/rendering/world/WorldRendererImpl.java @@ -69,6 +69,11 @@ public final class WorldRendererImpl implements WorldRenderer { */ private static final Logger logger = LoggerFactory.getLogger(WorldRendererImpl.class); private static final float GROUND_PLANE_HEIGHT_DISPARITY = -0.7f; + + int statChunkMeshEmpty; + int statChunkNotReady; + int statRenderedTriangles; + private RenderGraph renderGraph; private RenderingModuleRegistry renderingModuleRegistry; @@ -88,9 +93,6 @@ public final class WorldRendererImpl implements WorldRenderer { private float millisecondsSinceRenderingStart; private float secondsSinceLastFrame; - private int statChunkMeshEmpty; - private int statChunkNotReady; - private int statRenderedTriangles; private final RenderingConfig renderingConfig; private final Console console; From 3d7ba241bd0afcf29f2436496a406ea6d85f0727 Mon Sep 17 00:00:00 2001 From: Kevin Turner <83819+keturn@users.noreply.github.com> Date: Mon, 15 Nov 2021 10:15:48 -0800 Subject: [PATCH 4/4] fix(MonitoringSubsystem): move meter creation to ComponentSystem.preBegin EngineSubsystem initialization is too early to see all the systems we want to monitor. --- .../subsystem/common/MonitoringSubsystem.java | 33 ++++++++++--------- .../ingame/metrics/DebugMetricsSystem.java | 24 +++++++++++++- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java index 2120544cfeb..51aa237ce86 100644 --- a/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java +++ b/engine/src/main/java/org/terasology/engine/core/subsystem/common/MonitoringSubsystem.java @@ -23,9 +23,6 @@ import org.terasology.engine.core.subsystem.EngineSubsystem; import org.terasology.engine.monitoring.gui.AdvancedMonitor; import reactor.core.publisher.Flux; -import reactor.function.TupleUtils; -import reactor.util.function.Tuple2; -import reactor.util.function.Tuples; import java.time.Duration; import java.util.List; @@ -45,6 +42,12 @@ public String getName() { return "Monitoring"; } + @Override + public void preInitialise(Context rootContext) { + // FIXME: `@Share` is not implemented for EngineSubsystems? + rootContext.put(MonitoringSubsystem.class, this); + } + @Override public void initialise(GameEngine engine, Context rootContext) { if (rootContext.get(SystemConfig.class).monitoringEnabled.get()) { @@ -54,11 +57,6 @@ public void initialise(GameEngine engine, Context rootContext) { meterRegistry = initMeterRegistries(); } - @Override - public void postInitialise(Context context) { - initMeters(context); - } - /** * Initialize Micrometer metrics and publishers. * @@ -95,7 +93,7 @@ public Duration step() { } /** Initialize meters for all the things in this Context. */ - protected void initMeters(Context context) { + public void initMeters(Context context) { // We can build meters individually like this: var time = context.get(Time.class); Gauge.builder("terasology.fps", time::getFps) @@ -117,15 +115,17 @@ protected void initMeters(Context context) { } protected void registerForContext(Context context, Iterable gaugeMap) { - Flux.fromIterable(gaugeMap) - .map(entry -> Tuples.of(context.get(entry.iface), entry.gaugeSpecs)) - .filter(TupleUtils.predicate((subject, specs) -> subject != null)) - .doOnDiscard(Tuple2.class, TupleUtils.consumer((iface, gaugeSpecs) -> - logger.debug("Not building gauges for {}, none was in {}", iface, context))) - .subscribe(TupleUtils.consumer(this::registerAll)); + for (GaugeMapEntry entry : gaugeMap) { + var subject = context.get(entry.iface); + if (subject != null) { + registerAll(subject, entry.gaugeSpecs); + } else { + logger.warn("Not building gauges for {}, none was in {}", entry.iface, context); + } + } } - protected void registerAll(T subject, Set> gaugeSpecs) { + public void registerAll(T subject, Set> gaugeSpecs) { Flux.fromIterable(gaugeSpecs) .filter(spec -> spec.isInstanceOfType(subject)) // Make sure the gauge is right for the specific type. @@ -134,6 +134,7 @@ protected void registerAll(T subject, Set> gaugeSpecs } public void registerMeter(MeterBinder meterBinder) { + logger.debug("Binding {} to {}", meterBinder, meterRegistry); meterBinder.bindTo(meterRegistry); } diff --git a/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/metrics/DebugMetricsSystem.java b/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/metrics/DebugMetricsSystem.java index 4a82c26ba37..799b25fa64d 100644 --- a/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/metrics/DebugMetricsSystem.java +++ b/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/metrics/DebugMetricsSystem.java @@ -3,10 +3,13 @@ package org.terasology.engine.rendering.nui.layers.ingame.metrics; import com.google.common.base.Preconditions; +import org.terasology.engine.context.Context; +import org.terasology.engine.core.subsystem.common.MonitoringSubsystem; import org.terasology.engine.entitySystem.systems.BaseComponentSystem; import org.terasology.engine.entitySystem.systems.RegisterSystem; -import org.terasology.gestalt.module.sandbox.API; +import org.terasology.engine.registry.In; import org.terasology.engine.registry.Share; +import org.terasology.gestalt.module.sandbox.API; import java.util.ArrayList; @@ -25,6 +28,12 @@ @API public class DebugMetricsSystem extends BaseComponentSystem { + @In + MonitoringSubsystem monitoringSubsystem; + + @In + Context context; + private final MetricsMode defaultMode = new NullMetricsMode(); private ArrayList modes; private MetricsMode currentMode; @@ -43,8 +52,21 @@ public void initialise() { register(new HeapAllocationMode()); register(new RenderingExecTimeMeansMode("\n- Rendering - Execution Time: Running Means - Sorted Alphabetically -")); currentMode = defaultMode; + + if (monitoringSubsystem == null) { + monitoringSubsystem = context.get(MonitoringSubsystem.class); + } } + @Override + public void preBegin() { + monitoringSubsystem.initMeters(context); + } + + @Override + public void shutdown() { + // FIXME: Remove all the meters we added to the global registry. + } /** * Adds a MetricsMode instance to the set. Use the toggle() and getCurrentMode() methods to iterate over the set and