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] 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