Skip to content

docs: update CopyElement benchmark walkthrough to reflect current implementation #527

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.

Sign up for GitHub

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

Closed
Ganesh-reddy005 wants to merge 2 commits into from
Closed

docs: update CopyElement benchmark walkthrough to reflect current implementation #527

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22de385
docs: update CopyElement benchmark walkthrough to reflect current imp…
Sep 30, 2025
179d2bc
fix: correct result check in CopyElement benchmark
Oct 1, 2025
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
43 changes: 28 additions & 15 deletions docs/benchmark.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,36 @@ demonstrates all of these principles and how they are used to implement an
actual improvement to the library. Let's analyze an improvement to the
performance of `stumpless_copy_element` to do this.

An early version of `stumpless_copy_element` iterated through all of the
params in the element, adding them to the copy one by one. The code for this
looked like this snippet, which has been abbreviated to focus on the performance
of the code:
Previously, `stumpless_copy_element` manually iterated through all parameters
in an element, copying and adding them one by one. While this approach was
straightforward, it was inefficient due to repeated memory reallocations.

The function has since been optimized to handle parameter copying internally.
The benchmark now reflects this cleaner implementation:


```c
// first create a new element
copy = stumpless_new_element( stumpless_get_element_name( element ) );
static void CopyElement(benchmark::State& state){
Copy link
Owner

@goatshriek goatshriek Oct 1, 2025

Choose a reason for hiding this comment

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

You've replaced the wrong code snippet - please leave the example C code, and replace the benchmark code instead (starting on line 65 in the original file).

struct stumpless_element *element;
const struct stumpless_element *result;

// then handle all of the parameters
for( i = 0; i < element->param_count; i++ ) {
// copy the parameter
param_copy = stumpless_copy_param( element->params[i] );
INIT_MEMORY_COUNTER( copy_element );

// and then add it
stumpless_add_param( copy, param_copy );
}
element = stumpless_new_element( "copy-element-perf" );
stumpless_add_new_param( element, "param-1", "value-1" );
stumpless_add_new_param( element, "param-2", "value-2" );

for(auto _ : state){
result = stumpless_copy_element( element );
if( !result ) {
state.SkipWithError( "the element copy failed" );
} else {
stumpless_destroy_element_and_contents( result );
}
}

stumpless_destroy_element_and_contents( element );
stumpless_free_all( );
```

While it is logical and easy to follow, this method is inefficient because
Expand Down Expand Up @@ -78,8 +91,8 @@ static void CopyElement(benchmark::State& state){

for(auto _ : state){
result = stumpless_copy_element( element );
if( result <= 0 ) {
state.SkipWithError( "could not send an entry to the target" );
if( !result ) {
state.SkipWithError( "the element copy failed" );
} else {
stumpless_destroy_element_and_contents( result );
}
Expand Down