Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce treatment service memory usage #59

38 changes: 20 additions & 18 deletions common/utils/typeconverter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ func StringSliceToListSegmenterValue(values *[]string) *_segmenters.ListSegmente
if values == nil {
return nil
}
segmenterValues := []*_segmenters.SegmenterValue{}
for _, val := range *values {
segmenterValues = append(segmenterValues, &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_String_{String_: val},
})
segmenterValues := make([]*_segmenters.SegmenterValue, len(*values))
for i := 0; i < len(*values); i++ {
segmenterValues[i] = &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_String_{String_: (*values)[i]},
}
Comment on lines -16 to +20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a series of minor changes to initialise a fixed-length slice instead of a dynamic length slice to store the segmenter values so as to prevent repeated allocation of memory to copy-paste the slice values when append-ing.

}
return &_segmenters.ListSegmenterValue{Values: segmenterValues}
}
Expand All @@ -26,9 +26,11 @@ func BoolSliceToListSegmenterValue(values *[]bool) *_segmenters.ListSegmenterVal
if values == nil {
return nil
}
segmenterValues := []*_segmenters.SegmenterValue{}
for _, val := range *values {
segmenterValues = append(segmenterValues, &_segmenters.SegmenterValue{Value: &_segmenters.SegmenterValue_Bool{Bool: val}})
segmenterValues := make([]*_segmenters.SegmenterValue, len(*values))
for i := 0; i < len(*values); i++ {
segmenterValues[i] = &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_Bool{Bool: (*values)[i]},
}
}
return &_segmenters.ListSegmenterValue{Values: segmenterValues}
}
Expand All @@ -37,11 +39,11 @@ func Int64ListToListSegmenterValue(values *[]int64) *_segmenters.ListSegmenterVa
if values == nil {
return nil
}
segmenterValues := []*_segmenters.SegmenterValue{}
for _, val := range *values {
segmenterValues = append(segmenterValues, &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_Integer{Integer: val},
})
segmenterValues := make([]*_segmenters.SegmenterValue, len(*values))
for i := 0; i < len(*values); i++ {
segmenterValues[i] = &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_Integer{Integer: (*values)[i]},
}
}
return &_segmenters.ListSegmenterValue{Values: segmenterValues}
}
Expand All @@ -50,11 +52,11 @@ func FloatListToListSegmenterValue(values *[]float64) *_segmenters.ListSegmenter
if values == nil {
return nil
}
segmenterValues := []*_segmenters.SegmenterValue{}
for _, val := range *values {
segmenterValues = append(segmenterValues, &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_Real{Real: val},
})
segmenterValues := make([]*_segmenters.SegmenterValue, len(*values))
for i := 0; i < len(*values); i++ {
segmenterValues[i] = &_segmenters.SegmenterValue{
Value: &_segmenters.SegmenterValue_Real{Real: (*values)[i]},
}
}
return &_segmenters.ListSegmenterValue{Values: segmenterValues}
}
Expand Down
50 changes: 41 additions & 9 deletions treatment-service/models/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ type ExperimentIndex struct {
stringSets map[string]*set.Set
intSets map[string]*set.Set
realSets map[string]*set.Set
boolSets map[string]*set.Set

StartTime time.Time
EndTime time.Time
Expand Down Expand Up @@ -147,18 +148,15 @@ func (i *ExperimentIndex) MarshalJSON() ([]byte, error) {
}

func (i *ExperimentIndex) matchFlagSetSegment(segmentName string, value bool) MatchStrength {
if _, exists := i.Experiment.Segments[segmentName]; !exists ||
len(i.Experiment.Segments[segmentName].GetValues()) == 0 {
set, exists := i.boolSets[segmentName]
if !exists || set.Len() == 0 {
// Optional segmenter
return MatchStrengthWeak
}

for _, val := range i.Experiment.Segments[segmentName].GetValues() {
if val.GetBool() == value {
return MatchStrengthExact
}
if set.Has(value) {
return MatchStrengthExact
}

return MatchStrengthNone
}

Expand Down Expand Up @@ -204,8 +202,7 @@ func (i *ExperimentIndex) matchRealSetSegment(segmentName string, value float64)
func (i *ExperimentIndex) matchSegment(segmentName string, values []*_segmenters.SegmenterValue) Match {
if len(values) == 0 {
// We can either have an optional match on the experiment or none.
if _, exists := i.Experiment.Segments[segmentName]; !exists ||
len(i.Experiment.Segments[segmentName].GetValues()) == 0 {
if i.checkSegmentHasWeakMatch(segmentName) {
return Match{Strength: MatchStrengthWeak, Value: nil}
}
}
Expand Down Expand Up @@ -238,6 +235,27 @@ func (i *ExperimentIndex) isActive() bool {
return (i.StartTime.Before(time.Now()) || i.StartTime.Equal(time.Now())) && i.EndTime.After(time.Now())
}

func (i *ExperimentIndex) checkSegmentHasWeakMatch(segmentName string) bool {
if set, exists := i.stringSets[segmentName]; exists {
if set.Len() > 0 {
return false
}
} else if set, exists := i.intSets[segmentName]; exists {
if set.Len() > 0 {
return false
}
} else if set, exists := i.realSets[segmentName]; exists {
if set.Len() > 0 {
return false
}
} else if set, exists := i.boolSets[segmentName]; exists {
if set.Len() > 0 {
return false
}
}
return true
}

func (s *LocalStorage) InsertProjectSettings(projectSettings *pubsub.ProjectSettings) error {
s.Lock()
defer s.Unlock()
Expand Down Expand Up @@ -362,6 +380,7 @@ func NewExperimentIndex(experiment *pubsub.Experiment) *ExperimentIndex {
stringSets := make(map[string]*set.Set)
intSets := make(map[string]*set.Set)
realSets := make(map[string]*set.Set)
boolSets := make(map[string]*set.Set)

for key, segment := range experiment.Segments {
for _, val := range segment.Values {
Expand All @@ -384,15 +403,28 @@ func NewExperimentIndex(experiment *pubsub.Experiment) *ExperimentIndex {
realSets[key] = set.New()
}
realSets[key].Insert(val.GetReal())
case *_segmenters.SegmenterValue_Bool:
_, ok := boolSets[key]
if !ok {
boolSets[key] = set.New()
}
boolSets[key].Insert(val.GetBool())
}
}
}

// Delete all segments since they have already been converted to the various sets stored in ExperimentIndex,
// and are no longer used by the Treatment Service
// TODO: To make the ExperimentIndex store only the relevant data using appropriate structs rather than
// attempting to reuse this pubsub message type and deleting the redundant data from it
experiment.Segments = nil
krithika369 marked this conversation as resolved.
Show resolved Hide resolved

return &ExperimentIndex{
Experiment: experiment,
stringSets: stringSets,
intSets: intSets,
realSets: realSets,
boolSets: boolSets,
StartTime: time.Unix(experiment.StartTime.Seconds, 0).UTC(),
EndTime: time.Unix(experiment.EndTime.Seconds, 0).UTC(),
}
Expand Down
6 changes: 5 additions & 1 deletion treatment-service/models/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ func TestExperimentIndexMatchSegment(t *testing.T) {
stringSetsVal := []interface{}{"test1"}
intSetsVal := []interface{}{int64(1)}
realSetsVal := []interface{}{1.0}
boolSetsVal := []interface{}{true}
experimentIndex := ExperimentIndex{
stringSets: map[string]*set.Set{
"stringType": set.New(stringSetsVal...),
Expand All @@ -570,6 +571,9 @@ func TestExperimentIndexMatchSegment(t *testing.T) {
realSets: map[string]*set.Set{
"realType": set.New(realSetsVal...),
},
boolSets: map[string]*set.Set{
"flagType": set.New(boolSetsVal...),
},
Experiment: &_pubsub.Experiment{
Segments: map[string]*_segmenters.ListSegmenterValue{
"stringType": {
Expand Down Expand Up @@ -672,7 +676,7 @@ func TestExperimentIndexMatchSegment(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := experimentIndex.matchSegment(tt.args.segmentName, tt.args.value)
assert.Equal(t, got, tt.want)
assert.Equal(t, tt.want, got)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flipped this around because assert.Equal was used incorrectly.

})
}
}
Expand Down