Skip to content

Commit 7c75e47

Browse files
committed
Avoid multiple copies of the Map in replace_all_patterns
Signed-off-by: Bogdan Drutu <[email protected]>
1 parent b0f018b commit 7c75e47

File tree

2 files changed

+39
-32
lines changed

2 files changed

+39
-32
lines changed

pkg/ottl/ottlfuncs/func_replace_all_patterns.go

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -63,43 +63,45 @@ func replaceAllPatterns[K any](target ottl.PMapGetSetter[K], mode, regexPattern
6363
return nil, err
6464
}
6565

66-
updated := pcommon.NewMap()
67-
updated.EnsureCapacity(val.Len())
68-
AttributeLoop:
69-
for key, originalValue := range val.All() {
70-
switch mode {
71-
case modeValue:
72-
if originalValue.Type() == pcommon.ValueTypeStr && compiledPattern.MatchString(originalValue.Str()) {
73-
if !fn.IsEmpty() {
74-
updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, originalValue.Str(), replacementVal, replacementFormat)
75-
if err != nil {
76-
break AttributeLoop
77-
}
78-
updated.PutStr(key, updatedString)
79-
} else {
80-
updatedString := compiledPattern.ReplaceAllString(originalValue.Str(), replacementVal)
81-
updated.PutStr(key, updatedString)
66+
switch mode {
67+
case modeValue:
68+
for _, value := range val.All() {
69+
if value.Type() != pcommon.ValueTypeStr || !compiledPattern.MatchString(value.Str()) {
70+
continue
71+
}
72+
if !fn.IsEmpty() {
73+
updatedString, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, value.Str(), replacementVal, replacementFormat)
74+
if err != nil {
75+
continue
8276
}
77+
value.SetStr(updatedString)
8378
} else {
84-
originalValue.CopyTo(updated.PutEmpty(key))
79+
value.SetStr(compiledPattern.ReplaceAllString(value.Str(), replacementVal))
80+
}
81+
}
82+
case modeKey:
83+
// Because we are changing the keys we cannot do in-place update, but we can move values to the
84+
// updated map and then move back the updated map to the initial map to avoid a copy in the target.Set,
85+
// because the pcommon.Map.CopyTo will not do a copy if it is the same object in this case val.
86+
updated := pcommon.NewMap()
87+
updated.EnsureCapacity(val.Len())
88+
for key, value := range val.All() {
89+
if !compiledPattern.MatchString(key) {
90+
value.MoveTo(updated.PutEmpty(key))
91+
continue
8592
}
86-
case modeKey:
87-
if compiledPattern.MatchString(key) {
88-
if !fn.IsEmpty() {
89-
updatedKey, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, key, replacementVal, replacementFormat)
90-
if err != nil {
91-
break AttributeLoop
92-
}
93-
originalValue.CopyTo(updated.PutEmpty(updatedKey))
94-
} else {
95-
updatedKey := compiledPattern.ReplaceAllString(key, replacementVal)
96-
originalValue.CopyTo(updated.PutEmpty(updatedKey))
93+
if !fn.IsEmpty() {
94+
updatedKey, err := applyOptReplaceFunction(ctx, tCtx, compiledPattern, fn, key, replacementVal, replacementFormat)
95+
if err != nil {
96+
continue
9797
}
98+
value.MoveTo(updated.PutEmpty(updatedKey))
9899
} else {
99-
originalValue.CopyTo(updated.PutEmpty(key))
100+
value.MoveTo(updated.PutEmpty(compiledPattern.ReplaceAllString(key, replacementVal)))
100101
}
101102
}
103+
updated.MoveTo(val)
102104
}
103-
return nil, target.Set(ctx, tCtx, updated)
105+
return nil, target.Set(ctx, tCtx, val)
104106
}, nil
105107
}

pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,13 @@ func Test_replaceAllPatterns(t *testing.T) {
167167
replacementFormat: ottl.NewTestingOptional[ottl.StringGetter[pcommon.Map]](invalidPrefix),
168168
function: optionalArg,
169169
want: func(expectedMap pcommon.Map) {
170-
expectedMap.PutEmpty("test")
171-
expectedMap.Remove("test")
170+
expectedMap.PutStr("test", "hello world")
171+
expectedMap.PutStr("test2", "hello")
172+
expectedMap.PutStr("test3", "goodbye world1 and world2")
173+
expectedMap.PutInt("test4", 1234)
174+
expectedMap.PutDouble("test5", 1234)
175+
expectedMap.PutBool("test6", true)
176+
expectedMap.PutStr("test7", "")
172177
},
173178
},
174179
{

0 commit comments

Comments
 (0)