Skip to content

Commit fd1b819

Browse files
shameekgangulyshameekganguly
authored
Add support for custom profiler (#682)
Allow custom profiler implementations to be used with gz-common profiler. Signed-off-by: Shameek Ganguly <[email protected]>
1 parent 0ac86c0 commit fd1b819

11 files changed

+266
-37
lines changed

‎MODULE.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module(
44
repo_name = "org_gazebosim_gz-common",
55
)
66

7+
bazel_dep(name = "bazel_skylib", version = "1.7.1")
78
bazel_dep(name = "assimp", version = "5.4.3")
89
bazel_dep(name = "buildifier_prebuilt", version = "7.3.1")
910
bazel_dep(name = "cdt", version = "1.4.0")

‎profiler/BUILD.bazel‎

Lines changed: 125 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
12
load("@rules_gazebo//gazebo:headers.bzl", "gz_configure_header", "gz_export_header")
23

34
package(
@@ -36,56 +37,122 @@ gz_configure_header(
3637
package_xml = "//:package.xml",
3738
)
3839

39-
public_headers_no_gen = [
40-
"include/gz/common/Profiler.hh",
41-
]
42-
43-
sources = [
44-
"src/Profiler.cc",
45-
"src/RemoteryProfilerImpl.cc",
46-
]
47-
4840
gz_export_header(
4941
name = "Export",
5042
out = "include/gz/common/profiler/Export.hh",
5143
export_base = "GZ_COMMON_PROFILER",
5244
lib_name = "gz-common-profiler",
5345
)
5446

47+
cc_library(
48+
name = "ProfilerImplInterface",
49+
hdrs = ["include/gz/common/ProfilerImpl.hh"],
50+
includes = ["include"],
51+
visibility = ["//visibility:public"],
52+
deps = [":Export"],
53+
)
54+
55+
cc_library(
56+
name = "RemoteryProfilerImpl",
57+
srcs = ["src/RemoteryProfilerImpl.cc"],
58+
hdrs = [
59+
"include/RemoteryConfig.h",
60+
"src/RemoteryProfilerImpl.hh",
61+
],
62+
includes = ["include"],
63+
deps = [
64+
":ProfilerImplInterface",
65+
"//:gz-common",
66+
"@remotery",
67+
],
68+
)
69+
70+
# Build flag to control how the Gz Profiler is configured.
71+
# --//profiler:config="disabled" (default): Profiler will be disabled.
72+
# --//profiler:config="remotery": Profiler will be enabled and the Remotery
73+
# profiler implemenation will be used.
74+
# --//profiler:config="custom": Profiler will be enabled and a custom profiler
75+
# implementation can be set to be used. See Profiler class for details.
76+
#
77+
# Note to maintainers: This setup is different from what is used in CMake where
78+
# the config is split into two parts to control whether Remotery is used or not
79+
# separately from whether the profiler is enabled or disabled.
80+
string_flag(
81+
name = "config",
82+
build_setting_default = "disabled",
83+
values = [
84+
"disabled",
85+
"remotery",
86+
"custom",
87+
],
88+
)
89+
90+
config_setting(
91+
name = "disabled",
92+
flag_values = {
93+
":config": "disabled",
94+
},
95+
)
96+
97+
config_setting(
98+
name = "use_remotery",
99+
flag_values = {
100+
":config": "remotery",
101+
},
102+
)
103+
104+
config_setting(
105+
name = "use_custom",
106+
flag_values = {
107+
":config": "custom",
108+
},
109+
)
110+
111+
public_headers_no_gen = ["include/gz/common/Profiler.hh"]
112+
55113
public_headers = public_headers_no_gen + [
56114
"include/gz/common/profiler/Export.hh",
57115
]
58116

59-
private_headers = [
60-
"src/ProfilerImpl.hh",
61-
"src/RemoteryProfilerImpl.hh",
62-
"include/RemoteryConfig.h",
63-
]
117+
sources = ["src/Profiler.cc"]
64118

65119
cc_library(
66120
name = "profiler",
67121
srcs = sources,
68-
hdrs = public_headers + private_headers,
69-
defines = [
70-
"GZ_PROFILER_ENABLE=1",
71-
"GZ_PROFILER_REMOTERY=1",
72-
],
122+
hdrs = public_headers,
123+
defines = select({
124+
"disabled": [
125+
"GZ_PROFILER_ENABLE=0",
126+
"GZ_PROFILER_REMOTERY=0",
127+
],
128+
"use_remotery": [
129+
"GZ_PROFILER_ENABLE=1",
130+
"GZ_PROFILER_REMOTERY=1",
131+
],
132+
"use_custom": [
133+
"GZ_PROFILER_ENABLE=1",
134+
"GZ_PROFILER_REMOTERY=0",
135+
],
136+
}),
73137
includes = ["include"],
74138
visibility = ["//visibility:public"],
75139
deps = [
140+
":ProfilerImplInterface",
76141
"//:gz-common",
77-
"@remotery",
78-
],
142+
] + select({
143+
"use_remotery": [":RemoteryProfilerImpl"],
144+
"//conditions:default": [],
145+
}),
79146
)
80147

81148
cc_test(
82149
name = "Profiler_Disabled_TEST",
83150
srcs = ["src/Profiler_Disabled_TEST.cc"],
84-
copts = ["-Wno-macro-redefined"],
85-
defines = [
86-
"GZ_PROFILER_ENABLE=0",
87-
"GZ_PROFILER_REMOTERY=0",
88-
],
151+
# This test is only compatible with --//profiler:config="disabled"
152+
defines = select({
153+
"disabled": [],
154+
"//conditions:default": ["BAZEL_SKIP_PROFILER_TEST=1"],
155+
}),
89156
deps = [
90157
":profiler",
91158
"@googletest//:gtest",
@@ -96,9 +163,41 @@ cc_test(
96163
cc_test(
97164
name = "Profiler_Remotery_TEST",
98165
srcs = ["src/Profiler_Remotery_TEST.cc"],
166+
# This test is only compatible with --//profiler:config="remotery"
167+
defines = select({
168+
"use_remotery": [],
169+
"//conditions:default": ["BAZEL_SKIP_PROFILER_TEST=1"],
170+
}),
171+
deps = [
172+
":profiler",
173+
"@googletest//:gtest",
174+
"@googletest//:gtest_main",
175+
],
176+
)
177+
178+
cc_test(
179+
name = "Profiler_Custom_TEST",
180+
srcs = ["src/Profiler_Custom_TEST.cc"],
181+
# This test is only compatible with --//profiler:config="custom"
182+
defines = select({
183+
"use_custom": [],
184+
"//conditions:default": ["BAZEL_SKIP_PROFILER_TEST=1"],
185+
}),
99186
deps = [
100187
":profiler",
101188
"@googletest//:gtest",
102189
"@googletest//:gtest_main",
103190
],
104191
)
192+
193+
cc_test(
194+
name = "Profiler_Error_TEST",
195+
srcs = ["src/Profiler_Error_TEST.cc"],
196+
deps = [
197+
":RemoteryProfilerImpl",
198+
":profiler",
199+
"@googletest//:gtest",
200+
"@googletest//:gtest_main",
201+
"@remotery",
202+
],
203+
)

‎profiler/include/gz/common/Profiler.hh‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@
2626
#include <gz/common/profiler/Export.hh>
2727
#include <gz/common/SingletonT.hh>
2828

29+
#include "ProfilerImpl.hh"
30+
2931
namespace gz
3032
{
3133
namespace common
3234
{
33-
class ProfilerImpl;
34-
3535
/// \brief Used to perform application-wide performance profiling
3636
///
3737
/// This class provides the necessary infrastructure for recording profiling
@@ -87,6 +87,14 @@ namespace gz
8787
/// \brief End a profiling sample.
8888
public: void EndSample();
8989

90+
/// \brief Set a profiler implementation.
91+
/// Takes ownership of the pointer if the call succeeds. This method will
92+
/// fail if a profiler implementation was previously set (i.e. if `Valid`
93+
/// returns `True`).
94+
/// \param[in] _impl Profiler implementation to set.
95+
/// \return True if the new profiler implementation was accepted.
96+
public: bool SetImplementation(std::unique_ptr<ProfilerImpl> _impl);
97+
9098
/// \brief Get the underlying profiler implentation name
9199
public: std::string ImplementationName() const;
92100

‎profiler/src/ProfilerImpl.hh‎ renamed to ‎profiler/include/gz/common/ProfilerImpl.hh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121
#include <cstdint>
2222
#include <string>
2323

24+
#include <gz/common/profiler/Export.hh>
25+
2426
namespace gz
2527
{
2628
namespace common
2729
{
2830
/// \brief Interface to be implemented by profiler implementations.
29-
class ProfilerImpl
31+
class GZ_COMMON_PROFILER_VISIBLE ProfilerImpl
3032
{
3133
/// \brief Constructor.
3234
public: ProfilerImpl() = default;

‎profiler/src/CMakeLists.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ if(GZ_PROFILER_REMOTERY)
6060
if(NOT WIN32)
6161
list(APPEND PROFILER_TESTS Profiler_Error_TEST.cc)
6262
endif()
63+
else() # GZ_PROFILER_REMOTERY
64+
list(APPEND PROFILER_TESTS Profiler_Custom_TEST.cc)
6365
endif()
6466

6567
gz_add_component(profiler SOURCES ${PROFILER_SRCS} GET_TARGET_NAME profiler_target)
@@ -100,6 +102,17 @@ if(TARGET UNIT_Profiler_Remotery_TEST)
100102
PUBLIC "GZ_PROFILER_ENABLE=1")
101103
endif()
102104

105+
if(TARGET UNIT_Profiler_Error_TEST)
106+
target_include_directories(UNIT_Profiler_Error_TEST
107+
PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/Remotery/lib>
108+
)
109+
endif()
110+
111+
if(TARGET UNIT_Profiler_Custom_TEST)
112+
target_compile_definitions(UNIT_Profiler_Custom_TEST
113+
PUBLIC "GZ_PROFILER_ENABLE=1")
114+
endif()
115+
103116
if(GZ_PROFILER_REMOTERY)
104117
set(GZ_PROFILER_SCRIPT_PATH ${CMAKE_INSTALL_LIBEXECDIR}/gz/gz-common${PROJECT_VERSION_MAJOR})
105118
set(GZ_PROFILER_VIS_PATH ${GZ_DATA_INSTALL_DIR}/profiler_vis)

‎profiler/src/Profiler.cc‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,10 @@
1515
*
1616
*/
1717
#include "gz/common/Profiler.hh" // NOLINT(*)
18+
#include "gz/common/ProfilerImpl.hh"
1819
#include "gz/common/Console.hh"
1920

20-
#include "ProfilerImpl.hh"
21-
22-
#ifdef GZ_PROFILER_REMOTERY
21+
#if GZ_PROFILER_REMOTERY
2322
#include "RemoteryProfilerImpl.hh"
2423
#endif // GZ_PROFILER_REMOTERY
2524

@@ -30,7 +29,7 @@ using namespace common;
3029
Profiler::Profiler():
3130
impl(nullptr)
3231
{
33-
#ifdef GZ_PROFILER_REMOTERY
32+
#if GZ_PROFILER_REMOTERY
3433
impl = new RemoteryProfilerImpl();
3534
#endif // GZ_PROFILER_REMOTERY
3635

@@ -96,3 +95,23 @@ bool Profiler::Valid() const
9695
{
9796
return this->impl != nullptr;
9897
}
98+
99+
//////////////////////////////////////////////////
100+
bool Profiler::SetImplementation(std::unique_ptr<ProfilerImpl> _impl)
101+
{
102+
if (_impl == nullptr)
103+
{
104+
gzwarn << "Setting an empty profiler implementation is not supported"
105+
<< std::endl;
106+
return false;
107+
}
108+
if (this->impl != nullptr)
109+
{
110+
gzwarn << "A profiler implementation named '" << this->impl->Name()
111+
<< "' is already in use. Cannot set a new one." << std::endl;
112+
return false;
113+
}
114+
this->impl = _impl.release();
115+
gzdbg << "Gazebo profiling with: " << this->impl->Name() << std::endl;
116+
return true;
117+
}

0 commit comments

Comments
 (0)