From 0046f80d3375e86a2da9326562424ef490259e42 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Thu, 24 Feb 2022 19:11:56 +0000 Subject: [PATCH] Bug 1756251 - make GeckoJavaSampler.Sample effectively immutable. r=julienw As used, the class is effectively immutable which provides some thread safety guarantees so I updated the declarations so the class could take advantage of that. Effectively immutable classes need to be safely published in order to be thread safe. I believe we could avoid the need for safe publishing (i.e. strengthen thread safety guarantees) if we replaced the Frame[] type with a thread safe data type (AtomicReferenceArray, a concurrent list, etc.). Differential Revision: https://phabricator.services.mozilla.com/D139196 --- .../org/mozilla/gecko/GeckoJavaSampler.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoJavaSampler.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoJavaSampler.java index da8a25f42701..6fdb0cd2223a 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoJavaSampler.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoJavaSampler.java @@ -59,21 +59,24 @@ public class GeckoJavaSampler { return getProfilerTime(); } + /** + * A data container for a profiler sample. This class is effectively immutable (i.e. technically + * mutable but never mutated after construction) so is thread safe *if it is safely published* + * (see Java Concurrency in Practice, 2nd Ed., Section 3.5.3 for safe publication idioms). + */ private static class Sample { - public Frame[] mFrames; - public double mTime; - public long mJavaTime; // non-zero if Android system time is used + public final Frame[] mFrames; + public final double mTime; + public final long mJavaTime; // non-zero if Android system time is used public Sample(final StackTraceElement[] aStack) { mFrames = new Frame[aStack.length]; - if (GeckoThread.isStateAtLeast(GeckoThread.State.JNI_READY)) { - mTime = getProfilerTime(); - } - if (mTime == 0.0d) { - // getProfilerTime is not available yet; either libs are not loaded, - // or profiling hasn't started on the Gecko side yet - mJavaTime = SystemClock.elapsedRealtime(); - } + mTime = GeckoThread.isStateAtLeast(GeckoThread.State.JNI_READY) ? getProfilerTime() : 0; + + // if mTime == 0, getProfilerTime is not available yet; either libs are not loaded, + // or profiling hasn't started on the Gecko side yet + mJavaTime = mTime == 0.0d ? SystemClock.elapsedRealtime() : 0; + for (int i = 0; i < aStack.length; i++) { mFrames[aStack.length - 1 - i] = new Frame(aStack[i].getMethodName(), aStack[i].getClassName());