From 706f5bae5c3ce88097ead2e370dc2b0c066cdae5 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Tue, 28 Sep 2021 21:45:16 -0400 Subject: [PATCH 1/6] naive implementation --- .../perf/session/gauges/CpuGaugeCollector.java | 10 ++++++++-- .../firebase/perf/session/gauges/GaugeManager.java | 7 +++++-- .../perf/session/gauges/MemoryGaugeCollector.java | 10 ++++++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index 248c77bbece..3cc1a0531a0 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -75,7 +75,7 @@ public class CpuGaugeCollector { /* This is populated by CpuGaugeCollector but it's drained by GaugeManager.*/ public final ConcurrentLinkedQueue cpuMetricReadings; - private final ScheduledExecutorService cpuMetricCollectorExecutor; + private ScheduledExecutorService cpuMetricCollectorExecutor; private final String procFileName; private final long clockTicksPerSecond; @@ -84,7 +84,7 @@ public class CpuGaugeCollector { private CpuGaugeCollector() { cpuMetricReadings = new ConcurrentLinkedQueue<>(); - cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); +// cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); int pid = android.os.Process.myPid(); procFileName = "/proc/" + Integer.toString(pid) + "/stat"; @@ -169,6 +169,9 @@ private synchronized void scheduleCpuMetricCollectionWithRate( long cpuMetricCollectionRate, Timer referenceTime) { this.cpuMetricCollectionRateMs = cpuMetricCollectionRate; try { + if (cpuMetricCollectorExecutor == null) { + cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); + } cpuMetricCollectorJob = cpuMetricCollectorExecutor.scheduleAtFixedRate( () -> { @@ -187,6 +190,9 @@ private synchronized void scheduleCpuMetricCollectionWithRate( private synchronized void scheduleCpuMetricCollectionOnce(Timer referenceTime) { try { + if (cpuMetricCollectorExecutor == null) { + cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); + } @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = cpuMetricCollectorExecutor.schedule( diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index e15a9fe945e..ae77c593af3 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -50,7 +50,7 @@ public class GaugeManager { private static final long APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC = 20; private static final long INVALID_GAUGE_COLLECTION_FREQUENCY = -1; - private final ScheduledExecutorService gaugeManagerExecutor; + private ScheduledExecutorService gaugeManagerExecutor; private final ConfigResolver configResolver; private final CpuGaugeCollector cpuGaugeCollector; private final MemoryGaugeCollector memoryGaugeCollector; @@ -64,7 +64,7 @@ public class GaugeManager { private GaugeManager() { this( - Executors.newSingleThreadScheduledExecutor(), + null, TransportManager.getInstance(), ConfigResolver.getInstance(), null, @@ -133,6 +133,9 @@ public void startCollectingGauges( final ApplicationProcessState applicationProcessStateForScheduledTask = applicationProcessState; try { + if (gaugeManagerExecutor == null) { + gaugeManagerExecutor = Executors.newSingleThreadScheduledExecutor(); + } gaugeManagerDataCollectionJob = gaugeManagerExecutor.scheduleAtFixedRate( () -> { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index 5eb1dc68552..15d3c62dae2 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -48,7 +48,7 @@ public class MemoryGaugeCollector { // this value is set for the memoryMetricCollectionRateMs, we do not collect Memory Metrics. private static final int UNSET_MEMORY_METRIC_COLLECTION_RATE = -1; - private final ScheduledExecutorService memoryMetricCollectorExecutor; + private ScheduledExecutorService memoryMetricCollectorExecutor; /* This is populated by MemoryGaugeCollector but it's drained by GaugeManager.*/ public final ConcurrentLinkedQueue memoryMetricReadings; private final Runtime runtime; @@ -57,7 +57,7 @@ public class MemoryGaugeCollector { private long memoryMetricCollectionRateMs = UNSET_MEMORY_METRIC_COLLECTION_RATE; private MemoryGaugeCollector() { - this(Executors.newSingleThreadScheduledExecutor(), Runtime.getRuntime()); + this(null, Runtime.getRuntime()); } @VisibleForTesting @@ -129,6 +129,9 @@ private synchronized void scheduleMemoryMetricCollectionWithRate( this.memoryMetricCollectionRateMs = memoryMetricCollectionRate; try { + if (memoryMetricCollectorExecutor == null) { + memoryMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); + } memoryMetricCollectorJob = memoryMetricCollectorExecutor.scheduleAtFixedRate( () -> { @@ -147,6 +150,9 @@ private synchronized void scheduleMemoryMetricCollectionWithRate( private synchronized void scheduleMemoryMetricCollectionOnce(Timer referenceTime) { try { + if (memoryMetricCollectorExecutor == null) { + memoryMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); + } @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = memoryMetricCollectorExecutor.schedule( From 1ee76339da50314d11a86d0021aa8a2ee9a0e2a0 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Wed, 29 Sep 2021 18:00:39 -0400 Subject: [PATCH 2/6] use String.replace instead of String.replaceAll --- .../java/com/google/firebase/perf/session/PerfSession.java | 3 +-- .../google/firebase/perf/session/gauges/CpuGaugeCollector.java | 1 - .../com/google/firebase/perf/session/gauges/GaugeManager.java | 3 +++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java index 90d939d6771..d6f9aed0e3f 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java @@ -39,8 +39,7 @@ public class PerfSession implements Parcelable { * Creates a PerfSession object and decides what metrics to collect. */ public static PerfSession create() { - String sessionId = UUID.randomUUID().toString(); - sessionId = sessionId.replaceAll("\\-", ""); + String sessionId = UUID.randomUUID().toString().replace("-", ""); PerfSession session = new PerfSession(sessionId, new Clock()); session.setGaugeAndEventCollectionEnabled(shouldCollectGaugesAndEvents()); diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index 3cc1a0531a0..7bb03d42317 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -84,7 +84,6 @@ public class CpuGaugeCollector { private CpuGaugeCollector() { cpuMetricReadings = new ConcurrentLinkedQueue<>(); -// cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); int pid = android.os.Process.myPid(); procFileName = "/proc/" + Integer.toString(pid) + "/stat"; diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index ae77c593af3..c80e7646a06 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -201,6 +201,9 @@ public void stopCollectingGauges() { } // Flush any data that was collected for this session one last time. + if (gaugeManagerExecutor == null) { + gaugeManagerExecutor = Executors.newSingleThreadScheduledExecutor(); + } @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = gaugeManagerExecutor.schedule( From 6842500fcc9e3ef9ba6584add50aaa4123a25e34 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Thu, 30 Sep 2021 13:26:43 -0400 Subject: [PATCH 3/6] use Lazy<>() from firebase-components for cleaner code --- .../session/gauges/CpuGaugeCollector.java | 7 +-- .../perf/session/gauges/GaugeManager.java | 48 +++++++++---------- .../session/gauges/MemoryGaugeCollector.java | 8 +--- 3 files changed, 24 insertions(+), 39 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index 7bb03d42317..d5913e3aad7 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -84,6 +84,7 @@ public class CpuGaugeCollector { private CpuGaugeCollector() { cpuMetricReadings = new ConcurrentLinkedQueue<>(); + cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); int pid = android.os.Process.myPid(); procFileName = "/proc/" + Integer.toString(pid) + "/stat"; @@ -168,9 +169,6 @@ private synchronized void scheduleCpuMetricCollectionWithRate( long cpuMetricCollectionRate, Timer referenceTime) { this.cpuMetricCollectionRateMs = cpuMetricCollectionRate; try { - if (cpuMetricCollectorExecutor == null) { - cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); - } cpuMetricCollectorJob = cpuMetricCollectorExecutor.scheduleAtFixedRate( () -> { @@ -189,9 +187,6 @@ private synchronized void scheduleCpuMetricCollectionWithRate( private synchronized void scheduleCpuMetricCollectionOnce(Timer referenceTime) { try { - if (cpuMetricCollectorExecutor == null) { - cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); - } @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = cpuMetricCollectorExecutor.schedule( diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index c80e7646a06..f6b74a3935c 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -18,6 +18,8 @@ import androidx.annotation.Keep; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; + +import com.google.firebase.components.Lazy; import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.logging.AndroidLogger; import com.google.firebase.perf.session.PerfSession; @@ -50,10 +52,10 @@ public class GaugeManager { private static final long APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC = 20; private static final long INVALID_GAUGE_COLLECTION_FREQUENCY = -1; - private ScheduledExecutorService gaugeManagerExecutor; + private Lazy gaugeManagerExecutor; private final ConfigResolver configResolver; - private final CpuGaugeCollector cpuGaugeCollector; - private final MemoryGaugeCollector memoryGaugeCollector; + private final Lazy cpuGaugeCollector; + private final Lazy memoryGaugeCollector; private final TransportManager transportManager; @Nullable private GaugeMetadataManager gaugeMetadataManager; @@ -64,22 +66,22 @@ public class GaugeManager { private GaugeManager() { this( - null, + new Lazy<>(() -> Executors.newSingleThreadScheduledExecutor()), TransportManager.getInstance(), ConfigResolver.getInstance(), null, - CpuGaugeCollector.getInstance(), - MemoryGaugeCollector.getInstance()); + new Lazy<>(() -> CpuGaugeCollector.getInstance()), + new Lazy<>(() -> MemoryGaugeCollector.getInstance())); } @VisibleForTesting GaugeManager( - ScheduledExecutorService gaugeManagerExecutor, + Lazy gaugeManagerExecutor, TransportManager transportManager, ConfigResolver configResolver, GaugeMetadataManager gaugeMetadataManager, - CpuGaugeCollector cpuGaugeCollector, - MemoryGaugeCollector memoryGaugeCollector) { + Lazy cpuGaugeCollector, + Lazy memoryGaugeCollector) { this.gaugeManagerExecutor = gaugeManagerExecutor; this.transportManager = transportManager; @@ -133,11 +135,8 @@ public void startCollectingGauges( final ApplicationProcessState applicationProcessStateForScheduledTask = applicationProcessState; try { - if (gaugeManagerExecutor == null) { - gaugeManagerExecutor = Executors.newSingleThreadScheduledExecutor(); - } gaugeManagerDataCollectionJob = - gaugeManagerExecutor.scheduleAtFixedRate( + gaugeManagerExecutor.get().scheduleAtFixedRate( () -> { syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); }, @@ -193,20 +192,17 @@ public void stopCollectingGauges() { final String sessionIdForScheduledTask = sessionId; final ApplicationProcessState applicationProcessStateForScheduledTask = applicationProcessState; - cpuGaugeCollector.stopCollecting(); - memoryGaugeCollector.stopCollecting(); + cpuGaugeCollector.get().stopCollecting(); + memoryGaugeCollector.get().stopCollecting(); if (gaugeManagerDataCollectionJob != null) { gaugeManagerDataCollectionJob.cancel(false); } // Flush any data that was collected for this session one last time. - if (gaugeManagerExecutor == null) { - gaugeManagerExecutor = Executors.newSingleThreadScheduledExecutor(); - } @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = - gaugeManagerExecutor.schedule( + gaugeManagerExecutor.get().schedule( () -> { syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); }, @@ -228,13 +224,13 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) { GaugeMetric.Builder gaugeMetricBuilder = GaugeMetric.newBuilder(); // Adding CPU metric readings. - while (!cpuGaugeCollector.cpuMetricReadings.isEmpty()) { - gaugeMetricBuilder.addCpuMetricReadings(cpuGaugeCollector.cpuMetricReadings.poll()); + while (!cpuGaugeCollector.get().cpuMetricReadings.isEmpty()) { + gaugeMetricBuilder.addCpuMetricReadings(cpuGaugeCollector.get().cpuMetricReadings.poll()); } // Adding Memory metric readings. - while (!memoryGaugeCollector.memoryMetricReadings.isEmpty()) { - gaugeMetricBuilder.addAndroidMemoryReadings(memoryGaugeCollector.memoryMetricReadings.poll()); + while (!memoryGaugeCollector.get().memoryMetricReadings.isEmpty()) { + gaugeMetricBuilder.addAndroidMemoryReadings(memoryGaugeCollector.get().memoryMetricReadings.poll()); } // Adding Session ID info. @@ -290,7 +286,7 @@ private boolean startCollectingCpuMetrics( return false; } - cpuGaugeCollector.startCollecting(cpuMetricCollectionFrequency, referenceTime); + cpuGaugeCollector.get().startCollecting(cpuMetricCollectionFrequency, referenceTime); return true; } @@ -311,7 +307,7 @@ private boolean startCollectingMemoryMetrics( return false; } - memoryGaugeCollector.startCollecting(memoryMetricCollectionFrequency, referenceTime); + memoryGaugeCollector.get().startCollecting(memoryMetricCollectionFrequency, referenceTime); return true; } @@ -326,7 +322,7 @@ private boolean startCollectingMemoryMetrics( * enabled. */ public void collectGaugeMetricOnce(Timer referenceTime) { - collectGaugeMetricOnce(cpuGaugeCollector, memoryGaugeCollector, referenceTime); + collectGaugeMetricOnce(cpuGaugeCollector.get(), memoryGaugeCollector.get(), referenceTime); } private static void collectGaugeMetricOnce( diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index 15d3c62dae2..957df2cd6d2 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -57,7 +57,7 @@ public class MemoryGaugeCollector { private long memoryMetricCollectionRateMs = UNSET_MEMORY_METRIC_COLLECTION_RATE; private MemoryGaugeCollector() { - this(null, Runtime.getRuntime()); + this(Executors.newSingleThreadScheduledExecutor(), Runtime.getRuntime()); } @VisibleForTesting @@ -129,9 +129,6 @@ private synchronized void scheduleMemoryMetricCollectionWithRate( this.memoryMetricCollectionRateMs = memoryMetricCollectionRate; try { - if (memoryMetricCollectorExecutor == null) { - memoryMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); - } memoryMetricCollectorJob = memoryMetricCollectorExecutor.scheduleAtFixedRate( () -> { @@ -150,9 +147,6 @@ private synchronized void scheduleMemoryMetricCollectionWithRate( private synchronized void scheduleMemoryMetricCollectionOnce(Timer referenceTime) { try { - if (memoryMetricCollectorExecutor == null) { - memoryMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); - } @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = memoryMetricCollectorExecutor.schedule( From aaf965d716b9b7366045a42857712d67a6f99b10 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Thu, 30 Sep 2021 13:28:20 -0400 Subject: [PATCH 4/6] revert the removal of final --- .../google/firebase/perf/session/gauges/CpuGaugeCollector.java | 2 +- .../com/google/firebase/perf/session/gauges/GaugeManager.java | 2 +- .../firebase/perf/session/gauges/MemoryGaugeCollector.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index d5913e3aad7..248c77bbece 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -75,7 +75,7 @@ public class CpuGaugeCollector { /* This is populated by CpuGaugeCollector but it's drained by GaugeManager.*/ public final ConcurrentLinkedQueue cpuMetricReadings; - private ScheduledExecutorService cpuMetricCollectorExecutor; + private final ScheduledExecutorService cpuMetricCollectorExecutor; private final String procFileName; private final long clockTicksPerSecond; diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index f6b74a3935c..7aa5c364283 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -52,7 +52,7 @@ public class GaugeManager { private static final long APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC = 20; private static final long INVALID_GAUGE_COLLECTION_FREQUENCY = -1; - private Lazy gaugeManagerExecutor; + private final Lazy gaugeManagerExecutor; private final ConfigResolver configResolver; private final Lazy cpuGaugeCollector; private final Lazy memoryGaugeCollector; diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index 957df2cd6d2..5eb1dc68552 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -48,7 +48,7 @@ public class MemoryGaugeCollector { // this value is set for the memoryMetricCollectionRateMs, we do not collect Memory Metrics. private static final int UNSET_MEMORY_METRIC_COLLECTION_RATE = -1; - private ScheduledExecutorService memoryMetricCollectorExecutor; + private final ScheduledExecutorService memoryMetricCollectorExecutor; /* This is populated by MemoryGaugeCollector but it's drained by GaugeManager.*/ public final ConcurrentLinkedQueue memoryMetricReadings; private final Runtime runtime; From 691cc877324254144583c2ca6292504cf81a6208 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Thu, 30 Sep 2021 15:54:26 -0400 Subject: [PATCH 5/6] fix test and googleJavaFormat --- .../perf/session/gauges/GaugeManager.java | 35 +++++++++++-------- .../perf/session/gauges/GaugeManagerTest.java | 19 +++++----- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index 7aa5c364283..6dd8b730299 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -18,7 +18,6 @@ import androidx.annotation.Keep; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; - import com.google.firebase.components.Lazy; import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.logging.AndroidLogger; @@ -136,13 +135,16 @@ public void startCollectingGauges( try { gaugeManagerDataCollectionJob = - gaugeManagerExecutor.get().scheduleAtFixedRate( - () -> { - syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); - }, - /*initialDelay=*/ collectionFrequency * APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC, - /*period=*/ collectionFrequency * APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC, - TimeUnit.MILLISECONDS); + gaugeManagerExecutor + .get() + .scheduleAtFixedRate( + () -> { + syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); + }, + /*initialDelay=*/ collectionFrequency + * APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC, + /*period=*/ collectionFrequency * APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC, + TimeUnit.MILLISECONDS); } catch (RejectedExecutionException e) { logger.warn("Unable to start collecting Gauges: " + e.getMessage()); @@ -202,12 +204,14 @@ public void stopCollectingGauges() { // Flush any data that was collected for this session one last time. @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = - gaugeManagerExecutor.get().schedule( - () -> { - syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); - }, - TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, - TimeUnit.MILLISECONDS); + gaugeManagerExecutor + .get() + .schedule( + () -> { + syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); + }, + TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, + TimeUnit.MILLISECONDS); this.sessionId = null; this.applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; @@ -230,7 +234,8 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) { // Adding Memory metric readings. while (!memoryGaugeCollector.get().memoryMetricReadings.isEmpty()) { - gaugeMetricBuilder.addAndroidMemoryReadings(memoryGaugeCollector.get().memoryMetricReadings.poll()); + gaugeMetricBuilder.addAndroidMemoryReadings( + memoryGaugeCollector.get().memoryMetricReadings.poll()); } // Adding Session ID info. diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java index 483fc11d58e..e3e228def34 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.when; import androidx.test.core.app.ApplicationProvider; +import com.google.firebase.components.Lazy; import com.google.firebase.perf.FirebasePerformanceTestBase; import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.session.PerfSession; @@ -113,12 +114,12 @@ public void setUp() { testGaugeManager = new GaugeManager( - fakeScheduledExecutorService, + new Lazy<>(() -> fakeScheduledExecutorService), mockTransportManager, mockConfigResolver, fakeGaugeMetadataManager, - fakeCpuGaugeCollector, - fakeMemoryGaugeCollector); + new Lazy<>(() -> fakeCpuGaugeCollector), + new Lazy<>(() -> fakeMemoryGaugeCollector)); } @Test @@ -663,12 +664,12 @@ public void testLogGaugeMetadataDoesntLogWhenGaugeMetadataManagerNotAvailable() testGaugeManager = new GaugeManager( - fakeScheduledExecutorService, + new Lazy<>(() -> fakeScheduledExecutorService), mockTransportManager, mockConfigResolver, /* gaugeMetadataManager= */ null, - fakeCpuGaugeCollector, - fakeMemoryGaugeCollector); + new Lazy<>(() -> fakeCpuGaugeCollector), + new Lazy<>(() -> fakeMemoryGaugeCollector)); assertThat(testGaugeManager.logGaugeMetadata("sessionId", ApplicationProcessState.FOREGROUND)) .isFalse(); @@ -679,12 +680,12 @@ public void testLogGaugeMetadataLogsAfterApplicationContextIsSet() { testGaugeManager = new GaugeManager( - fakeScheduledExecutorService, + new Lazy<>(() -> fakeScheduledExecutorService), mockTransportManager, mockConfigResolver, /* gaugeMetadataManager= */ null, - fakeCpuGaugeCollector, - fakeMemoryGaugeCollector); + new Lazy<>(() -> fakeCpuGaugeCollector), + new Lazy<>(() -> fakeMemoryGaugeCollector)); assertThat(testGaugeManager.logGaugeMetadata("sessionId", ApplicationProcessState.FOREGROUND)) .isFalse(); From 3aa1bb71f0055e874a369c4223620430c9705c3e Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Mon, 4 Oct 2021 12:45:03 -0400 Subject: [PATCH 6/6] make GaugeCollectors non-singletons --- .../perf/session/gauges/CpuGaugeCollector.java | 12 +----------- .../firebase/perf/session/gauges/GaugeManager.java | 4 ++-- .../perf/session/gauges/MemoryGaugeCollector.java | 11 +---------- .../perf/session/gauges/GaugeManagerTest.java | 4 ++-- 4 files changed, 6 insertions(+), 25 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index 248c77bbece..c34fd31dc72 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -71,8 +71,6 @@ public class CpuGaugeCollector { // This utility isn't provided by TimeUnits.SECONDS.toMicros() - it only accepts longs. private static final long MICROSECONDS_PER_SECOND = TimeUnit.SECONDS.toMicros(1); - @Nullable private static CpuGaugeCollector instance = null; - /* This is populated by CpuGaugeCollector but it's drained by GaugeManager.*/ public final ConcurrentLinkedQueue cpuMetricReadings; private final ScheduledExecutorService cpuMetricCollectorExecutor; @@ -82,7 +80,7 @@ public class CpuGaugeCollector { @Nullable private ScheduledFuture cpuMetricCollectorJob = null; private long cpuMetricCollectionRateMs = UNSET_CPU_METRIC_COLLECTION_RATE; - private CpuGaugeCollector() { + CpuGaugeCollector() { cpuMetricReadings = new ConcurrentLinkedQueue<>(); cpuMetricCollectorExecutor = Executors.newSingleThreadScheduledExecutor(); @@ -103,14 +101,6 @@ private CpuGaugeCollector() { this.clockTicksPerSecond = clockTicksPerSecond; } - /** Returns the singleton instance of this class. */ - public static CpuGaugeCollector getInstance() { - if (instance == null) { - instance = new CpuGaugeCollector(); - } - return instance; - } - /** * Starts collecting CpuMetricReadings at the specified rate. If it is already collecting the * readings, it updates the rate at which they're being collected if a different diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index 6dd8b730299..4d03f237e65 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java @@ -69,8 +69,8 @@ private GaugeManager() { TransportManager.getInstance(), ConfigResolver.getInstance(), null, - new Lazy<>(() -> CpuGaugeCollector.getInstance()), - new Lazy<>(() -> MemoryGaugeCollector.getInstance())); + new Lazy<>(() -> new CpuGaugeCollector()), + new Lazy<>(() -> new MemoryGaugeCollector())); } @VisibleForTesting diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index 5eb1dc68552..8c3a3706a19 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -14,7 +14,6 @@ package com.google.firebase.perf.session.gauges; -import android.annotation.SuppressLint; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.firebase.perf.logging.AndroidLogger; @@ -40,9 +39,6 @@ public class MemoryGaugeCollector { private static final AndroidLogger logger = AndroidLogger.getInstance(); - @SuppressLint("StaticFieldLeak") - private static final MemoryGaugeCollector instance = new MemoryGaugeCollector(); - public static final long INVALID_MEMORY_COLLECTION_FREQUENCY = -1; // This value indicates that we do not know the frequency at which to collect Memory Metrics. If // this value is set for the memoryMetricCollectionRateMs, we do not collect Memory Metrics. @@ -56,7 +52,7 @@ public class MemoryGaugeCollector { @Nullable private ScheduledFuture memoryMetricCollectorJob = null; private long memoryMetricCollectionRateMs = UNSET_MEMORY_METRIC_COLLECTION_RATE; - private MemoryGaugeCollector() { + MemoryGaugeCollector() { this(Executors.newSingleThreadScheduledExecutor(), Runtime.getRuntime()); } @@ -67,11 +63,6 @@ private MemoryGaugeCollector() { this.runtime = runtime; } - /** Returns the singleton instance of this class. */ - public static MemoryGaugeCollector getInstance() { - return instance; - } - /** * Starts collecting MemoryMetricReadings at the specified rate. If it is already collecting the * readings, it updates the rate at which they're being collected if a different diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java index e3e228def34..b8f39a91ddb 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java @@ -78,8 +78,8 @@ public void setUp() { spy( new GaugeMetadataManager( Runtime.getRuntime(), ApplicationProvider.getApplicationContext())); - fakeCpuGaugeCollector = spy(CpuGaugeCollector.getInstance()); - fakeMemoryGaugeCollector = spy(MemoryGaugeCollector.getInstance()); + fakeCpuGaugeCollector = spy(new CpuGaugeCollector()); + fakeMemoryGaugeCollector = spy(new MemoryGaugeCollector()); doNothing() .when(fakeCpuGaugeCollector)