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.

Sign up for GitHub

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

Jump to bottom

Add cmake command-line setting for spdlog logging level #6215

Merged
merged 5 commits into from
Sep 15, 2020
Merged

Add cmake command-line setting for spdlog logging level #6215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ecc9b4b
Add cmake command-line setting for spdlog logging level
jlowe Sep 11, 2020
210cb31
changelog
jlowe Sep 11, 2020
cec9653
Change cmake logging option to LOGGING_LEVEL
jlowe Sep 12, 2020
b34f7bd
Merge branch 'branch-0.16' into spdlog-level-flag
jlowe Sep 15, 2020
8c9877a
Change default RMM logging level to INFO
jlowe Sep 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
- PR #6197 Remove librmm dependency for libcudf
- PR #6209 Remove CXX11 ABI handling from CMake
- PR #6223 Remove CXX11 ABI flag from JNI build
- PR #6215 Add cmake command-line setting for spdlog logging level

## Bug Fixes

Expand Down
16 changes: 16 additions & 0 deletions cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,22 @@ if(HT_DEFAULT_ALLOCATOR)
set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS} -DHT_DEFAULT_ALLOCATOR")
endif(HT_DEFAULT_ALLOCATOR)

# Set a default logging level if none was specified
set(DEFAULT_LOGGING_LEVEL INFO)

if(NOT LOGGING_LEVEL)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way we can just inherit this from the RMM cmake?

Copy link
Member Author

Choose a reason for hiding this comment

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

cudf is currently picking up RMM via a conda package which does not ship any cmake files, so I don't know of a way to do this as RMM is located by libcudf today.

I think we'd either have to add a cmake snippet to the librmm conda package and have libcudf pick up the cmake settings from there, or we'd need to have libcudf pull RMM's source as it does Thrust/CUB today. However I don't know if avoiding the RMM conda package and pulling RMM source is something we want to do. @kkraus14

I suspect the RMM cmake source may also need to be refactored to make picking this up from other projects easier to do.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eventually the plan will be to export the cmake info in the librmm conda package so someone can do find_package(rmm) and get a nice target that allows inheriting options like these. We're still likely a bit away from that though.

message(STATUS "Setting logging level to '${DEFAULT_LOGGING_LEVEL}' since none specified.")
set(LOGGING_LEVEL "${DEFAULT_LOGGING_LEVEL}" CACHE
STRING "Choose the logging level." FORCE)
# Set the possible values of build type for cmake-gui
set_property(CACHE LOGGING_LEVEL PROPERTY STRINGS
"TRACE" "DEBUG" "INFO" "WARN" "ERROR" "CRITICAL" "OFF")
else()
message(STATUS "Setting logging level to '${LOGGING_LEVEL}'")
endif(NOT LOGGING_LEVEL)

target_compile_definitions(cudf PUBLIC SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${LOGGING_LEVEL})

###################################################################################################
# - link libraries --------------------------------------------------------------------------------

Expand Down
2 changes: 2 additions & 0 deletions java/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
<CMAKE_EXPORT_COMPILE_COMMANDS>OFF</CMAKE_EXPORT_COMPILE_COMMANDS>
<CUDA_STATIC_RUNTIME>OFF</CUDA_STATIC_RUNTIME>
<PER_THREAD_DEFAULT_STREAM>OFF</PER_THREAD_DEFAULT_STREAM>
<LOGGING_LEVEL>INFO</LOGGING_LEVEL>
<native.build.path>${project.build.directory}/cmake-build</native.build.path>
<slf4j.version>1.7.30</slf4j.version>
</properties>
Expand Down Expand Up @@ -347,6 +348,7 @@
<arg value="${basedir}/src/main/native"/>
<arg value="-DCUDA_STATIC_RUNTIME=${CUDA_STATIC_RUNTIME}" />
<arg value="-DPER_THREAD_DEFAULT_STREAM=${PER_THREAD_DEFAULT_STREAM}" />
<arg value="-DLOGGING_LEVEL=${LOGGING_LEVEL}" />
<arg value="-DCMAKE_CXX_FLAGS=${cxx.flags}"/>
<arg value="-DCMAKE_EXPORT_COMPILE_COMMANDS=${CMAKE_EXPORT_COMPILE_COMMANDS}"/>
</exec>
Expand Down
16 changes: 16 additions & 0 deletions java/src/main/native/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,22 @@ if(PER_THREAD_DEFAULT_STREAM)
add_compile_definitions(CUDA_API_PER_THREAD_DEFAULT_STREAM)
endif(PER_THREAD_DEFAULT_STREAM)

# Set a default logging level if none was specified
set(DEFAULT_LOGGING_LEVEL INFO)

if(NOT LOGGING_LEVEL)
message(STATUS "Setting logging level to '${DEFAULT_LOGGING_LEVEL}' since none specified.")
set(LOGGING_LEVEL "${DEFAULT_LOGGING_LEVEL}" CACHE
STRING "Choose the logging level." FORCE)
# Set the possible values of build type for cmake-gui
set_property(CACHE LOGGING_LEVEL PROPERTY STRINGS
"TRACE" "DEBUG" "INFO" "WARN" "ERROR" "CRITICAL" "OFF")
else()
message(STATUS "Setting logging level to '${LOGGING_LEVEL}'")
endif(NOT LOGGING_LEVEL)

target_compile_definitions(cudfjni PUBLIC SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_${LOGGING_LEVEL})

###################################################################################################
# - link libraries --------------------------------------------------------------------------------

Expand Down