Skip to content

Commit 6de956a

Browse files
shameekgangulyiche033
shameekganguly
authored and
iche033
committed
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 3a7fb0f commit 6de956a

File tree

10 files changed

+238
-60
lines changed

10 files changed

+238
-60
lines changed

‎profiler/BUILD.bazel‎

Lines changed: 97 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
2+
load(
3+
"@gz//bazel/lint:lint.bzl",
4+
"add_lint_tests",
5+
)
16
load(
27
"@gz//bazel/skylark:build_defs.bzl",
38
"GZ_FEATURES",
@@ -7,10 +12,6 @@ load(
712
"gz_export_header",
813
"gz_include_header",
914
)
10-
load(
11-
"@gz//bazel/lint:lint.bzl",
12-
"add_lint_tests",
13-
)
1415

1516
package(
1617
default_applicable_licenses = [GZ_ROOT + "common:license"],
@@ -46,22 +47,18 @@ cmake_configure_file(
4647
visibility = ["//visibility:private"],
4748
)
4849

49-
public_headers_no_gen = [
50-
"include/gz/common/Profiler.hh",
51-
]
52-
53-
sources = [
54-
"src/Profiler.cc",
55-
"src/RemoteryProfilerImpl.cc",
56-
]
57-
5850
gz_export_header(
5951
name = "include/gz/common/profiler/Export.hh",
6052
export_base = "GZ_COMMON_PROFILER",
6153
lib_name = "gz-common-profiler",
6254
visibility = ["//visibility:private"],
6355
)
6456

57+
public_headers_no_gen = [
58+
"include/gz/common/Profiler.hh",
59+
"include/gz/common/ProfilerImpl.hh",
60+
]
61+
6562
gz_include_header(
6663
name = "profiler_hh_genrule",
6764
out = "include/gz/common/profiler.hh",
@@ -70,17 +67,6 @@ gz_include_header(
7067
],
7168
)
7269

73-
public_headers = public_headers_no_gen + [
74-
"include/gz/common/profiler/Export.hh",
75-
"include/gz/common/profiler.hh",
76-
]
77-
78-
private_headers = [
79-
"src/ProfilerImpl.hh",
80-
"src/RemoteryProfilerImpl.hh",
81-
"include/RemoteryConfig.h",
82-
]
83-
8470
cc_library(
8571
name = "remotery",
8672
srcs = ["src/Remotery/lib/Remotery.c"],
@@ -89,44 +75,106 @@ cc_library(
8975
)
9076

9177
cc_library(
92-
name = "profiler",
93-
srcs = sources,
94-
hdrs = public_headers + private_headers,
95-
defines = [
96-
"GZ_PROFILER_ENABLE=1",
97-
"GZ_PROFILER_REMOTERY=1",
78+
name = "ProfilerImplInterface",
79+
hdrs = [
80+
"include/gz/common/ProfilerImpl.hh",
81+
"include/gz/common/profiler/Export.hh",
82+
],
83+
includes = ["include"],
84+
visibility = GZ_VISIBILITY,
85+
)
86+
87+
cc_library(
88+
name = "RemoteryProfilerImpl",
89+
srcs = ["src/RemoteryProfilerImpl.cc"],
90+
hdrs = [
91+
"include/RemoteryConfig.h",
92+
"src/RemoteryProfilerImpl.hh",
9893
],
9994
includes = ["include"],
10095
visibility = GZ_VISIBILITY,
10196
deps = [
10297
GZ_ROOT + "common",
98+
":ProfilerImplInterface",
10399
":remotery",
104100
],
105101
)
106102

107-
cc_test(
108-
name = "Profiler_Disabled_TEST",
109-
srcs = ["src/Profiler_Disabled_TEST.cc"],
110-
copts = ["-Wno-macro-redefined"],
111-
defines = [
112-
"GZ_PROFILER_ENABLE=0",
113-
"GZ_PROFILER_REMOTERY=0",
114-
],
115-
deps = [
116-
":profiler",
117-
"@gtest",
118-
"@gtest//:gtest_main",
103+
# Build flag to control how the Gz Profiler is configured.
104+
# --//profiler:config="disabled" (default): Profiler will be disabled.
105+
# --//profiler:config="remotery": Profiler will be enabled and the Remotery
106+
# profiler implemenation will be used.
107+
# --//profiler:config="custom": Profiler will be enabled and a custom profiler
108+
# implementation can be set to be used. See Profiler class for details.
109+
#
110+
# Note to maintainers: This setup is different from what is used in CMake where
111+
# the config is split into two parts to control whether Remotery is used or not
112+
# separately from whether the profiler is enabled or disabled.
113+
string_flag(
114+
name = "config",
115+
build_setting_default = "disabled",
116+
values = [
117+
"disabled",
118+
"remotery",
119+
"custom",
119120
],
120121
)
121122

122-
cc_test(
123-
name = "Profiler_Remotery_TEST",
124-
srcs = ["src/Profiler_Remotery_TEST.cc"],
123+
config_setting(
124+
name = "disabled",
125+
flag_values = {
126+
":config": "disabled",
127+
},
128+
)
129+
130+
config_setting(
131+
name = "use_remotery",
132+
flag_values = {
133+
":config": "remotery",
134+
},
135+
)
136+
137+
config_setting(
138+
name = "use_custom",
139+
flag_values = {
140+
":config": "custom",
141+
},
142+
)
143+
144+
public_headers = public_headers_no_gen + [
145+
"include/gz/common/profiler.hh",
146+
"include/gz/common/profiler/Export.hh",
147+
]
148+
149+
sources = ["src/Profiler.cc"]
150+
151+
cc_library(
152+
name = "profiler",
153+
srcs = sources,
154+
hdrs = public_headers,
155+
defines = select({
156+
"disabled": [
157+
"GZ_PROFILER_ENABLE=0",
158+
"GZ_PROFILER_REMOTERY=0",
159+
],
160+
"use_remotery": [
161+
"GZ_PROFILER_ENABLE=1",
162+
"GZ_PROFILER_REMOTERY=1",
163+
],
164+
"use_custom": [
165+
"GZ_PROFILER_ENABLE=1",
166+
"GZ_PROFILER_REMOTERY=0",
167+
],
168+
}),
169+
includes = ["include"],
170+
visibility = GZ_VISIBILITY,
125171
deps = [
126-
":profiler",
127-
"@gtest",
128-
"@gtest//:gtest_main",
129-
],
172+
GZ_ROOT + "common",
173+
":ProfilerImplInterface",
174+
] + select({
175+
"use_remotery": [":RemoteryProfilerImpl"],
176+
"//conditions:default": [],
177+
}),
130178
)
131179

132180
add_lint_tests()

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

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

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

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

‎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: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ if(GZ_PROFILER_REMOTERY)
6565

6666
list(APPEND PROFILER_SRCS ${Remotery_SRC} RemoteryProfilerImpl.cc)
6767
list(APPEND PROFILER_TESTS Profiler_Remotery_TEST.cc)
68+
69+
else() # GZ_PROFILER_REMOTERY
70+
list(APPEND PROFILER_TESTS Profiler_Custom_TEST.cc)
6871
endif()
6972

7073
gz_add_component(profiler SOURCES ${PROFILER_SRCS} GET_TARGET_NAME profiler_target)
@@ -105,6 +108,17 @@ if(TARGET UNIT_Profiler_Remotery_TEST)
105108
PUBLIC "GZ_PROFILER_ENABLE=1")
106109
endif()
107110

111+
if(TARGET UNIT_Profiler_Error_TEST)
112+
target_include_directories(UNIT_Profiler_Error_TEST
113+
PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/Remotery/lib>
114+
)
115+
endif()
116+
117+
if(TARGET UNIT_Profiler_Custom_TEST)
118+
target_compile_definitions(UNIT_Profiler_Custom_TEST
119+
PUBLIC "GZ_PROFILER_ENABLE=1")
120+
endif()
121+
108122
if(GZ_PROFILER_REMOTERY)
109123
set(GZ_PROFILER_SCRIPT_PATH ${CMAKE_INSTALL_LIBEXECDIR}/gz/gz-common${PROJECT_VERSION_MAJOR})
110124
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+
}

‎profiler/src/Profiler_Custom_TEST.cc‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Copyright (C) 2025 Open Source Robotics Foundation
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
*/
17+
18+
#include "gz/common/Profiler.hh" // NOLINT(*)
19+
#include <gtest/gtest.h> // NOLINT(*)
20+
21+
#include <atomic> // NOLINT(*)
22+
#include <thread> // NOLINT(*)
23+
#include "gz/common/Console.hh"
24+
#include "gz/common/ProfilerImpl.hh"
25+
#include "gz/common/Util.hh" // NOLINT(*)
26+
27+
using namespace gz;
28+
using namespace common;
29+
30+
class CustomProfilerImpl final: public ProfilerImpl
31+
{
32+
public: std::string Name() const final
33+
{
34+
return "test_profiler";
35+
}
36+
37+
public: void SetThreadName(const char *_name) final {}
38+
39+
public: void LogText(const char *_text) final {}
40+
41+
public: void BeginSample(const char *_name, uint32_t *_hash) final
42+
{
43+
beginSampleCallCount++;
44+
}
45+
46+
public: void EndSample() final
47+
{
48+
endSampleCallCount++;
49+
}
50+
51+
/// \brief Number of times `BeginSample` was called.
52+
public: uint64_t beginSampleCallCount = 0;
53+
54+
/// \brief Number of times `EndSample` was called.
55+
public: uint64_t endSampleCallCount = 0;
56+
};
57+
58+
/////////////////////////////////////////////////
59+
TEST(Profiler, CustomProfilerImpl)
60+
{
61+
#ifdef BAZEL_SKIP_PROFILER_TEST
62+
gzerr << "Test case is disabled for current bazel build config." << std::endl;
63+
#else
64+
EXPECT_TRUE(GZ_PROFILER_ENABLE);
65+
auto profiler = std::make_unique<CustomProfilerImpl>();
66+
auto* profilerRawPtr = profiler.get();
67+
Profiler::Instance()->SetImplementation(std::move(profiler));
68+
EXPECT_TRUE(GZ_PROFILER_VALID);
69+
EXPECT_EQ(Profiler::Instance()->ImplementationName(),
70+
"test_profiler");
71+
{
72+
GZ_PROFILE("Test");
73+
EXPECT_EQ(1, profilerRawPtr->beginSampleCallCount);
74+
}
75+
EXPECT_EQ(1, profilerRawPtr->endSampleCallCount);
76+
#endif
77+
}

0 commit comments

Comments
 (0)