Skip to content

Commit 7832901

Browse files
Mario-DLMario-DL
authored
Improve ThreadSettingsQoS logging (#4744)
* Refs #20862: Add BB tests Signed-off-by: Mario Dominguez <[email protected]> * Refs #20862: ThreadSettings logging improvements impl Signed-off-by: Mario Dominguez <[email protected]> * Refs #20862: Linter Signed-off-by: Mario Dominguez <[email protected]> * Refs 20862: Apply Miguel suggestions Signed-off-by: Mario Dominguez <[email protected]> * Refs 20862: Apply second rev Signed-off-by: Mario Dominguez <[email protected]> * Refs 20862: Applied third round suggestions Signed-off-by: Mario Dominguez <[email protected]> * Refs 20862: Remove old method in threading_empty Signed-off-by: Mario Dominguez <[email protected]> --------- Signed-off-by: Mario Dominguez <[email protected]>
1 parent 9928d1d commit 7832901

File tree

9 files changed

+266
-84
lines changed

9 files changed

+266
-84
lines changed

‎src/cpp/rtps/transport/TCPTransportInterface.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,9 +1098,6 @@ void TCPTransportInterface::perform_listen_operation(
10981098
{
10991099
remote_locator = remote_endpoint_to_locator(channel);
11001100

1101-
uint32_t port = channel->local_endpoint().port();
1102-
set_name_to_current_thread("dds.tcp.%u", port);
1103-
11041101
if (channel->tcp_connection_type() == TCPChannelResource::TCPConnectionType::TCP_CONNECT_TYPE)
11051102
{
11061103
rtcp_message_manager->sendConnectionRequest(channel);

‎src/cpp/utils/threading.hpp

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#ifndef UTILS__THREADING_HPP_
1616
#define UTILS__THREADING_HPP_
1717

18+
#include <array>
19+
1820
#include "./thread.hpp"
1921

2022
namespace eprosima {
@@ -29,48 +31,55 @@ struct ThreadSettings;
2931
/**
3032
* @brief Give a name to the thread calling this function.
3133
*
32-
* @param[in] name A null-terminated string with the name to give to the calling thread.
33-
* The implementation for certain platforms may truncate the final thread
34-
* name if there is a limit on the length of the name of a thread.
34+
* @param[in, out] thread_name_buffer Buffer to store the name of the thread.
35+
* @param[in] name A null-terminated string with the name to give to the calling thread.
36+
* The implementation for certain platforms may truncate the final thread
37+
* name if there is a limit on the length of the name of a thread.
3538
*/
3639
void set_name_to_current_thread(
40+
std::array<char, 16>& thread_name_buffer,
3741
const char* name);
3842

3943
/**
4044
* @brief Give a name to the thread calling this function.
4145
*
42-
* @param[in] fmt A null-terminated string to be used as the format argument of
43-
* a `snprintf` like function, in order to accomodate the restrictions of
44-
* the OS. Those restrictions may truncate the final thread name if there
45-
* is a limit on the length of the name of a thread.
46-
* @param[in] arg Single variadic argument passed to the formatting function.
46+
* @param[in, out] thread_name_buffer Buffer to store the name of the thread.
47+
* @param[in] fmt A null-terminated string to be used as the format argument of
48+
* a `snprintf` like function, in order to accomodate the restrictions of
49+
* the OS. Those restrictions may truncate the final thread name if there
50+
* is a limit on the length of the name of a thread.
51+
* @param[in] arg Single variadic argument passed to the formatting function.
4752
*/
4853
void set_name_to_current_thread(
54+
std::array<char, 16>& thread_name_buffer,
4955
const char* fmt,
5056
uint32_t arg);
5157

5258
/**
5359
* @brief Give a name to the thread calling this function.
5460
*
55-
* @param[in] fmt A null-terminated string to be used as the format argument of
56-
* a `snprintf` like function, in order to accomodate the restrictions of
57-
* the OS. Those restrictions may truncate the final thread name if there
58-
* is a limit on the length of the name of a thread.
59-
* @param[in] arg1 First variadic argument passed to the formatting function.
60-
* @param[in] arg2 Second variadic argument passed to the formatting function.
61+
* @param[in, out] thread_name_buffer Buffer to store the name of the thread.
62+
* @param[in] fmt A null-terminated string to be used as the format argument of
63+
* a `snprintf` like function, in order to accomodate the restrictions of
64+
* the OS. Those restrictions may truncate the final thread name if there
65+
* is a limit on the length of the name of a thread.
66+
* @param[in] arg1 First variadic argument passed to the formatting function.
67+
* @param[in] arg2 Second variadic argument passed to the formatting function.
6168
*/
6269
void set_name_to_current_thread(
70+
std::array<char, 16>& thread_name_buffer,
6371
const char* fmt,
6472
uint32_t arg1,
6573
uint32_t arg2);
6674

67-
6875
/**
6976
* @brief Apply thread settings to the thread calling this function.
7077
*
78+
* @param[in] thread_name Name of the thread.
7179
* @param[in] settings Thread settings to apply.
7280
*/
7381
void apply_thread_settings_to_current_thread(
82+
const char* thread_name,
7483
const fastdds::rtps::ThreadSettings& settings);
7584

7685
/**
@@ -94,8 +103,9 @@ eprosima::thread create_thread(
94103
{
95104
return eprosima::thread(settings.stack_size, [=]()
96105
{
97-
apply_thread_settings_to_current_thread(settings);
98-
set_name_to_current_thread(name, args ...);
106+
std::array<char, 16> thread_name_buffer;
107+
set_name_to_current_thread(thread_name_buffer, name, args ...);
108+
apply_thread_settings_to_current_thread(thread_name_buffer.data(), settings);
99109
func();
100110
});
101111
}

‎src/cpp/utils/threading/threading_empty.ipp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,34 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <array>
16+
#include <cstdint>
17+
1518
namespace eprosima {
1619

1720
void set_name_to_current_thread(
21+
std::array<char, 16>& /* thread_name_buffer */,
1822
const char* /* name */)
1923
{
2024
}
2125

2226
void set_name_to_current_thread(
27+
std::array<char, 16>& /* thread_name_buffer */,
2328
const char* /* fmt */,
2429
uint32_t /* arg */)
2530
{
2631
}
2732

2833
void set_name_to_current_thread(
34+
std::array<char, 16>& /* thread_name_buffer */,
2935
const char* /* fmt */,
3036
uint32_t /* arg1 */,
3137
uint32_t /* arg2 */)
3238
{
3339
}
3440

3541
void apply_thread_settings_to_current_thread(
42+
const char* /* thread_name */,
3643
const fastdds::rtps::ThreadSettings& /*settings*/)
3744
{
3845
}

‎src/cpp/utils/threading/threading_osx.ipp

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <array>
1516
#include <cstdio>
1617
#include <cstring>
1718
#include <limits>
@@ -24,37 +25,42 @@
2425

2526
namespace eprosima {
2627

27-
template<typename... Args>
28+
template<typename ... Args>
2829
static void set_name_to_current_thread_impl(
29-
const char* fmt, Args... args)
30+
std::array<char, 16>& thread_name_buffer,
31+
const char* fmt,
32+
Args... args)
3033
{
31-
char thread_name[16]{};
32-
snprintf(thread_name, 16, fmt, args...);
33-
pthread_setname_np(thread_name);
34+
snprintf(thread_name_buffer.data(), 16, fmt, args ...);
35+
pthread_setname_np(thread_name_buffer.data());
3436
}
3537

3638
void set_name_to_current_thread(
39+
std::array<char, 16>& thread_name_buffer,
3740
const char* name)
3841
{
39-
set_name_to_current_thread_impl("%s", name);
42+
set_name_to_current_thread_impl(thread_name_buffer, "%s", name);
4043
}
4144

4245
void set_name_to_current_thread(
46+
std::array<char, 16>& thread_name_buffer,
4347
const char* fmt,
4448
uint32_t arg)
4549
{
46-
set_name_to_current_thread_impl(fmt, arg);
50+
set_name_to_current_thread_impl(thread_name_buffer, fmt, arg);
4751
}
4852

4953
void set_name_to_current_thread(
54+
std::array<char, 16>& thread_name_buffer,
5055
const char* fmt,
5156
uint32_t arg1,
5257
uint32_t arg2)
5358
{
54-
set_name_to_current_thread_impl(fmt, arg1, arg2);
59+
set_name_to_current_thread_impl(thread_name_buffer, fmt, arg1, arg2);
5560
}
5661

5762
static void configure_current_thread_scheduler(
63+
const char* thread_name,
5864
int sched_class,
5965
int sched_priority)
6066
{
@@ -64,67 +70,86 @@ static void configure_current_thread_scheduler(
6470
int current_class;
6571
int result = 0;
6672
bool change_priority = (std::numeric_limits<int32_t>::min() != sched_priority);
67-
73+
6874
// Get current scheduling parameters
6975
memset(&current_param, 0, sizeof(current_param));
7076
pthread_getschedparam(self_tid, &current_class, &current_param);
71-
77+
7278
memset(&param, 0, sizeof(param));
7379
param.sched_priority = 0;
7480
sched_class = (sched_class == -1) ? current_class : sched_class;
75-
81+
7682
//
77-
// Set Scheduler Class and Priority
83+
// Set Scheduler Class and Priority
7884
//
79-
80-
if(sched_class == SCHED_OTHER)
81-
{
82-
85+
86+
if (sched_class == SCHED_OTHER)
87+
{
88+
8389
//
8490
// Sched OTHER has a nice value, that we pull from the priority parameter.
8591
// - Requires priorty value to be zero (0).
86-
//
92+
//
8793
result = pthread_setschedparam(self_tid, sched_class, &param);
88-
if(0 == result && change_priority)
94+
if (0 == result && change_priority)
8995
{
9096
uint64_t tid;
9197
pthread_threadid_np(NULL, &tid);
9298
result = setpriority(PRIO_PROCESS, tid, sched_priority);
93-
}
99+
if (0 != result)
100+
{
101+
EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set priority of thread with id [" << tid << "," << thread_name << "] to value " << sched_priority << ". Error '" << strerror(
102+
result) << "'");
103+
}
104+
}
105+
else if (0 != result)
106+
{
107+
EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << ". Error '" << strerror(
108+
result) << "'");
109+
}
94110
}
95-
else if((sched_class == SCHED_FIFO) ||
111+
else if ((sched_class == SCHED_FIFO) ||
96112
(sched_class == SCHED_RR))
97113
{
98114
//
99115
// RT Policies use a different priority numberspace.
100116
//
101-
102117
param.sched_priority = change_priority ? sched_priority : current_param.sched_priority;
103118
result = pthread_setschedparam(self_tid, sched_class, &param);
104-
}
105-
106-
if (0 != result)
107-
{
108-
EPROSIMA_LOG_ERROR(SYSTEM, "Error '" << strerror(result) << "' configuring scheduler for thread " << self_tid);
119+
if (0 != result)
120+
{
121+
EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set scheduler of thread with id [" << self_tid << "," << thread_name << "] to value " << sched_class << " with priority " << param.sched_priority << ". Error '" << strerror(
122+
result) << "'");
123+
}
109124
}
110125
}
111126

112127
static void configure_current_thread_affinity(
128+
const char* thread_name,
113129
uint64_t affinity)
114130
{
115131
if (affinity <= static_cast<uint64_t>(std::numeric_limits<integer_t>::max()))
116132
{
133+
int result = 0;
117134
thread_affinity_policy_data_t policy = { static_cast<integer_t>(affinity) };
118135
pthread_t self_tid = pthread_self();
119-
thread_policy_set(pthread_mach_thread_np(self_tid), THREAD_AFFINITY_POLICY, (thread_policy_t)&policy, 1);
136+
result =
137+
thread_policy_set(pthread_mach_thread_np(self_tid), THREAD_AFFINITY_POLICY, (thread_policy_t)&policy,
138+
1);
139+
if (0 != result)
140+
{
141+
EPROSIMA_LOG_ERROR(SYSTEM, "Problem to set affinity of thread with id [" << self_tid << "," << thread_name << "] to value " << affinity << ". Error '" << strerror(
142+
result) << "'");
143+
}
120144
}
121145
}
122146

123147
void apply_thread_settings_to_current_thread(
148+
const char* thread_name,
124149
const fastdds::rtps::ThreadSettings& settings)
125150
{
126-
configure_current_thread_scheduler(settings.scheduling_policy, settings.priority);
127-
configure_current_thread_affinity(settings.affinity);
151+
configure_current_thread_scheduler(thread_name, settings.scheduling_policy, settings.priority);
152+
configure_current_thread_affinity(thread_name, settings.affinity);
128153
}
129154

130155
} // namespace eprosima

0 commit comments

Comments
 (0)