Use spdlog for logging in Gloo #470
Conversation
| set(BUILD_SHARED_LIBS ${_save_BUILD_SHARED_LIBS}) | ||
|
|
||
| if(GLOO_STATIC_OR_SHARED STREQUAL "STATIC") | ||
| list(APPEND gloo_ALL_TARGETS_DEPENDENCY_LIBS $<BUILD_INTERFACE:spdlog::spdlog_header_only>) |
| spdlog::set_level(spdlog::level::warn); | ||
| /* Override log level if the environment variable is set. */ | ||
| spdlog::cfg::load_env_levels("GLOO_LOG_LEVEL"); | ||
|
|
||
| /* Set custom format. This is similiar to PyTorch's format. Equivalent to: | ||
| * [{short-log-level}{month-num}{day-of-month} {hour}:{min}:{sec}.{microsec} | ||
| * {threadid} {source_file}:{line_no}] {MESSAGE} */ | ||
| spdlog::set_pattern("[%L%m%d %H:%M:%S.%f %t %s:%#] %v"); |
There was a problem hiding this comment.
Is there any way to create a non-global logger that's gloo specific? just thinking about potential conflicts with other libraries
There was a problem hiding this comment.
I'm not sure if I understand what you mean, sorry. This logger is specific to Gloo, in that no other consumer of Gloo can use it - the log header is not public, and spdlog is built into Gloo at compile time as a private dependency. If we were using a shared spdlog library, I guess we would have the opportunity to share loggers with other co-existing libraries using the same version of spdlog (which has advantages and disadvantages), but I imagine it's moot as we don't want to introduce a dependency there.
| * usual | ||
| */ | ||
| inline float format_as(float16 v) { | ||
| return cpu_half2float(v); |
There was a problem hiding this comment.
does this cause any issues with PyTorch? just wondering about header conflicts for rules like this
There was a problem hiding this comment.
I think this shouldn't have the potential to cause any conflicts - both float16 and this format_as impl are in the gloo namespace.
jackbondpreston-arm
force-pushed
the
jack/spdlog-main
branch
from
f9546cd to
cd3c648
Compare
|
I've pushed a new version containing the CI update to enable fetching the submodule. This should hopefully resolve the CI build failures. The superlinter failure can be ignored, I would say. It is failing only on Markdown, and it is failing on Markdown that was not added/changed in this PR (it seems to lint and enforce on the whole file if you just change a small section) - and doesn't necessarily seem to be suggesting changes that we would even want to make, e.g.: |
Also modifies Github CI config to ensure the checkout action also checks out the submodule.
spdlog is included and built from source, using the third_party/spdlog submodule. When Gloo is built as a static library, spdlog is used in its header-only library form. When Gloo is built as a shared library, spdlog is built statically and included. A key goal is to make spdlog a dependency internal to Gloo only. Gloo users should not have to install spdlog themselves. This commit does not add any usages of spdlog in Gloo, just the build infrastructure to use it.
Change Gloo logging macros to use SPDLOG_* macros for logging. This
change is largely mechanical - changing the structure of the logging
calls to use fmtlib::fmt-style formatting. For example:
GLOO_DEBUG("thingOne=", thingOne, " thingTwo=", thingTwo);
becomes:
GLOO_DEBUG("thingOne={} thingTwo={}", thingOne, thingTwo);
However, there are also some less mechanical changes:
- Since we don't want to expose spdlog as a public dependency of Gloo,
the logging infrastructure is moved into a different header
(gloo/common/log.h). This header is not included in GLOO_HDRS, so it
is not installed. It should only ever be included by .cc files, or
non-public headers.
- Due to the previous change, the GLOO_ENFORCE_* macros are moved into
a separate header (gloo/common/enforce.h). These are used often in
public header files, and are not inherently tied to the logging, so
this made the most sense.
- gloo/common/log.h (effectively renamed from logging.h) no longer needs
a .cc file, as it just passes GLOO_* macros through to the SPDLOG_*
macros.
- gloo has no init function which must be called first, and can be built
as a static library (precluding something like
__attribute__((constructor))). For this reason, all logger calls are
prefixed with a std::run_once which ensures the logger init/config is
performed before the first use of the logger.
- gloo/transport/tcp/debug_logger.{h,cc} are deleted, and replaced with
a custom fmt formatter implementation in gloo/transport/debug_data.h.
- Wherever lists of includes were touched as part of this change, they
were reformatted/fixed (e.g. X.h -> cX, alphabetical re-ordering,
changing gloo includes from <> to "").
jackbondpreston-arm
force-pushed
the
jack/spdlog-main
branch
from
cd3c648 to
3b85da0
Compare
|
Rebased + resolved conflict |
2 participants
Introduce spdlog for logging in Gloo.
Motivation
Implementation
Details of the implementation are included in the respective commit descriptions. There are 4 commits, building incrementally on top of eachother.
However, a key overview is: