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

Use coding-conventions helpers; accelerate build with ninja. #1150

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Apr 9, 2021

  • Always fetch the coding-conventions submodule if it's missing, not just when NEURON_CMAKE_FORMAT is set.
  • Use cpp_cc_build_time_copy utility from coding-conventions instead of custom commands using cmake -E copy_if_different, this is more expressive and allows the build system (Ninja, make etc.) to check for updates more efficiently ( + makes it easier to declare dependencies on the build-tree copies of files).
  • Don't copy ${PROJECT_SOURCE_DIR}/share/lib to ${PROJECT_BINARY_DIR}/share/nrn/lib twice.

In my setup (cmake -G Ninja ...) this makes the initial run of CMake a little faster and makes incremental builds with few changes O(10x) faster (cmake --build . with no changes now takes ~1s not ~10s). It also removes some spammy output from the test infrastructure towards the end of CMake execution. When using legacy Unix Makefiles/make the improvement is smaller but still extant.

(see also BlueBrain/CoreNeuron#517 which is along the same lines)

@olupton
Copy link
Collaborator Author

olupton commented Apr 9, 2021

At least some of the failures are related to BlueBrain/hpc-coding-conventions#100.

@olupton olupton force-pushed the olupton/cmake-improvements branch 2 times, most recently from 5f138a8 to e78fb7e Compare April 9, 2021 14:13
@codecov-io
Copy link

codecov-io commented Apr 9, 2021

Codecov Report

Merging #1150 (e78fb7e) into master (80f8ba3) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1150      +/-   ##
==========================================
+ Coverage   31.39%   31.47%   +0.07%     
==========================================
  Files         571      571              
  Lines      108818   108818              
==========================================
+ Hits        34160    34246      +86     
+ Misses      74658    74572      -86     
Impacted Files Coverage Δ
...rniv/nrncore_write/callbacks/nrncore_callbacks.cpp 94.70% <0.00%> (-0.32%) ⬇️
src/parallel/bbs.cpp 64.78% <0.00%> (+1.87%) ⬆️
src/parallel/bbssrv2mpi.cpp 55.74% <0.00%> (+4.59%) ⬆️
src/nrnmpi/bbsmpipack.cpp 86.44% <0.00%> (+12.42%) ⬆️
src/parallel/bbsclimpi.cpp 50.00% <0.00%> (+20.12%) ⬆️
src/parallel/bbssrvmpi.cpp 41.77% <0.00%> (+27.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80f8ba3...e78fb7e. Read the comment docs.

@olupton olupton force-pushed the olupton/cmake-improvements branch from e78fb7e to 1c3bad9 Compare April 9, 2021 15:17
@olupton olupton requested a review from pramodk April 12, 2021 07:12
@pramodk pramodk merged commit 5a37bcd into master Apr 13, 2021
@pramodk pramodk deleted the olupton/cmake-improvements branch April 13, 2021 15:46
@alexsavulescu alexsavulescu mentioned this pull request Mar 22, 2022
15 tasks
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.

3 participants