Mutate labels to avoid excessive object cloning.

siimon · Sep 21, 2018 · 709a407 · 709a407
1 parent 24cf59a
commit 709a407
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,9 @@ project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Added
 
+- Fixed performance by avoiding `Object.assign` on hot paths, as well as
+  mutating objects when appropriate.
+
 ## [11.1.2] - 2018-09-19
 
 ### Changed

diff --git a/lib/histogram.js b/lib/histogram.js
@@ -282,9 +282,17 @@ function convertLabelsAndValues(labels, value) {
 
 function extractBucketValuesForExport(histogram) {
 	return bucketData => {
-		const buckets = histogram.upperBounds.map(
-			createBucketValues(bucketData, histogram)
-		);
+		const buckets = [];
+		const bucketLabelNames = Object.keys(bucketData.labels);
+		let acc = 0;
+		for (const upperBound of histogram.upperBounds) {
+			acc += bucketData.bucketValues[upperBound];
+			const lbls = { le: upperBound };
+			for (const labelName of bucketLabelNames) {
+				lbls[labelName] = bucketData.labels[labelName];
+			}
+			buckets.push(setValuePair(lbls, acc, `${histogram.name}_bucket`));
+		}
 		return { buckets, data: bucketData };
 	};
 }
@@ -293,24 +301,17 @@ function addSumAndCountForExport(histogram) {
 	return (acc, d) => {
 		acc.push(...d.buckets);
 
-		const infLabel = Object.assign({ le: '+Inf' }, d.data.labels);
-		acc.push(setValuePair(infLabel, d.data.count, `${histogram.name}_bucket`));
-		acc.push(setValuePair(d.data.labels, d.data.sum, `${histogram.name}_sum`));
+		const infLabel = { le: '+Inf' };
+		for (const label of Object.keys(d.data.labels)) {
+			infLabel[label] = d.data.labels[label];
+		}
 		acc.push(
+			setValuePair(infLabel, d.data.count, `${histogram.name}_bucket`),
+			setValuePair(d.data.labels, d.data.sum, `${histogram.name}_sum`),
 			setValuePair(d.data.labels, d.data.count, `${histogram.name}_count`)
 		);
 		return acc;
 	};
 }
 
-function createBucketValues(bucket, histogram) {
-	let acc = 0;
-	return upperBound => {
-		acc += bucket.bucketValues[upperBound];
-		const lbls = Object.assign({ le: upperBound }, bucket.labels);
-		const valuePair = setValuePair(lbls, acc, `${histogram.name}_bucket`);
-		return valuePair;
-	};
-}
-
 module.exports = Histogram;
diff --git a/lib/registry.js b/lib/registry.js
@@ -29,39 +29,46 @@ class Registry {
 		const opts = Object.assign({}, defaultMetricsOpts, conf);
 		const item = metric.get();
 		const name = escapeString(item.name);
-		let help = escapeString(item.help);
-		help = ['#', 'HELP', name, help].join(' ');
-		const type = ['#', 'TYPE', name, item.type].join(' ');
-
-		const values = (item.values || []).reduce((valAcc, val) => {
-			const merged = Object.assign({}, this._defaultLabels, val.labels);
+		const help = `# HELP ${name} ${escapeString(item.help)}`;
+		const type = `# TYPE ${name} ${item.type}`;
+		const defaultLabelNames = Object.keys(this._defaultLabels);
+
+		let values = '';
+		for (const val of item.values || []) {
+			val.labels = val.labels || {};
+			for (const labelName of defaultLabelNames) {
+				val.labels[labelName] =
+					val.labels[labelName] || this._defaultLabels[labelName];
+			}
 
-			const labels = Object.keys(merged).map(
-				key => `${key}="${escapeLabelValue(merged[key])}"`
-			);
+			let labels = '';
+			for (const key of Object.keys(val.labels)) {
+				labels += `${key}="${escapeLabelValue(val.labels[key])}",`;
+			}
 
 			let metricName = val.metricName || item.name;
-			if (labels.length) {
-				metricName += `{${labels.join(',')}}`;
+			if (labels) {
+				metricName += `{${labels.replace(/,$/, '')}}`;
 			}
 
-			const line = [metricName, getValueAsString(val.value)];
-			if (opts.timestamps) {
-				line.push(val.timestamp);
+			let line = `${metricName} ${getValueAsString(val.value)}`;
+			if (opts.timestamps && val.timestamp) {
+				line += ` ${val.timestamp}`;
 			}
-			valAcc += line.join(' ').trim();
-			valAcc += '\n';
-			return valAcc;
-		}, '');
+			values += `${line.trim()}\n`;
+		}
 
-		const acc = [help, type, values].join('\n');
-		return acc;
+		return `${help}\n${type}\n${values}`.trim();
 	}
 
 	metrics(opts) {
-		return this.getMetricsAsArray()
-			.map(metric => this.getMetricAsPrometheusString(metric, opts))
-			.join('\n');
+		let metrics = '';
+
+		for (const metric of this.getMetricsAsArray()) {
+			metrics += `${this.getMetricAsPrometheusString(metric, opts)}\n\n`;
+		}
+
+		return metrics.replace(/\n$/, '');
 	}
 
 	registerMetric(metricFn) {
@@ -83,21 +90,25 @@ class Registry {
 	}
 
 	getMetricsAsJSON() {
-		return this.getMetricsAsArray().map(metric => {
+		const metrics = [];
+		const defaultLabelNames = Object.keys(this._defaultLabels);
+
+		for (const metric of this.getMetricsAsArray()) {
 			const item = metric.get();
-			if (!item.values) {
-				return item;
+
+			if (item.values) {
+				for (const val of item.values) {
+					for (const labelName of defaultLabelNames) {
+						val.labels[labelName] =
+							val.labels[labelName] || this._defaultLabels[labelName];
+					}
+				}
 			}
 
-			item.values = item.values.map(val =>
-				// Avoid mutation and merge metric labels with registry default labels
-				Object.assign({}, val, {
-					labels: Object.assign({}, this._defaultLabels, val.labels)
-				})
-			);
+			metrics.push(item);
+		}
 
-			return item;
-		});
+		return metrics;
 	}
 
 	removeSingleMetric(name) {