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

[chore] [exporterhelper] Simplify queue consumption logic #8982

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Nov 22, 2023

Remove the QueueRequest wrapper. Pass consumeFunc to the queue instead of asking for the QueueRequest wrapper with callback. The callback isn't needed for the memory queue, and the field for the callback is occupying the memory even it's always empty. The bounded queue benchmark results show the RAM savings:

Before:

Benchmark_QueueUsage_50000_1_50000
Benchmark_QueueUsage_50000_1_50000-10    	       1	69813548417 ns/op	 2806232 B/op	   99753 allocs/op

After:

Benchmark_QueueUsage_50000_1_50000
Benchmark_QueueUsage_50000_1_50000-10    	       1	65498055709 ns/op	 2404832 B/op	   99754 allocs/op

Cut from #8942

@dmitryax dmitryax requested a review from a team as a code owner November 22, 2023 02:01
@dmitryax dmitryax requested a review from mx-psi November 22, 2023 02:01
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3ce3aac) 91.54% compared to head (cc775f6) 91.55%.

Files Patch % Lines
...porter/exporterhelper/internal/persistent_queue.go 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8982      +/-   ##
==========================================
+ Coverage   91.54%   91.55%   +0.01%     
==========================================
  Files         316      315       -1     
  Lines       17117    17117              
==========================================
+ Hits        15669    15672       +3     
+ Misses       1152     1150       -2     
+ Partials      296      295       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Remove the QueueRequest wrapper. Pass consumeFunc to the queue instead of asking for the QueueRequest wrapper with callback. The callback isn't needed for the memory queue, and the field for the callback is occupying the memory even it's always empty. The bounded queue benchmark results shows the RAM savings:

Before:
```
Benchmark_QueueUsage_50000_1_50000
Benchmark_QueueUsage_50000_1_50000-10    	       1	69813548417 ns/op	 2806232 B/op	   99753 allocs/op
```

After:
```
Benchmark_QueueUsage_50000_1_50000
Benchmark_QueueUsage_50000_1_50000-10    	       1	65498055709 ns/op	 2404832 B/op	   99754 allocs/op
```
@dmitryax dmitryax merged commit 7e3e725 into open-telemetry:main Nov 22, 2023
32 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 22, 2023
@dmitryax dmitryax deleted the simlify-queue branch November 22, 2023 22:19
pantuza pushed a commit to pantuza/opentelemetry-collector that referenced this pull request Dec 8, 2023
…etry#8982)

Remove the QueueRequest wrapper. Pass consumeFunc to the queue instead
of asking for the QueueRequest wrapper with callback. The callback isn't
needed for the memory queue, and the field for the callback is occupying
the memory even it's always empty. The bounded queue benchmark results
show the RAM savings:

Before:
```
Benchmark_QueueUsage_50000_1_50000
Benchmark_QueueUsage_50000_1_50000-10    	       1	69813548417 ns/op	 2806232 B/op	   99753 allocs/op
```

After:
```
Benchmark_QueueUsage_50000_1_50000
Benchmark_QueueUsage_50000_1_50000-10    	       1	65498055709 ns/op	 2404832 B/op	   99754 allocs/op
```

Cut from
open-telemetry#8942
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.

None yet

3 participants