avoid extra json marshal when loading configuration from cache (FF-1564) (#31)

leoromanovsky · web-flow · commit 283a2e942d2c · 2024-02-15T16:25:55.000-08:00
diff --git a/.github/workflows/memory-profile-compare.yml b/.github/workflows/memory-profile-compare.yml
@@ -54,9 +54,15 @@ jobs:
         name: memprofile-main
         path: .
     
+    - name: Display structure of downloaded files
+      run: ls -R
+    
     - name: Compare memory profiles
-      run: make profile-memory-compare BASE_FILE=memprofile.main.out COMPARE_FILE=memprofile.feat.out > memory-profile-comparison.text
+      run: make profile-memory-compare BASE_FILE=memprofile.main.out FEAT_FILE=memprofile.feat.out > memory-profile-comparison.text
     
+    - name: Show memory profile comparison
+      run: cat memory-profile-comparison.text
+
     - name: Upload comparison result
       uses: actions/upload-artifact@v2
       with:
diff --git a/eppoclient/client.go b/eppoclient/client.go
@@ -3,7 +3,6 @@ package eppoclient
 import (
 	"crypto/md5"
 	"encoding/hex"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"time"
@@ -103,23 +102,6 @@ func (ec *EppoClient) getAssignment(subjectKey string, flagKey string, subjectAt
 
 	assignedVariation := variationShard.Value
 
-	// Log assignment
-	assignmentEvent := AssignmentEvent{
-		Experiment:        flagKey + "-" + rule.AllocationKey,
-		FeatureFlag:       flagKey,
-		Allocation:        rule.AllocationKey,
-		Variation:         assignedVariation,
-		Subject:           subjectKey,
-		Timestamp:         time.Now().String(),
-		SubjectAttributes: subjectAttributes,
-	}
-
-	_, jsonErr := json.Marshal(assignmentEvent)
-
-	if jsonErr != nil {
-		panic("incorrect json")
-	}
-
 	func() {
 		// need to catch panics from Logger and continue
 		defer func() {
@@ -129,6 +111,16 @@ func (ec *EppoClient) getAssignment(subjectKey string, flagKey string, subjectAt
 			}
 		}()
 
+		// Log assignment
+		assignmentEvent := AssignmentEvent{
+			Experiment:        flagKey + "-" + rule.AllocationKey,
+			FeatureFlag:       flagKey,
+			Allocation:        rule.AllocationKey,
+			Variation:         assignedVariation,
+			Subject:           subjectKey,
+			Timestamp:         time.Now().String(),
+			SubjectAttributes: subjectAttributes,
+		}
 		ec.logger.LogAssignment(assignmentEvent)
 	}()
 
diff --git a/eppoclient/configurationrequestor.go b/eppoclient/configurationrequestor.go
@@ -7,6 +7,10 @@ import (
 
 const RAC_ENDPOINT = "/randomized_assignment/v3/config"
 
+type racResponse struct {
+	Flags map[string]experimentConfiguration `json:"flags"`
+}
+
 type iConfigRequestor interface {
 	GetConfiguration(key string) (experimentConfiguration, error)
 	FetchAndStoreConfigurations()
@@ -36,24 +40,21 @@ func (ecr *experimentConfigurationRequestor) GetConfiguration(experimentKey stri
 }
 
 func (ecr *experimentConfigurationRequestor) FetchAndStoreConfigurations() {
-	var responseBody map[string]json.RawMessage
-
-	configs := dictionary{}
 	result := ecr.httpClient.get(RAC_ENDPOINT)
+	var wrapper racResponse
 
-	err := json.Unmarshal([]byte(result), &responseBody)
-
+	// Unmarshal JSON data directly into the wrapper struct
+	err := json.Unmarshal([]byte(result), &wrapper)
 	if err != nil {
-		fmt.Println("Failed to unmarshal RAC response json", result)
+		fmt.Println("Failed to unmarshal RAC response JSON", result)
 		fmt.Println(err)
+		return
 	}
 
-	err = json.Unmarshal(responseBody["flags"], &configs)
-
+	// Now wrapper.Flags contains all configurations mapped by their keys
+	// Pass this map directly to SetConfigurations
+	err = ecr.configStore.SetConfigurations(wrapper.Flags)
 	if err != nil {
-		fmt.Println("Failed to unmarshal RAC response json in experiments section", result)
-		fmt.Println(err)
+		fmt.Println("Failed to set configurations in cache", err)
 	}
-
-	ecr.configStore.SetConfigurations(configs)
 }
diff --git a/eppoclient/configurationstore.go b/eppoclient/configurationstore.go
@@ -1,15 +1,12 @@
 package eppoclient
 
 import (
-	"encoding/json"
 	"errors"
-	"log"
-
-	lru "github.com/hashicorp/golang-lru"
+	lru "github.com/hashicorp/golang-lru/v2"
 )
 
 type configurationStore struct {
-	cache *lru.Cache
+	cache *lru.Cache[string, experimentConfiguration]
 }
 
 type Variation struct {
@@ -35,7 +32,7 @@ type experimentConfiguration struct {
 func newConfigurationStore(maxEntries int) *configurationStore {
 	var configStore = &configurationStore{}
 
-	lruCache, err := lru.New(maxEntries)
+	lruCache, err := lru.New[string, experimentConfiguration](maxEntries)
 	configStore.cache = lruCache
 
 	if err != nil {
@@ -46,29 +43,18 @@ func newConfigurationStore(maxEntries int) *configurationStore {
 }
 
 func (cs *configurationStore) GetConfiguration(key string) (expConfig experimentConfiguration, err error) {
-	value, _ := cs.cache.Get(key)
-
-	if value == nil {
-		err = errors.New("not found")
-		return
-	}
-
-	jsonString, err := json.Marshal(value)
-
-	if err != nil {
-		log.Fatalln("Incorrect json")
-	}
-	ec := experimentConfiguration{}
-	err = json.Unmarshal(jsonString, &ec)
-	if err != nil {
-		log.Fatalln("failure to unmarshal json into experiment configuration")
+	// Attempt to get the value from the cache
+	expConfig, ok := cs.cache.Get(key)
+	if !ok {
+		return expConfig, errors.New("configuration not found in cache")
 	}
 
-	return ec, nil
+	return expConfig, nil
 }
 
-func (cs *configurationStore) SetConfigurations(configs dictionary) {
-	for key, element := range configs {
-		cs.cache.Add(key, element)
+func (cs *configurationStore) SetConfigurations(configs map[string]experimentConfiguration) error {
+	for key, config := range configs {
+		cs.cache.Add(key, config)
 	}
+	return nil
 }
diff --git a/eppoclient/configurationstore_test.go b/eppoclient/configurationstore_test.go
@@ -21,38 +21,55 @@ const TEST_MAX_SIZE = 10
 
 func Test_GetConfiguration_unknownKey(t *testing.T) {
 	var store = newConfigurationStore(TEST_MAX_SIZE)
-	store.SetConfigurations(dictionary{"randomization_algo": testExp})
-	_, err := store.GetConfiguration("unknown_exp")
+	err := store.SetConfigurations(map[string]experimentConfiguration{
+		"randomization_algo": testExp,
+	})
+
+	assert.NoError(t, err)
+	result, err := store.GetConfiguration("unknown_exp")
 
 	assert.Error(t, err)
+	assert.Equal(t, experimentConfiguration{}, result)
 }
 
 func Test_GetConfiguration_knownKey(t *testing.T) {
 	var store = newConfigurationStore(TEST_MAX_SIZE)
-	store.SetConfigurations(dictionary{"randomization_algo": testExp})
-	result, _ := store.GetConfiguration("randomization_algo")
+	err := store.SetConfigurations(map[string]experimentConfiguration{
+		"randomization_algo": testExp,
+	})
+	assert.NoError(t, err)
+	result, err := store.GetConfiguration("randomization_algo")
 
 	expected := "randomization_algo"
 
+	assert.NoError(t, err)
 	assert.Equal(t, expected, result.Name)
 }
 
 func Test_GetConfiguration_evictsOldEntriesWhenMaxSizeExceeded(t *testing.T) {
 	var store = newConfigurationStore(TEST_MAX_SIZE)
-	store.SetConfigurations(dictionary{"item_to_be_evicted": testExp})
-	result, _ := store.GetConfiguration("item_to_be_evicted")
+	err := store.SetConfigurations(map[string]experimentConfiguration{
+		"item_to_be_evicted": testExp,
+	})
+	assert.NoError(t, err)
+	result, err := store.GetConfiguration("item_to_be_evicted")
 
 	expected := "randomization_algo"
+	assert.NoError(t, err)
 	assert.Equal(t, expected, result.Name)
 
 	for i := 0; i < TEST_MAX_SIZE; i++ {
 		dictKey := fmt.Sprintf("test-entry-%v", i)
-		store.SetConfigurations(dictionary{dictKey: testExp})
+		err := store.SetConfigurations(map[string]experimentConfiguration{
+			dictKey: testExp,
+		})
+		assert.NoError(t, err)
 	}
 
-	result, err := store.GetConfiguration("item_to_be_evicted")
+	result, err = store.GetConfiguration("item_to_be_evicted")
 	assert.Error(t, err)
 
-	result, _ = store.GetConfiguration(fmt.Sprintf("test-entry-%v", TEST_MAX_SIZE-1))
+	result, err = store.GetConfiguration(fmt.Sprintf("test-entry-%v", TEST_MAX_SIZE-1))
+	assert.NoError(t, err)
 	assert.Equal(t, expected, result.Name)
 }
diff --git a/go.mod b/go.mod
@@ -3,7 +3,7 @@ module github.com/Eppo-exp/golang-sdk/v2
 go 1.19
 
 require (
-	github.com/hashicorp/golang-lru v0.5.4
+	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/stretchr/testify v1.8.0
 )
 
diff --git a/go.sum b/go.sum
@@ -1,8 +1,8 @@
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
-github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
+github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
+github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@ module github.com/Eppo-exp/golang-sdk/v2`
`3`	`3`	`go 1.19`
`4`	`4`
`5`	`5`	`require (`
`6`		`- github.com/hashicorp/golang-lru v0.5.4`
	`6`	`+ github.com/hashicorp/golang-lru/v2 v2.0.7`
`7`	`7`	`github.com/stretchr/testify v1.8.0`
`8`	`8`	`)`
`9`	`9`