Skip to content

fix: [Geneva Exporter] Add LZ4 compression benchmark #284

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions opentelemetry-exporter-geneva/geneva-uploader/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ wiremock = "0.6"
futures = "0.3"
num_cpus = "1.16"
lz4_flex = { version = "0.11" }
criterion = {version = "0.6"}
rand = {version = "0.9"}

[lints]
workspace = true
46 changes: 46 additions & 0 deletions opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#[cfg(test)]
mod benchmarks {
use crate::payload_encoder::lz4_chunked_compression::lz4_chunked_compression;
use criterion::{BenchmarkId, Criterion, Throughput};
use rand::{rngs::StdRng, Rng, SeedableRng};
use std::hint::black_box;

fn setup_test_data(size: usize) -> Vec<u8> {
let mut data = vec![0u8; size];
let mut rng = StdRng::seed_from_u64(42);
rng.fill(&mut data[..]);
data
}

Check warning on line 13 in opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs#L8-L13

Added lines #L8 - L13 were not covered by tests

/*
- Criterion benchmarks (lz4_chunked_compression, lz4_flex backend):

| Input size | Median time | Throughput |
|-------------|--------------------|--------------------|
| 1 byte | ~533 ns | ~1.6 MiB/s |
| 1 KiB | ~597 ns | ~1.5 GiB/s |
| 1 MiB | ~42 us | ~22.1 GiB/s |
- No significant regressions or improvements detected.
- Machine: Apple M4 Pro, 24 GB, Total Number of Cores: 14 (10 performance and 4 efficiency)
*/
#[test]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setup looks a bit unusual. Does this mean that we would be running this benchmark in the CI test? I think we only want to run this with cargo bench. Could we follow something similar to how we have added benchmarks in other methods?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is testing the crate private code, which won't be possible under benches/*. Let me see if this is something which can be re-exported as public specifically for benchmark, and then add it there. Else, will make it to be ignored with cargo test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I would suggest using [[bench]] target in that case and keep the benchmarks under src/. That way you can access private methods of the crate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, have made to be ignored by cargo test.

#[ignore = "benchmark on crate private, ignored by default during normal test runs"]
/// To run: $cargo test --release lz4_benchmark -- --nocapture --ignored
fn lz4_benchmark() {
let mut criterion = Criterion::default();

let mut group = criterion.benchmark_group("lz4_compression");

Check warning on line 32 in opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs#L29-L32

Added lines #L29 - L32 were not covered by tests

for size in [1, 1024, 1024 * 1024] {
let data = setup_test_data(size);

group.throughput(Throughput::Bytes(size as u64));
group.bench_with_input(BenchmarkId::new("lz4_flex", size), &data, |b, data| {
b.iter(|| black_box(lz4_chunked_compression(data).unwrap()));
});
}

Check warning on line 41 in opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs#L34-L41

Added lines #L34 - L41 were not covered by tests

group.finish();
criterion.final_summary();
}

Check warning on line 45 in opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-exporter-geneva/geneva-uploader/src/bench.rs#L43-L45

Added lines #L43 - L45 were not covered by tests
}
5 changes: 4 additions & 1 deletion opentelemetry-exporter-geneva/geneva-uploader/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
mod config_service;
pub mod ingestion_service;
mod payload_encoder;
pub(crate) mod payload_encoder;

mod uploader;

#[cfg(test)]
mod bench;

#[allow(unused_imports)]
pub(crate) use config_service::client::{
AuthMethod, GenevaConfigClient, GenevaConfigClientConfig, GenevaConfigClientError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ use lz4_flex::block::{compress_into, get_maximum_output_size};
pub(crate) fn lz4_chunked_compression(
input: &[u8],
) -> Result<Vec<u8>, lz4_flex::block::CompressError> {
const CHUNK_SIZE: usize = 64 * 1024;
lz4_chunked_compression_custom::<{ 64 * 1024 }>(input)
}

#[allow(dead_code)]
pub(crate) fn lz4_chunked_compression_custom<const CHUNK_SIZE: usize>(
input: &[u8],
) -> Result<Vec<u8>, lz4_flex::block::CompressError> {
let max_chunk_compressed = get_maximum_output_size(CHUNK_SIZE);

// Pre-allocate an output buffer large enough for the worst-case total output size:
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
mod lz4_chunked_compression;
pub(crate) mod lz4_chunked_compression;