Address comments in the PR (96688059) · Commits · The Tor Project / Applications / android-components

components/service/glean/src/main/java/mozilla/components/service/glean/histogram/FunctionalHistogram.kt

+14 −1

Original line number	Diff line number	Diff line
		@@ -23,7 +23,7 @@ import kotlin.math.log
		* In other words, there are n buckets for each power of 2 magnitude.
		*
		* The value of 8 for n was determined experimentally based on existing data to have sufficient
		* resolution.
		* resolution. See https://bugzilla.mozilla.org/show_bug.cgi?id=1565253
		*
		* @param values a map containing the minimum bucket value mapped to the accumulated count
		* @param sum the accumulated sum of all the samples in the histogram
		@@ -49,6 +49,9 @@ data class FunctionalHistogram(
		* A "bucket index" is the consecutive integer index of each bucket, useful as a
		* mathematical concept, even though the internal representation is stored and
		* sent using the minimum value in each bucket.
		*
		* @param sample The data sample
		* @return The bucket index the sample belongs in
		*/
		internal fun sampleToBucketIndex(sample: Long): Long {
		return log(sample.toDouble() + 1, EXPONENT).toLong()
		@@ -56,6 +59,9 @@ data class FunctionalHistogram(

		/**
		* Determines the minimum value of a bucket, given a bucket index.
		*
		* @param bucketIndex The ordinal index of a bucket
		* @return The minimum value of the bucket
		*/
		internal fun bucketIndexToBucketMinimum(bucketIndex: Long): Long {
		return pow(EXPONENT, bucketIndex.toDouble()).toLong()
		@@ -63,6 +69,9 @@ data class FunctionalHistogram(

		/**
		* Maps a sample to the minimum value of the bucket it belongs in.
		*
		* @param sample The sample value
		@ @return the minimum value of the bucket the sample belongs in
		*/
		internal fun sampleToBucketMinimum(sample: Long): Long {
		return if (sample == 0L) {
		@@ -131,6 +140,8 @@ data class FunctionalHistogram(
		/**
		* Helper function to build the [FunctionalHistogram] into a JSONObject for serialization
		* purposes.
		*
		* @return The histogram as JSON for persistence
		*/
		internal fun toJsonObject(): JSONObject {
		return JSONObject(mapOf(
		@@ -142,6 +153,8 @@ data class FunctionalHistogram(
		/**
		* Helper function to build the [FunctionalHistogram] into a JSONObject for sending in the
		* ping payload.
		*
		* @return The histogram as JSON for a ping payload
		*/
		internal fun toJsonPayloadObject(): JSONObject {
		val completeValues = if (values.size != 0) {

components/service/glean/src/main/java/mozilla/components/service/glean/histogram/PrecomputedHistogram.kt

+5 −1

Original line number	Diff line number	Diff line
		@@ -17,9 +17,9 @@ import org.json.JSONObject
		* to help serialize and deserialize data to the correct format for transport and
		* storage, as well as including helper functions to calculate the bucket sizes.
		*
		* @param bucketCount total number of buckets
		* @param rangeMin the minimum value that can be represented
		* @param rangeMax the maximum value that can be represented
		* @param bucketCount total number of buckets
		* @param histogramType the [HistogramType] representing the bucket layout
		* @param values a map containing the bucket index mapped to the accumulated count
		* @param sum the accumulated sum of all the samples in the custom distribution
		@@ -151,6 +151,8 @@ data class PrecomputedHistogram(
		/**
		* Helper function to build the [PrecomputedHistogram] into a JSONObject for serialization
		* purposes.
		*
		* @return The histogram as JSON for persistence
		*/
		internal fun toJsonObject(): JSONObject {
		return JSONObject(mapOf(
		@@ -168,6 +170,8 @@ data class PrecomputedHistogram(
		*
		* - this does not include the bucketing parameters
		* - all buckets [0, max) are inserted into values
		*
		* @return The histogram as JSON to send in a ping payload
		*/
		internal fun toJsonPayloadObject(): JSONObject {
		// Include all buckets [0, max), where max is the maximum bucket with

components/service/glean/src/main/java/mozilla/components/service/glean/storages/MemoryDistributionsStorageEngine.kt

+9 −8

Original line number	Diff line number	Diff line
		@@ -56,7 +56,8 @@ internal open class MemoryDistributionsStorageEngineImplementation(
		* Samples greater than 1TB are truncated to 1TB.
		*
		* @param metricData the metric information for the memory distribution
		* @param sample the value to accumulate, in bytes
		* @param sample the value to accumulate
		* @param memoryUnit the unit of the sample
		*/
		@Synchronized
		fun accumulate(
		@@ -73,8 +74,8 @@ internal open class MemoryDistributionsStorageEngineImplementation(
		* Samples greater than 1TB are truncated to 1TB.
		*
		* @param metricData the metric information for the memory distribution
		* @param samples the values to accumulate, in the given `timeUnit`
		* @param timeUnit the unit that the given samples are in, defaults to nanoseconds
		* @param samples the values to accumulate, in the given `memoryUnit`
		* @param memoryUnit the unit that the given samples are in
		*/
		@Suppress("ComplexMethod")
		@Synchronized
		@@ -83,7 +84,7 @@ internal open class MemoryDistributionsStorageEngineImplementation(
		samples: LongArray,
		memoryUnit: MemoryUnit
		) {
		// Remove invalid samples, and convert to nanos
		// Remove invalid samples, and convert to bytes
		var numTooLongSamples = 0
		var numNegativeSamples = 0
		var factor = memoryToBytes(memoryUnit, 1)
		@@ -122,7 +123,7 @@ internal open class MemoryDistributionsStorageEngineImplementation(
		logger,
		numTooLongSamples
		)
		// Too long samples should just be truncated, but otherwise we record and handle them
		// Too large samples should just be truncated, but otherwise we record and handle them
		}

		val dummy = FunctionalHistogram()
		@@ -132,9 +133,9 @@ internal open class MemoryDistributionsStorageEngineImplementation(
		it.accumulate(sample)
		it
		} ?: let {
		val newTD = FunctionalHistogram()
		newTD.accumulate(sample)
		return@let newTD
		val newMD = FunctionalHistogram()
		newMD.accumulate(sample)
		return@let newMD
		}
		}
		}

components/service/glean/src/main/java/mozilla/components/service/glean/utils/MemoryUtils.kt

+1 −1

Original line number	Diff line number	Diff line
		@@ -9,7 +9,7 @@ import mozilla.components.service.glean.private.MemoryUnit
		/**
		* Convenience method to convert a memory size in a different unit to bytes.
		*
		* @param memoryUnit the unit the value is in
		* @param memoryUnit the [MemoryUnit] the value is in
		* @param value a memory size in the given unit
		*
		* @return the memory size, in bytes

components/service/glean/src/test/java/mozilla/components/service/glean/histogram/PrecomputedHistogramTest.kt

+14 −14

Original line number	Diff line number	Diff line
		@@ -96,23 +96,23 @@ class PrecomputedHistogramTest {
		val jsonTdd = tdd.toJsonObject()

		// Verify properties
		assertEquals("JSON bucket count must match Custom Distribution bucket count",
		assertEquals("JSON bucket count must match Precomputed Histogram bucket count",
		tdd.bucketCount, jsonTdd.getInt("bucket_count"))
		val jsonRange = jsonTdd.getJSONArray("range")
		assertEquals("JSON range minimum must match Custom Distribution range minimum",
		assertEquals("JSON range minimum must match Precomputed Histogram range minimum",
		tdd.rangeMin, jsonRange.getLong(0))
		assertEquals("JSON range maximum must match Custom Distribution range maximum",
		assertEquals("JSON range maximum must match Precomputed Histogram range maximum",
		tdd.rangeMax, jsonRange.getLong(1))
		assertEquals("JSON histogram type must match Custom Distribution histogram type",
		assertEquals("JSON histogram type must match Precomputed Histogram histogram type",
		tdd.histogramType.toString().toLowerCase(), jsonTdd.getString("histogram_type"))
		val jsonValue = jsonTdd.getJSONObject("values")
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		tdd.values[1], jsonValue.getLong("1"))
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		tdd.values[2], jsonValue.getLong("2"))
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		tdd.values[3], jsonValue.getLong("3"))
		assertEquals("JSON sum must match Custom Distribution sum",
		assertEquals("JSON sum must match Precomputed Histogram sum",
		tdd.sum, jsonTdd.getLong("sum"))

		// Convert to JSON object using toJsonObject()
		@@ -121,17 +121,17 @@ class PrecomputedHistogramTest {
		// Verify properties
		assertEquals(2, jsonPayload.length())
		val jsonPayloadValue = jsonPayload.getJSONObject("values")
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		0, jsonPayloadValue.getLong("0"))
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		tdd.values[1], jsonPayloadValue.getLong("1"))
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		tdd.values[2], jsonPayloadValue.getLong("2"))
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		tdd.values[3], jsonPayloadValue.getLong("3"))
		assertEquals("JSON values must match Custom Distribution values",
		assertEquals("JSON values must match Precomputed Histogram values",
		0, jsonPayloadValue.getLong("4"))
		assertEquals("JSON sum must match Custom Distribution sum",
		assertEquals("JSON sum must match Precomputed Histogram sum",
		tdd.sum, jsonPayload.getLong("sum"))
		}
		}