Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mpz-common): Context::blocking #141

Merged
merged 2 commits into from
May 31, 2024
Merged

Conversation

sinui0
Copy link
Collaborator

@sinui0 sinui0 commented May 24, 2024

This PR adds the Context::blocking method which supports temporarily moving a context to a separate thread to perform blocking computation. I've also moved the CPU backend shim abstraction into mpz-common.

This method is needed to avoid deadlocks on single threaded machines. The alternative approach is to use an mpsc channel between the worker thread and the calling context, but that doesn't work if single-threaded.

@sinui0 sinui0 requested review from themighty1 and th4s May 24, 2024 20:28
@sinui0 sinui0 force-pushed the feat/context-blocking branch 2 times, most recently from fb2cada to 9ec66f2 Compare May 28, 2024 18:33
Copy link
Member

@th4s th4s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I am not sure I understand this PR. A few questions:

This method is needed to avoid deadlocks on single threaded machines.

What is a single-threaded machine and why do we need to support single-threaded machines? Or do you mean a machine with a single CPU core?

The alternative approach is to use an mpsc channel between the worker thread and the calling context, but that doesn't work if single-threaded.

Why doesn't this work?

Base automatically changed from feat/mt-executor to threading-refactor May 29, 2024 16:54
@sinui0
Copy link
Collaborator Author

sinui0 commented May 29, 2024

What is a single-threaded machine and why do we need to support single-threaded machines? Or do you mean a machine with a single CPU core?

Yes, I'm referring to single-core CPUs, and generally environments without threading. Not to be confused with our logical thread abstraction in mpz-common.

Why doesn't this work?

It does work for CpuBackend::blocking_async but not for CpuBackend::blocking. Hard to point out exactly why, but it leads to a deadlock.


The primary purpose of the Context::blocking method is that it handles moving a context to another thread in multi-threaded case. This isn't otherwise possible because most callers are going to be working with a &mut impl Context which obviously isn't 'static.

crates/mpz-common/src/cpu.rs Outdated Show resolved Hide resolved
crates/mpz-common/src/cpu.rs Outdated Show resolved Hide resolved
crates/mpz-common/src/cpu.rs Outdated Show resolved Hide resolved
crates/mpz-common/src/cpu.rs Outdated Show resolved Hide resolved
@sinui0 sinui0 merged commit 9b51bd4 into threading-refactor May 31, 2024
@sinui0 sinui0 deleted the feat/context-blocking branch May 31, 2024 13:35
sinui0 added a commit that referenced this pull request May 31, 2024
commit 2f35271
Author: sinu.eth <[email protected]>
Date:   Fri May 31 06:36:31 2024 -0700

    feat(mpz-common): scoped! macro (#143)

commit 9b51bd4
Author: sinu.eth <[email protected]>
Date:   Fri May 31 06:35:16 2024 -0700

    feat(mpz-common): Context::blocking (#141)

    * feat(mpz-common): Context::blocking

    * Apply suggestions from code review

    Co-authored-by: dan <[email protected]>

    ---------

    Co-authored-by: dan <[email protected]>

commit 8f0b298
Author: th4s <[email protected]>
Date:   Fri May 31 10:30:08 2024 +0200

    Add IO wrapper for OLE (#138)

    * Add `mpz-ole` content of old branch.

    * Reworked message enum.

    * Refactored to work with new `mpz-ole-core`.

    * Add part of feedback.

    * Add more feedback.

    * Add opaque error type.

    * Add `Display` for `OLEErrorKind`

    * Use `ok_or_elese` for lazy heap alloc.

    * Adapted `mpz-ole` to `hybrid-array`.

    * WIP: Improving API of const generics...

    * Add random OT for `hybrid-array`.

    * Adapt `mpz-ole` to use new random OT.

    * Added feedback.

    * Use random OT over field elements instead of arrays.

    * Refactored ideal implementation to use `mpz-common`.

    * Added more feedback.
sinui0 added a commit that referenced this pull request Jun 25, 2024
* feat(mpz-common): Context::blocking

* Apply suggestions from code review

Co-authored-by: dan <[email protected]>

---------

Co-authored-by: dan <[email protected]>
sinui0 added a commit that referenced this pull request Jun 25, 2024
* feat(mpz-common): Context::blocking

* Apply suggestions from code review

Co-authored-by: dan <[email protected]>

---------

Co-authored-by: dan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants