Skip to content

Commit

Permalink
feat(cpp-client): Add GetTidAsString as a utility function (deephaven…
Browse files Browse the repository at this point in the history
…#5843)

Add `GetTidAsString()` as a utility function.

Note that the previous version of this code was `inline`. However, on
the Windows platform, making it inline involves pulling in some windows
includes including `windows.h`. The problem with including `windows.h`
is that it defines some macros that might conflict. In particular it
defines the `min` and `max` macros that definitely conflict.

In order to free ourselves and our Windows customers from seeing the
conflicts that might arise, we isolate the Windows-specific code in a
.cc file rather than including `windows.h` in a .h file.

There are other approaches that would work too, such as defining
`NOMINMAX` or changing all of our calls that look like `min()` and
`max()` to have extra parentheses: `(min)()` and `(max)()`.

However in my opinion, isolating this Windows-specific code in one .cc
file was simplest for now.
  • Loading branch information
kosak authored Jul 24, 2024
1 parent 8d2947c commit 8a7329e
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 0 deletions.
1 change: 1 addition & 0 deletions cpp-client/deephaven/dhcore/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ set(ALL_FILES
src/ticking/ticking.cc
src/utility/cython_support.cc
src/utility/utility.cc
src/utility/utility_platform_specific.cc

include/private/deephaven/dhcore/ticking/immer_table_state.h
include/private/deephaven/dhcore/ticking/index_decoder.h
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,12 @@ TimePointToStr(
*/
std::string Basename(std::string_view path);

/**
* Returns the current thread ID as a string.
* @return The current thread ID as a string.
*/
[[nodiscard]] std::string GetTidAsString();

template <class T> [[nodiscard]] std::string
TypeName(const T& t) {
return demangle(typeid(t).name());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
*/
#include <string>
#include "deephaven/dhcore/utility/utility.h"

// for GetTidAsString
#if defined(__linux__)
#include <unistd.h>
#include <sys/syscall.h>
#elif defined(_WIN32)
#include <windows.h>
#endif

namespace deephaven::dhcore::utility {
[[nodiscard]] std::string GetTidAsString() {
#if defined(__linux__)
const pid_t tid = syscall(__NR_gettid); // this is more portable than gettid().
return std::to_string(tid);
#elif defined(_WIN32)
auto tid = GetCurrentThreadId();
return std::to_string(tid);
#else
#error "Don't have a way to getting thread id on your platform"
#endif
}
} // namespace deephaven::dhcore::utility
10 changes: 10 additions & 0 deletions cpp-client/deephaven/tests/src/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using deephaven::dhcore::utility::Base64Encode;
using deephaven::dhcore::utility::Basename;
using deephaven::dhcore::utility::EpochMillisToStr;
using deephaven::dhcore::utility::GetTidAsString;
using deephaven::dhcore::utility::ObjectId;

namespace deephaven::client::tests {
Expand Down Expand Up @@ -41,4 +42,13 @@ TEST_CASE("Basename", "[utility]") {
CHECK(Basename(R"(C:\Users\kosak\)").empty());
#endif
}

// This isn't much of a test, but if it can compile on all supported
// platforms (Linux and Windows) then that is at least a sanity check
// (that the entry point exists). For now we just visuallyi spot-check
// that ireturns the right value.
TEST_CASE("ThreadId", "[utility]") {
auto tid = GetTidAsString();
fmt::println("This should be my thread id: {}", tid);
}
} // namespace deephaven::client::tests

0 comments on commit 8a7329e

Please sign in to comment.