Open rrsettgast opened 2 years ago
On the note of circular dependencies, I ran the tools we added last time we looked at circular dependencies in #1352:
Running config-build.py
with the --graphviz
option (code link) produces the graph of the dependencies according to CMake.
Note: In order to get this graph, I built GEOSX with shared libraries on and object libraries turned off. For whatever reason, I couldn't get CMake to detect the object libraries registered through blt to show up.
By CMake's view, we don't have any circular dependencies currently.
This script produces two files: circular_dependencies.txt and dependency_matrix.txt (Excel view).
circular_dependencies.txt
shows what header includes exist between two components.
Currently, there are 9:
dependency_matrix.txt
shows if a component contains headers to another component (yes/no)
I did the following to run this script:
ml ninja (load the module)
./scripts/config-build.py -hc host-configs/LLNL/quartz-clang\@10.0.0.cmake -ecc -G Ninja
python3 ./scripts/find_cycles.py -c build-quartz-clang\@10.0.0-debug/compile_commands.json
Output on quartz: find_cycles_quartz_output.txt
The output shows 42 cycles, but most of them look to be outside GEOSX and in individual third-party libraries. Notably, the first cycle is a header including itself.
This is a really fruitful analysis, thx @bmhan12
In a nutshell, is it fair to state that we cannot really find real C++
cycles, but that our C++
architecture does not respect our cmake
architecture?
PS: There is an option in find_cycles .py
to remove some directories.
PPS: find_cycles.py
could be improved to add the cmake
clustering into the game! It's missing this option to find this other kind of cycles.
The current picture from cmake not looking good:
Describe the issue At some point we were having problems running nsightcompute when dynamically linking our libraries, so we switched to static linking. This is undesirable for two reasons:
Proposed cleanup Switch back to dynamic linking as default. I think that this requires us to clean up our circular dependencies once and for all. We should maintain the option to statically link in case there are any problems later.