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.

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

Use pthread_setaffinity_np for setting affinity rather than sched_setaffinity #190

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

saikishor
Copy link
Member

@saikishor saikishor commented Nov 6, 2024

This PR aims to change the earlier approach with pthread_setaffinity_np as it is more generic and it can be scalable to all other controllers.

The good thing is it doesn't affect the recently merged changes : ros-controls/ros2_control#1852

I've also added corresponding tests in different places

@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.97%. Comparing base (45e93d7) to head (593dc27).

Files with missing lines Patch % Lines
src/realtime_helpers.cpp 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
+ Coverage   72.13%   72.97%   +0.84%     
==========================================
  Files           8        8              
  Lines         384      396      +12     
  Branches       64       65       +1     
==========================================
+ Hits          277      289      +12     
+ Misses         69       68       -1     
- Partials       38       39       +1     
Flag Coverage Δ
unittests 72.97% <90.90%> (+0.84%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
include/realtime_tools/async_function_handler.hpp 91.25% <ø> (ø)
src/realtime_helpers.cpp 37.34% <90.90%> (+10.58%) ⬆️

@saikishor
Copy link
Member Author

@firesurfer can you check this once?

Copy link
Contributor

@firesurfer firesurfer left a comment

Choose a reason for hiding this comment

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

In general this looks good to me with a minor notes:
https://en.cppreference.com/w/cpp/thread/thread/native_handle native_handle is only available for systems with a posix threading model. I am not sure if there might be some weird combination of systems (apart from windows) where it is not available and might lead to compilation errors

@saikishor
Copy link
Member Author

In general this looks good to me with a minor notes: https://en.cppreference.com/w/cpp/thread/thread/native_handle native_handle is only available for systems with a posix threading model. I am not sure if there might be some weird combination of systems (apart from windows) where it is not available and might lead to compilation errors

Most of the systems now supports. The following information, I got it using Copilot

Systems that support a POSIX threading model (also known as POSIX threads or Pthreads) include most Unix-like operating systems. POSIX threads are a standard for thread creation and synchronization, defined by the POSIX.1c standard (IEEE Std 1003.1c-1995). Here are some systems that support POSIX threads:

Unix-like Operating Systems

  • Linux: Most Linux distributions support POSIX threads natively. The GNU C Library (glibc) provides the implementation of POSIX threads on Linux.
  • macOS: macOS (formerly OS X) supports POSIX threads. The implementation is provided by the system's libc.
  • FreeBSD: FreeBSD supports POSIX threads through its native threading library.
  • NetBSD: NetBSD also supports POSIX threads.
  • OpenBSD: OpenBSD includes support for POSIX threads.
  • Solaris: Solaris (and its derivatives like OpenSolaris and illumos) supports POSIX threads.

Other Operating Systems

  • AIX: IBM's AIX operating system supports POSIX threads.
  • HP-UX: Hewlett-Packard's HP-UX supports POSIX threads.
  • QNX: QNX, a real-time operating system, supports POSIX threads.
  • VxWorks: VxWorks, another real-time operating system, supports POSIX threads.
  • Windows: While Windows does not natively support POSIX threads, there are libraries like Pthreads-w32 that provide a POSIX threads API on top of the native Windows threading model.

As this is the case, I think this should be good to go

Copy link
Contributor

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

This looks good and should give more flexibility ;-)

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM

@bmagyar bmagyar merged commit c0d6b2b into ros-controls:master Nov 8, 2024
22 of 26 checks passed
@saikishor saikishor deleted the change/way_of_affinity branch November 8, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants