open-telemetry / opentelemetry-cpp

The OpenTelemetry C++ Client
https://opentelemetry.io/
Apache License 2.0
811 stars 391 forks source link

[SDK] Added reserve for spans array in BatchSpanProcessor. #2724

Closed msiddhu closed 3 days ago

msiddhu commented 4 days ago

Changes

Added .reserve for spans_arr in BatchSpanProcessor (Small Optimization)

Helps to allocate the amount of memory needed for number of records so that dynamic memory allocation doesn't happen in the consume method.

.push_back() reallocates memory each time the method is called.

Using .reserve() would avoid memory reallocation as already the memory is allocated.

References: C++ Vector push_back C++ Vector reserve

linux-foundation-easycla[bot] commented 4 days ago

CLA Signed

The committers listed above are authorized under a signed CLA.

codecov[bot] commented 4 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.67%. Comparing base (497eaf4) to head (17aeaba). Report is 91 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/open-telemetry/opentelemetry-cpp/pull/2724/graphs/tree.svg?width=650&height=150&src=pr&token=FJESTYQ2AD&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)](https://app.codecov.io/gh/open-telemetry/opentelemetry-cpp/pull/2724?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) ```diff @@ Coverage Diff @@ ## main #2724 +/- ## ========================================== + Coverage 87.12% 87.67% +0.56% ========================================== Files 200 190 -10 Lines 6109 5855 -254 ========================================== - Hits 5322 5133 -189 + Misses 787 722 -65 ``` | [Files](https://app.codecov.io/gh/open-telemetry/opentelemetry-cpp/pull/2724?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) | Coverage Δ | | |---|---|---| | [sdk/src/logs/batch\_log\_record\_processor.cc](https://app.codecov.io/gh/open-telemetry/opentelemetry-cpp/pull/2724?src=pr&el=tree&filepath=sdk%2Fsrc%2Flogs%2Fbatch_log_record_processor.cc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-c2RrL3NyYy9sb2dzL2JhdGNoX2xvZ19yZWNvcmRfcHJvY2Vzc29yLmNj) | `89.29% <100.00%> (+1.79%)` | :arrow_up: | | [sdk/src/trace/batch\_span\_processor.cc](https://app.codecov.io/gh/open-telemetry/opentelemetry-cpp/pull/2724?src=pr&el=tree&filepath=sdk%2Fsrc%2Ftrace%2Fbatch_span_processor.cc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-c2RrL3NyYy90cmFjZS9iYXRjaF9zcGFuX3Byb2Nlc3Nvci5jYw==) | `94.12% <100.00%> (+1.70%)` | :arrow_up: | ... and [105 files with indirect coverage changes](https://app.codecov.io/gh/open-telemetry/opentelemetry-cpp/pull/2724/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)
msiddhu commented 3 days ago

Thanks for suggesting. I think CI Checks should starting running now.

lalitb commented 3 days ago

Similar change can be done for BatchLogProcessor. @msiddhu just in case you can handle as separate as same PR :)

msiddhu commented 3 days ago

Thanks for pointing out. Made the suggested changes. Please review.