Skip to content

Conversation

GuillaumeLagrange
Copy link
Contributor

No description provided.

Copy link

linear bot commented Feb 21, 2025

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch 7 times, most recently from 967e2d1 to 11f563e Compare February 21, 2025 12:59
Copy link

codspeed-hq bot commented Feb 21, 2025

CodSpeed Instrumentation Performance Report

Congrats! CodSpeed is installed 🎉

🆕 7 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks

  • BM_StringCopy (7.7 µs)
  • BM_memcpy[4096] (15.6 µs)
  • BM_memcpy[512] (8.3 µs)
  • BM_memcpy[64] (7.5 µs)
  • BM_memcpy[8192] (21.4 µs)
  • BM_memcpy[8] (7.5 µs)
  • BM_rand_vector (6.8 µs)

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch 3 times, most recently from 8fceb44 to 4601025 Compare February 21, 2025 13:32
@art049
Copy link
Member

art049 commented Feb 24, 2025

As discussed, we need to generate the debug symbols as well

@art049
Copy link
Member

art049 commented Feb 24, 2025

Also, please use a better cmake formater

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch 4 times, most recently from a4d52fb to 1fe4db9 Compare February 26, 2025 14:46
@GuillaumeLagrange
Copy link
Contributor Author

As discussed, we need to generate the debug symbols as well

By default in the example, debug symbols are present. I think the best wya to handle this will be to have a specific callout to make sure the users enable debug symbols when building, not much more we can do as a bench library

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch 8 times, most recently from 8271fbd to 582a6d5 Compare February 26, 2025 15:19
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch 3 times, most recently from bf3c469 to fb9bf9b Compare February 26, 2025 15:37
@GuillaumeLagrange
Copy link
Contributor Author

@art049 you can review from 651190d

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch from fb9bf9b to dbbabfb Compare February 27, 2025 09:15
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch from dbbabfb to 0a4db23 Compare March 3, 2025 08:50
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch from 0a4db23 to 3830af0 Compare March 3, 2025 09:48
Comment on lines +468 to +471
b.Setup();
State st = b.RunInstrumented(CodSpeed::getInstance(), &timer, manager.get(),
nullptr, nullptr);
b.Teardown();
Copy link
Member

Choose a reason for hiding this comment

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

Is the original behavior to call setup and teardown just once before and after all the iterations? (instead of running before/after each iteration)? We might create some inconsistencies if that's not the case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I gathered from here

And here

Apparently it's one setup/teardown call per "repetition" aka round

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-288-support-instrumentation-withgooglebenchmark branch from 3830af0 to 4772cff Compare March 7, 2025 16:29
@GuillaumeLagrange GuillaumeLagrange requested a review from art049 March 7, 2025 16:30
@GuillaumeLagrange GuillaumeLagrange merged commit 4772cff into main Mar 7, 2025
3 checks passed
@GuillaumeLagrange GuillaumeLagrange deleted the cod-288-support-instrumentation-withgooglebenchmark branch March 12, 2025 15:48
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.

2 participants