From e33ca2d0ae896e0a9f22073c263666c8ad20b58c Mon Sep 17 00:00:00 2001
From: Matt Jaffee <matthew.jaffee@gmail.com>
Date: Fri, 8 Mar 2019 14:35:52 -0600
Subject: [PATCH] use UnionInPlace in import-roaring

get the count of the existing fragment and compare it to the incoming bits to
decide which should be unioned into the other. This should generally result in
far fewer allocations, though there is much work that needs to be done within
UnionInPlace to further improve things.

unrelatedly, I added a TODO to change the long-query-time option to move it out
of cluster. It should probably be happening at the API level so that different
handlers can reuse it, but if we're going to do that we'll want to make sure
that any potentially time intensive operations are pulled into api from
handler (e.g. protobuf decoding)
---
 cluster.go       |  1 +
 fragment.go      | 13 ++++++++++---
 server/config.go |  9 +++++----
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/cluster.go b/cluster.go
index 4565e91f3..5c5c0e7f1 100644
--- a/cluster.go
+++ b/cluster.go
@@ -184,6 +184,7 @@ type cluster struct { // nolint: maligned
 	ReplicaN int
 
 	// Threshold for logging long-running queries
+	// TODO(2.0) move this out of cluster. (why is it here??)
 	longQueryTime time.Duration
 
 	// Maximum number of Set() or Clear() commands per request.
diff --git a/fragment.go b/fragment.go
index 067814a0b..c09461470 100644
--- a/fragment.go
+++ b/fragment.go
@@ -1732,8 +1732,10 @@ func (f *fragment) importRoaring(data []byte, clear bool) error {
 	rowSet := make([]uint64, 0)
 	var lastRow uint64 = math.MaxUint64
 
+	incomingCnt := 0
 	for iter.Next() {
-		key, _ := iter.Value()
+		key, c := iter.Value()
+		incomingCnt += int(c.N())
 
 		// virtual row for the current container
 		vRow := key >> shardVsContainerExponent
@@ -1749,8 +1751,13 @@ func (f *fragment) importRoaring(data []byte, clear bool) error {
 	if clear {
 		bm = f.storage.Difference(bm)
 	} else {
-		if f.storage.Count() > 0 {
-			bm = f.storage.Union(bm)
+		if cnt := f.storage.Count(); cnt > 0 {
+			if incomingCnt > int(cnt) {
+				bm.UnionInPlace(f.storage)
+			} else {
+				f.storage.UnionInPlace(bm)
+				bm = f.storage
+			}
 		}
 	}
 
diff --git a/server/config.go b/server/config.go
index 6223f07da..9c787f92e 100644
--- a/server/config.go
+++ b/server/config.go
@@ -75,10 +75,11 @@ type Config struct {
 
 	Cluster struct {
 		// Disabled controls whether clustering functionality is enabled.
-		Disabled      bool          `toml:"disabled"`
-		Coordinator   bool          `toml:"coordinator"`
-		ReplicaN      int           `toml:"replicas"`
-		Hosts         []string      `toml:"hosts"`
+		Disabled    bool     `toml:"disabled"`
+		Coordinator bool     `toml:"coordinator"`
+		ReplicaN    int      `toml:"replicas"`
+		Hosts       []string `toml:"hosts"`
+		// TODO(2.0) move this out of cluster. (why is it here??)
 		LongQueryTime toml.Duration `toml:"long-query-time"`
 	} `toml:"cluster"`