Skip to content

Commit de84219

Browse files
author
Bernard Knueven
committed
remove locks
1 parent c29e0ec commit de84219

2 files changed

Lines changed: 43 additions & 24 deletions

File tree

mpisppy/cylinders/spwindow.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,12 @@ def __init__(self, my_fields: dict, strata_comm: MPI.Comm, field_order=None):
282282
# Gather layouts across ranks
283283
self.strata_buffer_layouts = strata_comm.allgather(self.buffer_layout)
284284

285+
# Keep one passive-target access epoch open for the lifetime of the
286+
# window. Individual operations complete at their target with Flush;
287+
# they do not repeatedly acquire and release target locks.
288+
self.window.Lock_all()
289+
self._epoch_open = True
290+
285291
def free(self):
286292
if self.window is not None:
287293
guard_error = None
@@ -290,6 +296,9 @@ def free(self):
290296
self._verify_window_guards(field, "free", "before")
291297
except RuntimeError as error:
292298
guard_error = error
299+
if self._epoch_open:
300+
self.window.Unlock_all()
301+
self._epoch_open = False
293302
self.window.Free()
294303
self.buff = None
295304
self.buffer_layout = None
@@ -465,18 +474,16 @@ def get(self, dest: nptyping.ArrayLike, strata_rank: int, field: Field,
465474
target_rank=strata_rank,
466475
)
467476
window = self.window
468-
# The shared epoch may overlap other readers, but it cannot overlap
469-
# the publisher's exclusive self-target epoch below. Unlock provides
470-
# completion for the Get before its destination and guards are read.
471-
window.Lock(strata_rank, MPI.LOCK_SHARED)
472477
window.Get(
473478
(transfer, field_layout.transfer_nbytes, MPI.BYTE),
474479
strata_rank,
475480
(field_layout.transfer_offset_bytes,
476481
field_layout.transfer_nbytes,
477482
MPI.BYTE),
478483
)
479-
window.Unlock(strata_rank)
484+
# Flush provides local completion, so the destination and its
485+
# transmitted guards are safe to inspect below.
486+
window.Flush(strata_rank)
480487
self._verify_transfer_guards(
481488
storage, field_layout, field, "get", "after",
482489
target_rank=strata_rank,
@@ -502,19 +509,18 @@ def put(self, values: nptyping.ArrayLike, field: Field):
502509
)
503510
self._verify_window_guards(field, "put", "before")
504511
window = self.window
505-
# Publishing through a self-target Put is intentional: unlike an
506-
# ordinary local store, this exclusive epoch participates in the same
507-
# MPI lock arbitration as remote readers and makes the field snapshot
508-
# indivisible with respect to their shared epochs.
509-
window.Lock(self.strata_rank, MPI.LOCK_EXCLUSIVE)
512+
# Publishing through a self-target Put is intentional: it keeps local
513+
# and remote publication on the same MPI-visible path. Phase 3 will
514+
# add the metadata protocol that lets readers reject an overlapping
515+
# publication; Phase 2 only changes epoch management.
510516
window.Put(
511517
(transfer, field_layout.transfer_nbytes, MPI.BYTE),
512518
self.strata_rank,
513519
(field_layout.transfer_offset_bytes,
514520
field_layout.transfer_nbytes,
515521
MPI.BYTE),
516522
)
517-
window.Unlock(self.strata_rank)
523+
window.Flush(self.strata_rank)
518524
self._verify_transfer_guards(
519525
storage, field_layout, field, "put", "after",
520526
target_rank=self.strata_rank,

mpisppy/tests/test_spwindow_partial_get.py

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,6 @@ def __init__(self, wrapped):
2828
self.wrapped = wrapped
2929
self.calls = []
3030

31-
def Lock(self, rank, lock_type):
32-
self.calls.append(("Lock", rank, lock_type))
33-
return self.wrapped.Lock(rank, lock_type)
34-
35-
def Unlock(self, rank):
36-
self.calls.append(("Unlock", rank))
37-
return self.wrapped.Unlock(rank)
38-
3931
def Put(self, *args):
4032
self.calls.append(("Put",))
4133
return self.wrapped.Put(*args)
@@ -44,6 +36,18 @@ def Get(self, *args):
4436
self.calls.append(("Get",))
4537
return self.wrapped.Get(*args)
4638

39+
def Flush(self, rank):
40+
self.calls.append(("Flush", rank))
41+
return self.wrapped.Flush(rank)
42+
43+
def Unlock_all(self):
44+
self.calls.append(("Unlock_all",))
45+
return self.wrapped.Unlock_all()
46+
47+
def Free(self):
48+
self.calls.append(("Free",))
49+
return self.wrapped.Free()
50+
4751
def __getattr__(self, name):
4852
return getattr(self.wrapped, name)
4953

@@ -158,7 +162,7 @@ def test_transmitted_canaries_identify_record(self):
158162
0, Field.NONANTS_VALS, "left", layout.padded_len + 1),
159163
)
160164

161-
def test_put_and_get_preserve_exclusive_shared_locking(self):
165+
def test_put_and_get_flush_persistent_epoch(self):
162166
recording = _RecordingWindow(self.win.window)
163167
self.win.window = recording
164168

@@ -167,12 +171,21 @@ def test_put_and_get_preserve_exclusive_shared_locking(self):
167171
np.empty(self.padded, dtype="d"), 0, Field.NONANTS_VALS)
168172

169173
self.assertEqual(recording.calls, [
170-
("Lock", 0, MPI.LOCK_EXCLUSIVE),
171174
("Put",),
172-
("Unlock", 0),
173-
("Lock", 0, MPI.LOCK_SHARED),
175+
("Flush", 0),
174176
("Get",),
175-
("Unlock", 0),
177+
("Flush", 0),
178+
])
179+
180+
def test_free_closes_epoch_before_freeing_window(self):
181+
recording = _RecordingWindow(self.win.window)
182+
self.win.window = recording
183+
184+
self.win.free()
185+
186+
self.assertEqual(recording.calls, [
187+
("Unlock_all",),
188+
("Free",),
176189
])
177190

178191

0 commit comments

Comments
 (0)