Don't use boost::intrusive_ptr for thread_id_type #3120
Conversation
| @@ -34,11 +34,6 @@ | |||
|
|
|||
| namespace hpx { namespace threads | |||
| { | |||
| class HPX_EXPORT thread_data; | |||
There was a problem hiding this comment.
The HPX_EXPORT needs to go somewhere else, now, I think.
There was a problem hiding this comment.
it's in thread_id_type.hpp now.
There was a problem hiding this comment.
You are saying that this PR should fix all includes to not rely on thread_id_type being implicitly defined by some other include?
There was a problem hiding this comment.
Not necessarily. OTOH, we're trying to cleanup our header-include situation. So when introducing a new header file, perhaps it might make sense to introduce it properly... We could even add an inspect check enforcing this.
There was a problem hiding this comment.
Currently, the header exposing everything was thread_data_fwd.hpp, which still holds. Depending on how fine-grained we want to resolve those headers...
I'd do this in a separate PR though to not conflate those more or less unrelated changes with the header fixups.
| @@ -176,7 +176,7 @@ namespace hpx { namespace actions | |||
| : arguments_(std::forward<Ts>(vs)...), | |||
There was a problem hiding this comment.
This file now needs to #include thread_id_type.hpp
sithhell
force-pushed
the
thread_data_refcount
branch
from
79eeeb6
to
48e1453
Compare
| { | ||
| constexpr thread_id_type() | ||
| : thrd_(nullptr) {} | ||
| constexpr thread_id_type(thread_data* thrd) |
There was a problem hiding this comment.
We should make this constructor explicit to avoid unexpected conversions (we already have an implicit operator thread_data*() below).
sithhell
force-pushed
the
thread_data_refcount
branch
from
48e1453
to
9559988
Compare
|
Please fix the Windows and the CircleCI builds, then we can go ahead and merge this. |
sithhell
force-pushed
the
thread_data_refcount
branch
3 times, most recently
from
3f3143e
to
e0c1b07
Compare
This change gets rid of using boost::intrusive_ptr to refcount thread_data. The lifetime of thread_data is perfectly managed by the scheduling policy. That is, once a task terminates, it can be deleted/reused.
sithhell
force-pushed
the
thread_data_refcount
branch
from
e0c1b07
to
9d4fcca
Compare
|
The build errors should be fixed now. |
This change was neceessary after #3120 was merged. It was working with clang 5 and gcc 6 and up for some reason.
This change was neceessary after #3120 was merged. It was working with clang 5 and gcc 6 and up for some reason.
This change was neceessary after #3120 was merged. It was working with clang 5 and gcc 6 and up for some reason.
This change gets rid of using boost::intrusive_ptr to refcount thread_data. The
lifetime of thread_data is perfectly managed by the scheduling policy. That is,
once a task terminates, it can be deleted/reused.
Any background context you want to provide?
refcounting requires atomic operations which may needlessly (in this case) slow down an application.