From c83bfafcf8a7c8d5c735e9ecc0fc031617895801 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 09:40:14 -0400
Subject: [PATCH 1/7] Add benchmark for FetchResultToPromResult

---
 src/query/storage/converter_test.go | 43 +++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/src/query/storage/converter_test.go b/src/query/storage/converter_test.go
index 6f06f18bf7..79e8563cd5 100644
--- a/src/query/storage/converter_test.go
+++ b/src/query/storage/converter_test.go
@@ -31,6 +31,7 @@ import (
 	"github.com/m3db/m3/src/query/generated/proto/prompb"
 	"github.com/m3db/m3/src/query/models"
 	"github.com/m3db/m3/src/query/test/seriesiter"
+	"github.com/m3db/m3/src/query/ts"
 	"github.com/m3db/m3x/ident"
 	"github.com/m3db/m3x/pool"
 	xsync "github.com/m3db/m3x/sync"
@@ -229,3 +230,45 @@ func TestPromReadQueryToM3(t *testing.T) {
 		})
 	}
 }
+
+var (
+	benchResult *prompb.QueryResult
+)
+
+func BenchmarkFetchResultToPromResult(b *testing.B) {
+	var (
+		numSeries              = 1000
+		numDatapointsPerSeries = 1000
+		numTagsPerSeries       = 10
+		fr                     = &FetchResult{
+			SeriesList: make(ts.SeriesList, 0, numSeries),
+		}
+	)
+
+	for i := 0; i < numSeries; i++ {
+		values := make(ts.Datapoints, 0, numDatapointsPerSeries)
+		for i := 0; i < numDatapointsPerSeries; i++ {
+			values = append(values, ts.Datapoint{
+				Timestamp: time.Time{},
+				Value:     float64(i),
+			})
+		}
+
+		tags := make(models.Tags, 0, numTagsPerSeries)
+		for i := 0; i < numTagsPerSeries; i++ {
+			tags = append(tags, models.Tag{
+				Name:  fmt.Sprintf("name-%d", i),
+				Value: fmt.Sprintf("value-%d", i),
+			})
+		}
+
+		series := ts.NewSeries(
+			fmt.Sprintf("series-%d", i), values, tags)
+
+		fr.SeriesList = append(fr.SeriesList, series)
+	}
+
+	for i := 0; i < b.N; i++ {
+		benchResult = FetchResultToPromResult(fr)
+	}
+}

From 0f2b9c8adc3c89492669fe3d3247d2ecf91b7b6a Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 09:56:23 -0400
Subject: [PATCH 2/7] Significantly improve performance of
 FetchResultToPromResult

---
 src/query/storage/converter.go | 59 +++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/src/query/storage/converter.go b/src/query/storage/converter.go
index befb0231d4..e2416ed221 100644
--- a/src/query/storage/converter.go
+++ b/src/query/storage/converter.go
@@ -135,51 +135,80 @@ func TimestampToTime(timestampMS int64) time.Time {
 
 // TimeToTimestamp converts a time.Time to prometheus timestamp
 func TimeToTimestamp(timestamp time.Time) int64 {
+	// Significantly faster than time.Truncate()
 	return timestamp.UnixNano() / int64(time.Millisecond)
 }
 
 // FetchResultToPromResult converts fetch results from M3 to Prometheus result
 func FetchResultToPromResult(result *FetchResult) *prompb.QueryResult {
-	timeseries := make([]*prompb.TimeSeries, 0)
-
+	// Perform bulk allocation upfront then convert to pointers afterwards
+	// to reduce total number of allocations. See BenchmarkFetchResultToPromResult
+	// if modifying.
+	timeseries := make([]prompb.TimeSeries, 0, len(result.SeriesList))
 	for _, series := range result.SeriesList {
 		promTs := SeriesToPromTS(series)
 		timeseries = append(timeseries, promTs)
 	}
 
+	timeSeriesPointers := make([]*prompb.TimeSeries, 0, len(result.SeriesList))
+	for i := range timeseries {
+		timeSeriesPointers = append(timeSeriesPointers, &timeseries[i])
+	}
+
 	return &prompb.QueryResult{
-		Timeseries: timeseries,
+		Timeseries: timeSeriesPointers,
 	}
 }
 
 // SeriesToPromTS converts a series to prometheus timeseries
-func SeriesToPromTS(series *ts.Series) *prompb.TimeSeries {
+func SeriesToPromTS(series *ts.Series) prompb.TimeSeries {
 	labels := TagsToPromLabels(series.Tags)
 	samples := SeriesToPromSamples(series)
-	return &prompb.TimeSeries{Labels: labels, Samples: samples}
+	return prompb.TimeSeries{Labels: labels, Samples: samples}
 }
 
 // TagsToPromLabels converts tags to prometheus labels
 func TagsToPromLabels(tags models.Tags) []*prompb.Label {
-	labels := make([]*prompb.Label, 0, len(tags))
+	// Perform bulk allocation upfront then convert to pointers afterwards
+	// to reduce total number of allocations. See BenchmarkFetchResultToPromResult
+	// if modifying.
+	labels := make([]prompb.Label, 0, len(tags))
 	for _, t := range tags {
-		labels = append(labels, &prompb.Label{Name: t.Name, Value: t.Value})
+		labels = append(labels, prompb.Label{Name: t.Name, Value: t.Value})
+	}
+
+	labelsPointers := make([]*prompb.Label, 0, len(tags))
+	for i := range labels {
+		labelsPointers = append(labelsPointers, &labels[i])
 	}
 
-	return labels
+	return labelsPointers
 }
 
 // SeriesToPromSamples series datapoints to prometheus samples
 func SeriesToPromSamples(series *ts.Series) []*prompb.Sample {
-	samples := make([]*prompb.Sample, series.Len())
-	for i := 0; i < series.Len(); i++ {
-		samples[i] = &prompb.Sample{
-			Timestamp: series.Values().DatapointAt(i).Timestamp.UnixNano() / int64(time.Millisecond),
-			Value:     series.Values().ValueAt(i),
-		}
+	var (
+		seriesLen = series.Len()
+		values    = series.Values()
+		// Perform bulk allocation upfront then convert to pointers afterwards
+		// to reduce total number of allocations. See BenchmarkFetchResultToPromResult
+		// if modifying.
+		samples = make([]prompb.Sample, 0, seriesLen)
+	)
+	for i := 0; i < seriesLen; i++ {
+		dp := values.DatapointAt(i)
+		samples = append(samples, prompb.Sample{
+			Timestamp: TimeToTimestamp(dp.Timestamp),
+			Value:     dp.Value,
+		})
+	}
+
+	samplesPointers := make([]*prompb.Sample, 0, len(samples))
+	for i := range samples {
+		samplesPointers = append(samplesPointers, &samples[i])
 	}
 
-	return samples
+	return samplesPointers
 }
 
 func iteratorToTsSeries(

From b33a8aaab6c12831fe9fb58a936cb244ce0949c1 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 10:42:27 -0400
Subject: [PATCH 3/7] More optimizations (Add Datapoints method)

---
 src/query/storage/converter.go |  8 ++++----
 src/query/ts/values.go         | 13 +++++++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/query/storage/converter.go b/src/query/storage/converter.go
index e2416ed221..d80ad9556a 100644
--- a/src/query/storage/converter.go
+++ b/src/query/storage/converter.go
@@ -188,15 +188,15 @@ func TagsToPromLabels(tags models.Tags) []*prompb.Label {
 // SeriesToPromSamples series datapoints to prometheus samples
 func SeriesToPromSamples(series *ts.Series) []*prompb.Sample {
 	var (
-		seriesLen = series.Len()
-		values    = series.Values()
+		seriesLen  = series.Len()
+		values     = series.Values()
+		datapoints = values.Datapoints()
 		// Perform bulk allocation upfront then convert to pointers afterwards
 		// to reduce total number of allocations. See BenchmarkFetchResultToPromResult
 		// if modifying.
 		samples = make([]prompb.Sample, 0, seriesLen)
 	)
-	for i := 0; i < seriesLen; i++ {
-		dp := values.DatapointAt(i)
+	for _, dp := range datapoints {
 		samples = append(samples, prompb.Sample{
 			Timestamp: TimeToTimestamp(dp.Timestamp),
 			Value:     dp.Value,
diff --git a/src/query/ts/values.go b/src/query/ts/values.go
index 27a5995551..6e12f4671b 100644
--- a/src/query/ts/values.go
+++ b/src/query/ts/values.go
@@ -46,6 +46,9 @@ type Values interface {
 	// DatapointAt returns the datapoint at the nth element
 	DatapointAt(n int) Datapoint
 
+	// Datapoints returns all the datapoints
+	Datapoints() []Datapoint
+
 	// AlignToBounds returns values aligned to the start time and duration
 	AlignToBounds(bounds models.Bounds) []Datapoints
 }
@@ -68,6 +71,9 @@ func (d Datapoints) ValueAt(n int) float64 { return d[n].Value }
 // DatapointAt returns the value at the nth element.
 func (d Datapoints) DatapointAt(n int) Datapoint { return d[n] }
 
+// Datapoints returns all the datapoints.
+func (d Datapoints) Datapoints() []Datapoint { return d }
+
 // Values returns the values representation.
 func (d Datapoints) Values() []float64 {
 	values := make([]float64, len(d))
@@ -137,6 +143,13 @@ func (b *fixedResolutionValues) DatapointAt(point int) Datapoint {
 		Value:     b.ValueAt(point),
 	}
 }
+func (b *fixedResolutionValues) Datapoints() []Datapoint {
+	datapoints := make([]Datapoint, 0, len(b.values))
+	for i := range b.values {
+		datapoints = append(datapoints, b.DatapointAt(i))
+	}
+	return datapoints
+}
 
 // AlignToBounds returns values aligned to given bounds.
 // TODO: Consider bounds as well

From d2528cf1eaac2efb5850032be38c77d650d32bd7 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 10:46:34 -0400
Subject: [PATCH 4/7] Add comment

---
 src/query/storage/converter.go | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/query/storage/converter.go b/src/query/storage/converter.go
index d80ad9556a..6de1dfd42e 100644
--- a/src/query/storage/converter.go
+++ b/src/query/storage/converter.go
@@ -139,7 +139,10 @@ func TimeToTimestamp(timestamp time.Time) int64 {
 	return timestamp.UnixNano() / int64(time.Millisecond)
 }
 
-// FetchResultToPromResult converts fetch results from M3 to Prometheus result
+// FetchResultToPromResult converts fetch results from M3 to Prometheus result.
+// TODO(rartoul): Ideally we would pool all of these intermediary datastructures,
+// but right now its hooked into generated code so accomplishing that would be
+// non-trivial.
 func FetchResultToPromResult(result *FetchResult) *prompb.QueryResult {
 	// Perform bulk allocation upfront then convert to pointers afterwards
 	// to reduce total number of allocations. See BenchmarkFetchResultToPromResult
@@ -160,14 +163,14 @@ func FetchResultToPromResult(result *FetchResult) *prompb.QueryResult {
 	}
 }
 
-// SeriesToPromTS converts a series to prometheus timeseries
+// SeriesToPromTS converts a series to prometheus timeseries.
 func SeriesToPromTS(series *ts.Series) prompb.TimeSeries {
 	labels := TagsToPromLabels(series.Tags)
 	samples := SeriesToPromSamples(series)
 	return prompb.TimeSeries{Labels: labels, Samples: samples}
 }
 
-// TagsToPromLabels converts tags to prometheus labels
+// TagsToPromLabels converts tags to prometheus labels.TagsToPromLabels.
 func TagsToPromLabels(tags models.Tags) []*prompb.Label {
 	// Perform bulk allocation upfront then convert to pointers afterwards
 	// to reduce total number of allocations. See BenchmarkFetchResultToPromResult
@@ -185,7 +188,7 @@ func TagsToPromLabels(tags models.Tags) []*prompb.Label {
 	return labelsPointers
 }
 
-// SeriesToPromSamples series datapoints to prometheus samples
+// SeriesToPromSamples series datapoints to prometheus samples.SeriesToPromSamples.
 func SeriesToPromSamples(series *ts.Series) []*prompb.Sample {
 	var (
 		seriesLen  = series.Len()

From a3ec56b880e47affba1116b2dd0a3a8c29c89b0e Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 10:46:55 -0400
Subject: [PATCH 5/7] Add benchmark result as comment

---
 src/query/storage/converter_test.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/query/storage/converter_test.go b/src/query/storage/converter_test.go
index 79e8563cd5..8c6fe5ea2d 100644
--- a/src/query/storage/converter_test.go
+++ b/src/query/storage/converter_test.go
@@ -235,6 +235,7 @@ var (
 	benchResult *prompb.QueryResult
 )
 
+// BenchmarkFetchResultToPromResult-8   	     100	  10563444 ns/op	25368543 B/op	    4443 allocs/op
 func BenchmarkFetchResultToPromResult(b *testing.B) {
 	var (
 		numSeries              = 1000

From 30436d38ddf9c59e2ab97bb0041cebac0914b7b1 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 10:50:05 -0400
Subject: [PATCH 6/7] Add comment about pooling

---
 src/query/storage/converter.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/query/storage/converter.go b/src/query/storage/converter.go
index 6de1dfd42e..b3ff0c8500 100644
--- a/src/query/storage/converter.go
+++ b/src/query/storage/converter.go
@@ -140,9 +140,9 @@ func TimeToTimestamp(timestamp time.Time) int64 {
 }
 
 // FetchResultToPromResult converts fetch results from M3 to Prometheus result.
-// TODO(rartoul): Ideally we would pool all of these intermediary datastructures,
-// but right now its hooked into generated code so accomplishing that would be
-// non-trivial.
+// TODO(rartoul): We should pool all of these intermediary datastructures, or
+// at least the []*prompb.Sample (as thats the most heavily allocated object)
+// since we have full control over the lifecycle.
 func FetchResultToPromResult(result *FetchResult) *prompb.QueryResult {
 	// Perform bulk allocation upfront then convert to pointers afterwards
 	// to reduce total number of allocations. See BenchmarkFetchResultToPromResult

From e278bde98cc2a4efbf6491579dd3a9f7f7c34c0c Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Tue, 2 Oct 2018 10:56:44 -0400
Subject: [PATCH 7/7] Fix

---
 src/query/storage/converter.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/query/storage/converter.go b/src/query/storage/converter.go
index b3ff0c8500..b1f3556677 100644
--- a/src/query/storage/converter.go
+++ b/src/query/storage/converter.go
@@ -170,7 +170,7 @@ func SeriesToPromTS(series *ts.Series) prompb.TimeSeries {
 	return prompb.TimeSeries{Labels: labels, Samples: samples}
 }
 
-// TagsToPromLabels converts tags to prometheus labels.TagsToPromLabels.
+// TagsToPromLabels converts tags to prometheus labels.
 func TagsToPromLabels(tags models.Tags) []*prompb.Label {
 	// Perform bulk allocation upfront then convert to pointers afterwards
 	// to reduce total number of allocations. See BenchmarkFetchResultToPromResult