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

CMake: Add separate configure-time option for MsgStack #2309

Merged
merged 6 commits into from
May 25, 2021
Merged

Conversation

dschwoerer
Copy link
Contributor

Formatting took over 50% of the run time, so I added an option to disable it while keeping the full checks enabled.

This is really useful if you want to debug on a small mesh, as you get the stack trace anyway with e.g. gdb.

dschwoerer and others added 3 commits May 4, 2021 15:29
fmt is very slow if used with debugging. Disabling the msgstack can give
a signficicant speedup.
@dschwoerer dschwoerer added the build-system issues with make/configure/... label May 7, 2021
@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2021

clang-tidy review says "All clean, LGTM! 👍"

@ZedThree
Copy link
Member

ZedThree commented May 7, 2021

Might make sense to make it just on/off, with a default value of CHECK > 1? If so, the name should be BOUT_USE_MSGSTACK

Please could you also add it to include/bout/build_config.hxx, and write it out in src/bout++.cxx, in printCompileTimeOptions, setupDumpFile?

The other option would be to just bump up the CHECK level that it gets enabled at.

@dschwoerer
Copy link
Contributor Author

Might make sense to make it just on/off, with a default value of CHECK > 1? If so, the name should be BOUT_USE_MSGSTACK

I don't mind, will change 👍

Please could you also add it to include/bout/build_config.hxx, and write it out in src/bout++.cxx, in printCompileTimeOptions, setupDumpFile?

Sure 👍

The other option would be to just bump up the CHECK level that it gets enabled at.

Even at level 4 I wouldn't want them, as they add no value if I already have compiled with -g.
Without -g and/or without the native stacktrace the msgstack would add value, but I prefer -g as that gives much more info at smaller cost.

@dschwoerer
Copy link
Contributor Author

I cannot find anything in the docs on this, is it possible to evaluate a statement without if?

I want to assign the result of the comparison CHECK > 1 - is there another way to:

if (CHECK GREATER 1)
  set(VALUE ON)
else ()
  set(VALUE OFF)
endif()

@ZedThree
Copy link
Member

Yeah, I think that's the cleanest way. Everything is stringly-typed in CMake, so boolean expressions basically only work inside if

It is simpler, and only slightly reduces the usecases.
Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

LGTM, pick one of the suggestions depending on how you want to use this :)

CMakeLists.txt Outdated Show resolved Hide resolved
dschwoerer and others added 2 commits May 25, 2021 14:40
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/bout++.cxx Show resolved Hide resolved
@ZedThree ZedThree changed the title Split msg stack CMake: Add separate configure-time option for MsgStack May 25, 2021
@ZedThree ZedThree merged commit 6fd6122 into next May 25, 2021
@ZedThree ZedThree deleted the split-msg-stack branch May 25, 2021 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system issues with make/configure/...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants