tracer: align structs

[JianyiGao]

What does this PR do?

Use fieldalignment tool to reorder struct fields in ddtrace/tracer, minimize the size of the struct in memory.
Fixed corresponding tests.

Note that two structs inside ddtrace/tracer are not reordered because of readability concern: startupInfo and groupedStats

Motivation

AIT-8461

Reviewer's Checklist

• Changed code has unit tests for its functionality at or near 100% coverage.
• There is a benchmark for any new code, or changes to existing code.
• If this interacts with the agent in a new way, a system test has been added.

For Datadog employees:

• If this PR touches code that handles credentials of any kind, such as Datadog API keys, I've requested a review from @DataDog/security-design-and-guidance.
• This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

[pr-commenter]

Benchmarks

Benchmark execution time: 2023-10-24 19:29:15

Comparing candidate commit d305a5c in PR branch jennie.gao/align-structs with baseline commit 5e9b0b1 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 40 metrics, 1 unstable metrics.

[felixge]

Thanks for working on this! Are there any plans for quantifying the memory improvements this might give us? Are we planning to enforce or strongly encourage struct alignment optimizations going forward? If yes, do we have a plan for that?

No worries if we don't have a full plan here. I'm mostly curious, not looking for reasons to push back on this.

[JianyiGao]

Thanks for the comment @felixge! Detailed documentation of memory enhancement can be found in the attached Jira ticket.
To highlight some key findings:
Structs are reduced 5 - 40% in size for most candidates, here are some examples for major improvements

abandonedspans.go:27:34: struct with 32 pointer bytes could be 16
option.go:112:13: struct of size 416 could be 360
payload.go:43:14: struct with 88 pointer bytes could be 40
rules_sampler.go:58:19: struct with 80 pointer bytes could be 48
span.go:64:11: struct with 200 pointer bytes could be 120
tracer.go:44:13: struct with 176 pointer bytes could be 112
writer.go:134:21: struct with 88 pointer bytes could be 48

Regarding automating struct alignment optimizations, one concern is that running fieldalignment -fix ./… removes floating comments on struct fields. This is due to an open issue with figuring out comments within the Go Abstract Syntax Tree. I manually put back some comments as part of this change. Enforcing this optimization as part of the CI checks might be challenging because of this issue, but we would love to hear your insights!

[felixge]

Thanks for the comment @felixge! Detailed documentation of memory enhancement can be found in the attached Jira ticket. To highlight some key findings: Structs are reduced 5 - 40% in size for most candidates, here are some examples for major improvements

abandonedspans.go:27:34: struct with 32 pointer bytes could be 16
option.go:112:13: struct of size 416 could be 360
payload.go:43:14: struct with 88 pointer bytes could be 40
rules_sampler.go:58:19: struct with 80 pointer bytes could be 48
span.go:64:11: struct with 200 pointer bytes could be 120
tracer.go:44:13: struct with 176 pointer bytes could be 112
writer.go:134:21: struct with 88 pointer bytes could be 48

Awesome, thank you so much. Those numbers look promising! What I'd also be curious about is how this translates into memory usage at runtime. Is there any plan do some testing around that?

Regarding automating struct alignment optimizations, one concern is that running fieldalignment -fix ./… removes floating comments on struct fields. This is due to an open issue with figuring out comments within the Go Abstract Syntax Tree. I manually put back some comments as part of this change. Enforcing this optimization as part of the CI checks might be challenging because of this issue, but we would love to hear your insights!

I think for the vast majority of structs the alignments doesn't matter because we don't create enough instances of them. So enforcing this across the code base in CI doesn't seem necessary unless it's really easy and convenient to setup.

As far as ensuring we don't regress on the structs where it matters: That would be nice to have, even if it's just a CI check that complains if a struct goes from optimal alignment to non-optimal alignment. Is that doable?

Also: I'm very curious about the motivation for doing all of this. IIRC we had some wins with improving alignment on some key structs (TODO: link), but I'm not sure why we're now looking to do it for all the structs? Again, I'm not against it, just trying to follow along out of curiosity :)

[JianyiGao]

What I'd also be curious about is how this translates into memory usage at runtime. Is there any plan do some testing around that?

I talked to the team and we can do benchmark testing to see how memory usage changes before and after this change. Will give an update when the numbers are available!

That would be nice to have, even if it's just a CI check that complains if a struct goes from optimal alignment to non-optimal alignment. Is that doable?

I enabled the govet in the linter with d305a5c, now the check annotations for potential improvements are available

Also: I'm very curious about the motivation for doing all of this.
I have attached a one pager in the Jira ticket hope that provides more context on the motivation behind this change.

Let me know if you have other questions!

[felixge]

Awesome! Thanks for enabling the annotations, that's fantastic 🤩

[JianyiGao]

Closing this PR as we are moving all V2 related changes to point against v2-dev branch. I will experiment adding benchmark test in the new PR and follow up. Thanks for all the input!