Skip to content

Fireperf: [Startup improvement] lazy initialize expensive objects in GaugeManager #3011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CpuMetricReading> cpuMetricReadings;
private final ScheduledExecutorService cpuMetricCollectorExecutor;
Expand All @@ -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();

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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;
Expand Down Expand Up @@ -50,10 +51,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 final ScheduledExecutorService gaugeManagerExecutor;
private final Lazy<ScheduledExecutorService> gaugeManagerExecutor;
private final ConfigResolver configResolver;
private final CpuGaugeCollector cpuGaugeCollector;
private final MemoryGaugeCollector memoryGaugeCollector;
private final Lazy<CpuGaugeCollector> cpuGaugeCollector;
private final Lazy<MemoryGaugeCollector> memoryGaugeCollector;
private final TransportManager transportManager;

@Nullable private GaugeMetadataManager gaugeMetadataManager;
Expand All @@ -64,22 +65,22 @@ public class GaugeManager {

private GaugeManager() {
this(
Executors.newSingleThreadScheduledExecutor(),
new Lazy<>(() -> Executors.newSingleThreadScheduledExecutor()),
TransportManager.getInstance(),
ConfigResolver.getInstance(),
null,
CpuGaugeCollector.getInstance(),
MemoryGaugeCollector.getInstance());
new Lazy<>(() -> new CpuGaugeCollector()),
new Lazy<>(() -> new MemoryGaugeCollector()));
}

@VisibleForTesting
GaugeManager(
ScheduledExecutorService gaugeManagerExecutor,
Lazy<ScheduledExecutorService> gaugeManagerExecutor,
TransportManager transportManager,
ConfigResolver configResolver,
GaugeMetadataManager gaugeMetadataManager,
CpuGaugeCollector cpuGaugeCollector,
MemoryGaugeCollector memoryGaugeCollector) {
Lazy<CpuGaugeCollector> cpuGaugeCollector,
Lazy<MemoryGaugeCollector> memoryGaugeCollector) {

this.gaugeManagerExecutor = gaugeManagerExecutor;
this.transportManager = transportManager;
Expand Down Expand Up @@ -134,13 +135,16 @@ public void startCollectingGauges(

try {
gaugeManagerDataCollectionJob =
gaugeManagerExecutor.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());
Expand Down Expand Up @@ -190,8 +194,8 @@ 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);
Expand All @@ -200,12 +204,14 @@ public void stopCollectingGauges() {
// Flush any data that was collected for this session one last time.
@SuppressWarnings("FutureReturnValueIgnored")
ScheduledFuture unusedFuture =
gaugeManagerExecutor.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;
Expand All @@ -222,13 +228,14 @@ 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.
Expand Down Expand Up @@ -284,7 +291,7 @@ private boolean startCollectingCpuMetrics(
return false;
}

cpuGaugeCollector.startCollecting(cpuMetricCollectionFrequency, referenceTime);
cpuGaugeCollector.get().startCollecting(cpuMetricCollectionFrequency, referenceTime);
return true;
}

Expand All @@ -305,7 +312,7 @@ private boolean startCollectingMemoryMetrics(
return false;
}

memoryGaugeCollector.startCollecting(memoryMetricCollectionFrequency, referenceTime);
memoryGaugeCollector.get().startCollecting(memoryMetricCollectionFrequency, referenceTime);
return true;
}

Expand All @@ -320,7 +327,7 @@ private boolean startCollectingMemoryMetrics(
* enabled.
*/
public void collectGaugeMetricOnce(Timer referenceTime) {
collectGaugeMetricOnce(cpuGaugeCollector, memoryGaugeCollector, referenceTime);
collectGaugeMetricOnce(cpuGaugeCollector.get(), memoryGaugeCollector.get(), referenceTime);
}

private static void collectGaugeMetricOnce(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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());
}

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -77,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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down