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

WIP: internal/traceprof: make endpoint call counting efficient #3154

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nsrip-dd
Copy link
Contributor

@nsrip-dd nsrip-dd commented Feb 5, 2025

Switch to a data structure designed for efficient concurrent updates for
endpoint call counting. This is a prerequisite for enabling the feature
by default.

TODO:

  • Say more about the choice of library. Is it well tested? Correct?
  • Think harder about corner cases, especially around the size limit
    and concurrent modification when reading

Benchmark results on my M1 Max Macbook Pro:

goos: darwin
goarch: arm64
pkg: gopkg.in/DataDog/dd-trace-go.v1/internal/traceprof
cpu: Apple M1 Max
				 │   before.txt   │               after.txt               │
				 │     sec/op     │    sec/op      vs base                │
EndpointCounter/enabled=true-10    139.800n ± 11%    5.000n ± 18%  -96.42% (p=0.000 n=10)
EndpointCounter/enabled=false-10    0.2716n ±  5%   0.2682n ±  1%        ~ (p=0.118 n=10)
geomean                              6.162n          1.158n        -81.21%

				 │  before.txt  │              after.txt              │
				 │     B/op     │    B/op     vs base                 │
EndpointCounter/enabled=true-10    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
EndpointCounter/enabled=false-10   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                       ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

				 │  before.txt  │              after.txt              │
				 │  allocs/op   │ allocs/op   vs base                 │
EndpointCounter/enabled=true-10    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
EndpointCounter/enabled=false-10   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                       ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

@nsrip-dd nsrip-dd force-pushed the nick.ripley/fast-prof-endpoint-call-counting branch from fab6af7 to 7d9e01e Compare February 5, 2025 17:30
@pr-commenter
Copy link

pr-commenter bot commented Feb 5, 2025

Benchmarks

Benchmark execution time: 2025-02-05 18:31:39

Comparing candidate commit 071ccfd in PR branch nick.ripley/fast-prof-endpoint-call-counting with baseline commit 739be25 in branch main.

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

scenario:BenchmarkHttpServeTrace-24

  • 🟥 execution_time [+336.269ns; +452.731ns] or [+2.098%; +2.825%]

scenario:BenchmarkSetTagStringPtr-24

  • 🟥 execution_time [+4.396ns; +8.324ns] or [+2.533%; +4.796%]

Switch to a data structure designed for efficient concurrent updates for
endpoint call counting. This is a prerequisite for enabling the feature
by default. TODO: say more about the choice of library

Benchmark results on my M1 Max Macbook Pro:

	goos: darwin
	goarch: arm64
	pkg: gopkg.in/DataDog/dd-trace-go.v1/internal/traceprof
	cpu: Apple M1 Max
					 │   before.txt   │               after.txt               │
					 │     sec/op     │    sec/op      vs base                │
	EndpointCounter/enabled=true-10    139.800n ± 11%    5.000n ± 18%  -96.42% (p=0.000 n=10)
	EndpointCounter/enabled=false-10    0.2716n ±  5%   0.2682n ±  1%        ~ (p=0.118 n=10)
	geomean                              6.162n          1.158n        -81.21%

					 │  before.txt  │              after.txt              │
					 │     B/op     │    B/op     vs base                 │
	EndpointCounter/enabled=true-10    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
	EndpointCounter/enabled=false-10   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
	geomean                                       ²               +0.00%                ²
	¹ all samples are equal
	² summaries must be >0 to compute geomean

					 │  before.txt  │              after.txt              │
					 │  allocs/op   │ allocs/op   vs base                 │
	EndpointCounter/enabled=true-10    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
	EndpointCounter/enabled=false-10   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
	geomean                                       ²               +0.00%                ²
	¹ all samples are equal
	² summaries must be >0 to compute geomean
@nsrip-dd nsrip-dd force-pushed the nick.ripley/fast-prof-endpoint-call-counting branch from 7d9e01e to 071ccfd Compare February 5, 2025 18:05
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Feb 5, 2025

Datadog Report

Branch report: nick.ripley/fast-prof-endpoint-call-counting
Commit report: 83474b9
Test service: dd-trace-go

✅ 0 Failed, 5222 Passed, 74 Skipped, 2m 14.34s Total Time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant