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

Allow list initialization of thread::attributes #70

Open
LnnrtS opened this issue Jan 19, 2021 · 3 comments
Open

Allow list initialization of thread::attributes #70

LnnrtS opened this issue Jan 19, 2021 · 3 comments

Comments

@LnnrtS
Copy link
Contributor

LnnrtS commented Jan 19, 2021

I would like do do something like this

thread myThread(someFunc, nullptr, {4000, thread::priority::high});

Since there is only a default constructor for thread::attributes I have do do this

thread::attributes attr;
attr.th_stack_size_bytes = 4000;
attr.th_priority = thread::priority::normal;
thread myThread(someFunc, nullptr, attr);

A practical use case is having thread as member of a class with the need to initialize via the member initializer list.
This doesn't work:

class MyClass
{
public:
  MyClass() : myThread(someFunc, nullptr, {4000, thread::priority::high})
  {
  }
  ~MyClass()
  {
    // ..stop/join..
  }
private:
  thread myThread;
}

so I have to use this workaround

class MyClass
{
public:
  MyClass()
  {
    thread::attributes attr;
    attr.th_stack_size_bytes = 4000;
    attr.th_priority = thread::priority::normal;
    new (&threadStorage) thread(someFunc, nullptr, attr);
    threadPtr = (thread*) &threadStorage;
  }
  ~MyClass()
  {
    // ..stop/join..
   threadPtr->~thread();
  }
private:
  std::aligned_storage_t<sizeof(thread),alignof(thread)> threadStorage;
  thread* threadPtr;
}
@LnnrtS LnnrtS changed the title Allow list initialization of thread::attributes from initializer list Allow list initialization of thread::attributes Jan 19, 2021
@ilg-ul
Copy link
Contributor

ilg-ul commented Jan 19, 2021

Using a separate attributes structure to initialise various objects is a POSIX trick in C to avoid changing the API when adding new members to the structure, and for those using the C API probably there is not much to do.

However, in C++ life is a bit better, and we have the luxury of multiple constructors, so I think that your proposal makes sense, and not only for threads, but possibly for other objects that use attributes.

The only thing that we have to be very careful when adding the new constructors is to leave space for incremental updates, which, as far as I can tell at first sight, probably means not using defaults, but defining separate constructors with 1, 2, 3, ... parameters.

Could you study if such a solution is realistic?

@LnnrtS
Copy link
Contributor Author

LnnrtS commented Jan 20, 2021

That seems to be the most straightforward way

class attributes
{
  // allocated stack with default size and default priority or non-default priority
  // forward to second constructor
  attributes(priority_t = priority::normal);
  
  // allocated stack and default priority or non-default priority
  attributes(std::size_t, priority_t = priority::normal);
  
  // provided stack and default priority or non-default priority
  attributes(void*, std::size_t, priority_t = priority::normal);

private:
  void* th_stack_address;
  std::size_t  th_stack_size;
  priority_t th_priority;
}

Some constructors needs to be marked as explicit and/or scoped enums need to be used in order to prevent direct conversion from integer types: attributes myAttr = 5;

When looking at the code I thought it might also be helpful to

  • separate stack properties from priority
  • use more descriptive types for the stack properties (std::span (C++20) or at least some pointer + length struct)

@ilg-ul
Copy link
Contributor

ilg-ul commented Jan 20, 2021

I guess it might eventually work, but I'm not convinced that using defaults is a good idea.

separate stack properties from priority

What exactly do you mean by this? I have to read the C++20 specs for std::span.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants