From 68d3bc35fc217d79ce4e9c2dee194ec87c0a6468 Mon Sep 17 00:00:00 2001
From: 0xAP <ap.droit@gmail.com>
Date: Wed, 20 Oct 2021 10:51:00 +0200
Subject: [PATCH] bundle: make merge.InterfacesMaps mutable to improve loading
 performance (#3882)

Fixes #3860.

Signed-off-by: Arvid Prozesky <arvid.prozesky@sap.com>
---
 internal/merge/merge.go      | 48 ++++++++++++++++++++++++---------
 internal/merge/merge_test.go | 52 ++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 12 deletions(-)

diff --git a/internal/merge/merge.go b/internal/merge/merge.go
index fa53236d01..16f39350be 100644
--- a/internal/merge/merge.go
+++ b/internal/merge/merge.go
@@ -8,33 +8,57 @@ package merge
 
 // InterfaceMaps returns the result of merging a and b. If a and b cannot be
 // merged because of conflicting key-value pairs, ok is false.
-func InterfaceMaps(a map[string]interface{}, b map[string]interface{}) (c map[string]interface{}, ok bool) {
+func InterfaceMaps(a map[string]interface{}, b map[string]interface{}) (map[string]interface{}, bool) {
 
-	c = map[string]interface{}{}
-	for k := range a {
-		c[k] = a[k]
+	if a == nil {
+		return b, true
 	}
 
+	if hasConflicts(a, b) {
+		return nil, false
+	}
+
+	return merge(a, b), true
+}
+
+func merge(a, b map[string]interface{}) map[string]interface{} {
+
 	for k := range b {
 
 		add := b[k]
-		exist, ok := c[k]
+		exist, ok := a[k]
+		if !ok {
+			a[k] = add
+			continue
+		}
+
+		existObj := exist.(map[string]interface{})
+		addObj := add.(map[string]interface{})
+
+		a[k] = merge(existObj, addObj)
+	}
+
+	return a
+}
+
+func hasConflicts(a, b map[string]interface{}) bool {
+	for k := range b {
+
+		add := b[k]
+		exist, ok := a[k]
 		if !ok {
-			c[k] = add
 			continue
 		}
 
 		existObj, existOk := exist.(map[string]interface{})
 		addObj, addOk := add.(map[string]interface{})
 		if !existOk || !addOk {
-			return nil, false
+			return true
 		}
 
-		c[k], ok = InterfaceMaps(existObj, addObj)
-		if !ok {
-			return nil, false
+		if hasConflicts(existObj, addObj) {
+			return true
 		}
 	}
-
-	return c, true
+	return false
 }
diff --git a/internal/merge/merge_test.go b/internal/merge/merge_test.go
index 7cf63770fb..a7daf81e99 100644
--- a/internal/merge/merge_test.go
+++ b/internal/merge/merge_test.go
@@ -5,12 +5,51 @@
 package merge
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
 	"github.com/open-policy-agent/opa/util"
 )
 
+func BenchmarkInterfaceMaps(b *testing.B) {
+
+	for _, size := range []int{100, 1000, 10000} {
+
+		b.Run(fmt.Sprintf("store size %v", size), func(b *testing.B) {
+			aRaw := "{"
+			for i := 0; i < size; i++ {
+				if i != 0 {
+					aRaw += ","
+				}
+				aRaw += fmt.Sprintf(`"%v":{"a":{"b":"c","d":"e"}}`, i)
+			}
+			aRaw += "}"
+
+			aVals := map[string]interface{}{}
+			if err := util.UnmarshalJSON([]byte(aRaw), &aVals); err != nil {
+				panic(err)
+			}
+			for i := 0; i < b.N; i++ {
+				bRaw := fmt.Sprintf(`{"a%v":{"b":"c","d":"e"}}`, i)
+				bVals := map[string]interface{}{}
+				if err := util.UnmarshalJSON([]byte(bRaw), &bVals); err != nil {
+					panic(err)
+				}
+
+				b.StartTimer()
+				_, ok := InterfaceMaps(aVals, bVals)
+				b.StopTimer()
+
+				if !ok {
+					b.Fatal("merging interfaces failed")
+				}
+			}
+		})
+
+	}
+}
+
 func TestMergeDocs(t *testing.T) {
 
 	tests := []struct {
@@ -30,6 +69,10 @@ func TestMergeDocs(t *testing.T) {
 		if err := util.UnmarshalJSON([]byte(tc.a), &a); err != nil {
 			panic(err)
 		}
+		aInitial := map[string]interface{}{}
+		if err := util.UnmarshalJSON([]byte(tc.a), &aInitial); err != nil {
+			panic(err)
+		}
 
 		b := map[string]interface{}{}
 		if err := util.UnmarshalJSON([]byte(tc.b), &b); err != nil {
@@ -43,6 +86,10 @@ func TestMergeDocs(t *testing.T) {
 				t.Errorf("Expected merge(%v,%v) == false but got: %v", a, b, c)
 			}
 
+			if !reflect.DeepEqual(a, aInitial) {
+				t.Errorf("Expected conflicting merge to not mutate a (%v) but got a: %v", aInitial, a)
+			}
+
 		} else {
 
 			expected := map[string]interface{}{}
@@ -54,6 +101,11 @@ func TestMergeDocs(t *testing.T) {
 			if !ok || !reflect.DeepEqual(c, expected) {
 				t.Errorf("Expected merge(%v, %v) == %v but got: %v (ok: %v)", a, b, expected, c, ok)
 			}
+
+			if reflect.DeepEqual(a, aInitial) || !reflect.DeepEqual(a, c) {
+				t.Errorf("Expected merge to mutate a (%v) but got %v", aInitial, a)
+			}
+
 		}
 	}
 }