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

Minor debug cleanup #1099

Merged
merged 7 commits into from
Apr 26, 2024
Merged

Minor debug cleanup #1099

merged 7 commits into from
Apr 26, 2024

Conversation

biddisco
Copy link
Contributor

part 1 : Simplifies the debug helper code by removing ifdefs and using __has_include for c++ name demmangling
part 2 : declutter some of the mpi code for example by moving a namespace check into a macro so it doesn't need to be used in many places.
pushing this PR because it is in my incoming MPI change PR and can be reviewed separately

Note that removing the debug code and replacing it with fmt:: is a next step.

This hides the namespace use and cleans up a number of otherwise
useless using namespace declarations for anything inside one of the
debug macros
@pika-bot
Copy link
Collaborator

Performance test report

pika Performance

Comparison

BENCHMARKRESULT
Task Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
pika Datetime2024-02-19T15:15:15+00:002024-04-19T07:55:20+00:00
pika Commit0abc084a88fcf3
Hostnamenid00025nid00025
Envfile
Clusternamedaintdaint
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime2024-02-19T16:26:16.072067+01:002024-04-19T10:05:25.936921+02:00

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (>10%)
++/--Large performance improvement/degradation (>10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@biddisco
Copy link
Contributor Author

Added one clang format patch since I had to reformat my stuff and noticed there's a mismatch in in versions in scripts

@msimberg msimberg enabled auto-merge April 26, 2024 08:52
@msimberg msimberg added this pull request to the merge queue Apr 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 26, 2024
@msimberg msimberg merged commit 29f687d into main Apr 26, 2024
37 of 38 checks passed
@msimberg msimberg added this to the 0.25.0 milestone May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Archive
Development

Successfully merging this pull request may close these issues.

3 participants