Remove shared memory from MPFuture, fix minor bugs #317

justheuristic · 2021-07-13T10:51:04Z

re-written MPFuture to use pipe-only communication instead of SharedMemory
- rationale: each shared memory object is a file, using thousands of them floods the open files limit
- one alternative was to group multple shared memory objects into one file with different offsets, but that would require heavier engineering
fixed coroutine set_event_threadsafe is never awaited

# Conflicts: # hivemind/moe/server/task_pool.py # hivemind/utils/mpfuture.py # tests/test_util_modules.py

justheuristic · 2021-07-13T10:54:14Z

~~[not black-compatible yet]~~ converted below

codecov · 2021-07-13T10:55:49Z

Codecov Report

Merging #317 (f147d18) into master (d97fede) will decrease coverage by 0.02%.
The diff coverage is 88.27%.

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
- Coverage   81.97%   81.95%   -0.03%     
==========================================
  Files          66       66              
  Lines        5898     5918      +20     
==========================================
+ Hits         4835     4850      +15     
- Misses       1063     1068       +5

Impacted Files	Coverage Δ
hivemind/moe/server/task_pool.py	`43.16% <0.00%> (ø)`
hivemind/utils/mpfuture.py	`89.83% <86.86%> (-4.86%)`	⬇️
hivemind/p2p/p2p_daemon.py	`92.40% <100.00%> (+1.11%)`	⬆️
hivemind/p2p/servicer.py	`92.45% <100.00%> (-0.14%)`	⬇️
hivemind/dht/__init__.py	`89.38% <0.00%> (+1.76%)`	⬆️
hivemind/averaging/key_manager.py	`97.72% <0.00%> (+2.27%)`	⬆️

hivemind/utils/mpfuture.py

tests/test_util_modules.py

hivemind/utils/mpfuture.py

Co-authored-by: Max Ryabinin <[email protected]>

…make-mpfuture-great-again

hivemind/utils/mpfuture.py

borzunov · 2021-07-13T20:25:34Z

hivemind/utils/mpfuture.py

-
-    def _set_event_threadsafe(self):
+    def _set_event_if_necessary(self):
+        if self._aio_event is None or self._aio_event.is_set():


nit: self._aio_event.is_set() is not guaranteed to be thread-safe. This check can probably be removed.

decided to keep is_set for performance reasons

[It's thread-safe in the current implementation, and we've agreed that it's hard to imagine that it will stop being thread-safe.]

hivemind/utils/mpfuture.py

borzunov · 2021-07-13T21:40:01Z

hivemind/utils/mpfuture.py

+                        name=f"{__name__}.BACKEND",
+                        daemon=True,
+                    )
+                    cls._pipe_waiter_thread.start()

    @classmethod
    def _process_updates_in_background(cls, receiver_pipe: mp.connection.Connection):
        pid = os.getpid()
        while True:


nit: This loop never stops gracefully. Ideally, it would be cool to stop it via a threading.Event in the __del__ finalizer of the last active future in the current process.

However, graceful shutdown may be out of scope of this PR.

hivemind/utils/mpfuture.py

Co-authored-by: Alexander Borzunov <[email protected]>

…make-mpfuture-great-again

borzunov · 2021-07-13T22:38:04Z

hivemind/utils/mpfuture.py

-            self._state_cache[self._state], self._result = base.FINISHED, result
-            self._send_update(UpdateType.RESULT, result)
+            self._send_update(MessageType.RESULT, result)
+        super().set_result(result)


nit: In Python 3.7, this may raise RuntimeError (instead of InvalidStateError) if set_result is called concurrently inside one process.

This is a minor issue and we've agreed it won't be fixed.

This reverts commit 197666c.

justheuristic added 3 commits July 11, 2021 16:29

fix "coroutine was never awaited"

4248a91

client-server-style MPFuture

c2ce35c

Merge branch 'master' into make-mpfuture-great-again

a310f26

# Conflicts: # hivemind/moe/server/task_pool.py # hivemind/utils/mpfuture.py # tests/test_util_modules.py

justheuristic requested review from mryab and borzunov July 13, 2021 10:54

justheuristic added 4 commits July 13, 2021 13:57

re-black

5321f7f

re-black

904c808

py39 compat

34e2d1d

py39 compat

0e38f5d

justheuristic linked an issue Jul 13, 2021 that may be closed by this pull request

[BUG] Tests freeze without increasing ulimit -n #312

Closed

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

borzunov reviewed Jul 13, 2021

View reviewed changes

tests/test_util_modules.py Outdated Show resolved Hide resolved

justheuristic and others added 3 commits July 13, 2021 18:27

Merge branch 'master' into make-mpfuture-great-again

1849fe2

review

668698d

Merge branch 'master' into make-mpfuture-great-again

ba2d0cd

mryab reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

mryab reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

mryab reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

mryab reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

justheuristic and others added 6 commits July 13, 2021 23:12

make moe test easier

5d87238

Update hivemind/utils/mpfuture.py

7652a99

Co-authored-by: Max Ryabinin <[email protected]>

Update hivemind/utils/mpfuture.py

e5fc179

Co-authored-by: Max Ryabinin <[email protected]>

Update hivemind/utils/mpfuture.py

f5600c6

Co-authored-by: Max Ryabinin <[email protected]>

Update hivemind/utils/mpfuture.py

9b563bb

Co-authored-by: Max Ryabinin <[email protected]>

Merge remote-tracking branch 'origin/make-mpfuture-great-again' into …

2dcd26d

…make-mpfuture-great-again

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Show resolved Hide resolved

mryab changed the title ~~Remove sharedmemory from MPFuture, fix minor bugs~~ Remove shared memory from MPFuture, fix minor bugs Jul 13, 2021

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

mryab approved these changes Jul 13, 2021

View reviewed changes

make moe test easier

bd852ef

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

borzunov and others added 5 commits July 14, 2021 00:09

review

d6a4048

review

f95a00a

review

d564efc

review

1667264

review

8a01bf1

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Show resolved Hide resolved

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

review

1a4d7a8

borzunov reviewed Jul 13, 2021

View reviewed changes

hivemind/utils/mpfuture.py Outdated Show resolved Hide resolved

justheuristic and others added 3 commits July 14, 2021 01:20

Update hivemind/utils/mpfuture.py

ad34fdf

Co-authored-by: Alexander Borzunov <[email protected]>

review

3ee5732

Merge remote-tracking branch 'origin/make-mpfuture-great-again' into …

f147d18

…make-mpfuture-great-again

borzunov approved these changes Jul 13, 2021

View reviewed changes

justheuristic merged commit 197666c into master Jul 13, 2021

justheuristic deleted the make-mpfuture-great-again branch July 13, 2021 22:37

borzunov reviewed Jul 13, 2021

View reviewed changes

borzunov added a commit that referenced this pull request Jul 23, 2021

Revert "Remove shared memory from MPFuture, fix minor bugs (#317)"

557da14

This reverts commit 197666c.

borzunov mentioned this pull request Jul 23, 2021

Convert averager to libp2p backend #323

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove shared memory from MPFuture, fix minor bugs #317

Remove shared memory from MPFuture, fix minor bugs #317

justheuristic commented Jul 13, 2021 •

edited

Loading

justheuristic commented Jul 13, 2021 •

edited

Loading

codecov bot commented Jul 13, 2021 •

edited

Loading

borzunov Jul 13, 2021 •

edited

Loading

justheuristic Jul 13, 2021

borzunov Jul 13, 2021 •

edited by justheuristic

Loading

borzunov Jul 13, 2021

borzunov Jul 13, 2021

Remove shared memory from MPFuture, fix minor bugs #317

Remove shared memory from MPFuture, fix minor bugs #317

Conversation

justheuristic commented Jul 13, 2021 • edited Loading

justheuristic commented Jul 13, 2021 • edited Loading

codecov bot commented Jul 13, 2021 • edited Loading

Codecov Report

borzunov Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

justheuristic Jul 13, 2021

Choose a reason for hiding this comment

borzunov Jul 13, 2021 • edited by justheuristic Loading

Choose a reason for hiding this comment

borzunov Jul 13, 2021

Choose a reason for hiding this comment

borzunov Jul 13, 2021

Choose a reason for hiding this comment

justheuristic commented Jul 13, 2021 •

edited

Loading

justheuristic commented Jul 13, 2021 •

edited

Loading

codecov bot commented Jul 13, 2021 •

edited

Loading

borzunov Jul 13, 2021 •

edited

Loading

borzunov Jul 13, 2021 •

edited by justheuristic

Loading