refactoring transport benchmarks for new UCX #10
refactoring transport benchmarks for new UCX #10angainor wants to merge 8 commits intoboeschf:ucxfrom
Conversation
…ed: request might be already invalid
… destruction order needs to be assured.
| m_db.init(m_worker.address()); | ||
| } | ||
|
|
||
| ~transport_context() |
There was a problem hiding this comment.
@boeschf We must destroy the ucp workers before the context, otherwise UCX segfaults. So order of destructors is important. I added a simple loop here, but you might want to do it in some other way.
There was a problem hiding this comment.
this is solved now (I forgot the set the m_moved flag to false)
|
|
||
| ucp_worker_handle& operator=(ucp_worker_handle&& other) noexcept | ||
| { | ||
| destroy(); |
There was a problem hiding this comment.
did not really understand why this was here, and why you mark m_moved and then conditionally call ucp_worker_destroy(). The problem is that the workers were always moved, and somehow the destroy() never worked at the end of the program.
There was a problem hiding this comment.
this is ok here (in case we're move assigning a new worker to an existing worker, the existing worker should be destroyed)
| return m_req->m_worker->m_parallel_context->thread_primitives().critical( | ||
| [this]() | ||
| { | ||
| if if (!(*m_req->m_completed)) |
There was a problem hiding this comment.
@boeschf we cannot reference m_req here, because it might have been freed already.
There was a problem hiding this comment.
yes we can, we check earlier and bail out
2 participants
Most changes are cosmetic, but some commits are important. Will comment on those separately.