Decrease Clones

[jeniawhite]

What does this PR do?

Investigating the code, I've noticed that we are doing unnecessary clones.

This takes up significant resources from the memory and cpu time (Garbage Collection cycles).
In deepUpdates of events we are starting off with a clone in order to manipulate the timestamp and the metadata fields, also the processors rely on this clone to not alter data from my understanding.
There is no reason for deepUpdate to handle the cloning of data, this should be handled by the consumer (processors).
In order to avoid mutation of input data, I've deferred the keys and rolled them back upon completion of the function.

The only occurrence that uses this code is located in add_fields.go:

func (af *addFields) Run(event *beat.Event) (*beat.Event, error) {
	if event == nil || len(af.fields) == 0 {
		return event, nil
	}

	fields := af.fields
	if af.shared {
		fields = fields.Clone()
	}

	if af.overwrite {
		event.DeepUpdate(fields)
	} else {
		event.DeepUpdateNoOverwrite(fields)
	}

	return event, nil
}

Which is the action for the processors, and if you look at the code it is already cloning when the data is shared.
This means that we are basically double cloning and it is redundant.

The next step would be to investigate the shared variable because it seems like that it is always true and my assumption is that we do not necessary always run in a shared usecase.

After the changes I've ran a benchmark that generates 20Million small events and I saw a huge improvement on the stability of GC cycles and memory allocation.

Old code:

New code:

Why is it important?

This is a component in the heart of libbeat that manages events and is being utilized by multiple products.
This component is being utilized in the hottest paths (every event processed by this code).

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

[amitkanfer]

🚀

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2023-07-13T15:11:15.534+0000

• Duration: 74 min 47 sec

Test stats 🧪

Test	Results
Failed	0
Passed	27469
Skipped	2029
Total	29498

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[jlind23]

@jeniawhite is this one ready to be reviewed or do you still have things to do on it?

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b evgb-DecreaseClone upstream/evgb-DecreaseClone
git merge upstream/main
git push upstream evgb-DecreaseClone

[jeniawhite]

@jlind23 Removed the draft tag, ready for review.
@rdner Would be great if you'd take a look.
Thank you!

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b evgb-DecreaseClone upstream/evgb-DecreaseClone
git merge upstream/main
git push upstream evgb-DecreaseClone

[rdner]

Just comment suggestions for better clarity of what's going on.