[BUG] Consider removing spdlog dependency for substantial compile time improvements

[ahendriksen]

Describe the bug

Including the spdlog headers is quite expensive. Just adding #include <spdlog/spdlog.h> to an empty file adds 2.8 seconds to the compilation time. For the pairwise distance kernels in the RAFT library, removing the spdlog include can reduce compile times by 50%.

Steps/Code to reproduce bug

time nvcc -arch sm_70 -I/path/to/spdlog/include -x cu -c <(echo '// empty file')
real    0m1.042s

time nvcc -arch sm_70 -I/path/to/spdlog/include -x cu -c <(echo '#include <spdlog/spdlog.h> ')
real    0m3.840s

Expected behavior A smaller increase in compile time. For context, including <string> adds on the order of 100ms to the compilation time:

time nvcc -arch sm_70 -I/path/to/spdlog/include -x cu -c <(echo '#include <string> ')
real    0m1.160s

Additional context

RAFT RAFT also uses spdlog. An issue has been opened there as well.

Reason The reason that compilation takes much longer is that spdlog instantiates a bunch of templates in every translation unit when used as a header only library. This happens in pattern_formatter::handle_flag_, which is instantiated here. Just adding back the spdlog header doubles the compile times of cicc (device side) and also gcc on the host side.

Precompiled-library Another option is to not use spdlog as a header only library. The effect can be simulated by defining SPDLOG_COMPILED_LIB. When this is defined, spdlog adds only 0.5 seconds:

time nvcc -DSPDLOG_COMPILED_LIB -arch sm_70 -I/home/ahendriksen/projects/raft-spdlog-issue/cpp/build/_deps/spdlog-src/include -x cu -c <(echo '#include <spdlog/spdlog.h> ')
real    0m1.520s

time nvcc -DSPDLOG_COMPILED_LIB -arch sm_70 -I/home/ahendriksen/projects/raft-spdlog-issue/cpp/build/_deps/spdlog-src/include -x cu -c <(echo '//empty file')
real    0m1.053s

[cjnolet]

What if we were to make the use of header-only SPDLOG optiona (and maybe keep it on default)? While I was investigating the compile times on RAFT, I happened to notice that the only different on the CMAKE side to enable this should be changing the spdlog target that rmm links against.

[harrism]

PR welcome.

[vyasr]

@ahendriksen is this still worth pursuing, or were the changes in #1241 sufficient that we don't need to worry about this anymore? If we do still want this, do we need to reopen #1232?

[ahendriksen]

I would say: let's keep it in the back of mind, but the urgency for removing spdlog is gone.

For RAFT, the compile time issues have been mostly resolved by #1241. RAFT works around the inclusion of spdlog in pool_memory_resource.hpp (overview).

For projects starting out with RMM, the out-of-the-box compile time should be greatly improved.

Summary:

• Issue can be closed as far as RAFT is concerned.
• Other RAPIDS projects may benefit from #1232. I cannot speak for their demand.
• A nice to have would be to make sure that the pool memory resource also includes spdlog only in debug builds.