Skip to content

Commit 2f49fa2

Browse files
committed
Remove lock out of critical path of creating/completing continuations
Signed-off-by: Joseph Schuchart <[email protected]>
1 parent 7dcfdfe commit 2f49fa2

File tree

1 file changed

+28
-36
lines changed

1 file changed

+28
-36
lines changed

ompi/mpiext/continue/c/continuation.c

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ static void ompi_cont_request_construct(ompi_cont_request_t* cont_req)
9292
OMPI_REQUEST_INIT(&cont_req->super, true);
9393
cont_req->super.req_type = OMPI_REQUEST_CONT;
9494
cont_req->super.req_complete = REQUEST_COMPLETED;
95-
cont_req->super.req_state = OMPI_REQUEST_INACTIVE;
95+
cont_req->super.req_state = OMPI_REQUEST_ACTIVE;
9696
cont_req->super.req_persistent = true;
9797
cont_req->super.req_free = &ompi_continue_request_free;
9898
cont_req->super.req_status = ompi_status_empty; /* always returns MPI_SUCCESS */
@@ -160,20 +160,22 @@ void ompi_continue_cont_release(ompi_continuation_t *cont)
160160
ompi_cont_request_t *cont_req = cont->cont_req;
161161
assert(OMPI_REQUEST_CONT == cont_req->super.req_type);
162162

163-
const bool using_threads = opal_using_threads();
164-
if (using_threads) {
165-
opal_atomic_lock(&cont_req->cont_lock);
166-
}
167-
int num_active = --cont_req->cont_num_active;
163+
int num_active = opal_atomic_add_fetch_32(&cont_req->cont_num_active, -1);
168164
assert(num_active >= 0);
169165
if (0 == num_active) {
170-
assert(!REQUEST_COMPLETE(&cont_req->super));
171-
opal_atomic_wmb();
172-
/* signal that all continuations were found complete */
173-
ompi_request_complete(&cont_req->super, true);
174-
}
175-
if (using_threads) {
176-
opal_atomic_unlock(&cont_req->cont_lock);
166+
const bool using_threads = opal_using_threads();
167+
if (using_threads) {
168+
opal_atomic_lock(&cont_req->cont_lock);
169+
}
170+
/* double check that no other thread has completed or restarted the request already */
171+
if (0 == cont_req->cont_num_active && !REQUEST_COMPLETE(&cont_req->super)) {
172+
opal_atomic_wmb();
173+
/* signal that all continuations were found complete */
174+
ompi_request_complete(&cont_req->super, true);
175+
}
176+
if (using_threads) {
177+
opal_atomic_unlock(&cont_req->cont_lock);
178+
}
177179
}
178180
OBJ_RELEASE(cont_req);
179181

@@ -434,19 +436,19 @@ ompi_continuation_t *ompi_continue_cont_create(
434436
/* signal that the continuation request has a new continuation */
435437
OBJ_RETAIN(cont_req);
436438

437-
const bool using_threads = opal_using_threads();
438-
if (using_threads) {
439-
opal_atomic_lock(&cont_req->cont_lock);
440-
}
441-
int32_t num_active = cont_req->cont_num_active++;
439+
int32_t num_active = opal_atomic_add_fetch_32(&cont_req->cont_num_active, 1);
442440
if (num_active == 0) {
443-
/* (re)activate the continuation request upon first registration */
444-
assert(REQUEST_COMPLETE(&cont_req->super));
445-
cont_req->super.req_complete = REQUEST_PENDING;
446-
cont_req->super.req_state = OMPI_REQUEST_ACTIVE;
447-
}
448-
if (using_threads) {
449-
opal_atomic_unlock(&cont_req->cont_lock);
441+
const bool using_threads = opal_using_threads();
442+
if (using_threads) {
443+
opal_atomic_lock(&cont_req->cont_lock);
444+
}
445+
if (0 != cont_req->cont_num_active && REQUEST_COMPLETE(&cont_req->super)) {
446+
/* (re)activate the continuation request upon first registration */
447+
cont_req->super.req_complete = REQUEST_PENDING;
448+
}
449+
if (using_threads) {
450+
opal_atomic_unlock(&cont_req->cont_lock);
451+
}
450452
}
451453

452454
return cont;
@@ -477,15 +479,7 @@ static int request_completion_cb(ompi_request_t *request)
477479

478480
/* inactivate / free the request */
479481
if (request->req_persistent) {
480-
if (OMPI_REQUEST_CONT == request->req_type && opal_using_threads()) {
481-
/* handle with care: another thread may register a new continuation already */
482-
ompi_cont_request_t *cont_req = cont->cont_req;
483-
opal_atomic_lock(&cont_req->cont_lock);
484-
if (cont_req->cont_num_active == 0) {
485-
cont_req->super.req_state = OMPI_REQUEST_INACTIVE;
486-
}
487-
opal_atomic_unlock(&cont_req->cont_lock);
488-
} else {
482+
if (OMPI_REQUEST_CONT != request->req_type) {
489483
request->req_state = OMPI_REQUEST_INACTIVE;
490484
}
491485
} else {
@@ -549,8 +543,6 @@ int ompi_continue_attach(
549543

550544
req_cont_data->cont_obj = cont;
551545

552-
assert(request->req_state == OMPI_REQUEST_ACTIVE || request->req_state == OMPI_REQUEST_INACTIVE);
553-
554546
ompi_request_set_callback(request, &request_completion_cb, req_cont_data);
555547
++num_registered;
556548

0 commit comments

Comments
 (0)