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

Unserialized and Reentrant Signals #797

Merged
merged 19 commits into from
Jul 23, 2021
Merged

Unserialized and Reentrant Signals #797

merged 19 commits into from
Jul 23, 2021

Conversation

andersio
Copy link
Member

@andersio andersio commented Aug 15, 2020

  1. Signal now offers an unserialized variant and a reentrant variant for advanced users. (Unserialized and Reentrant Signals #797)

    These new primitives would be adopted by ReactiveCocoa UIKit bindings to improve interoperability with RAC Loop, to tackle some legitimate recursive delivery scenarios (e.g. around first responder management), and also to reduce fine-grained locking across ReactiveSwift and ReactiveCocoa.

    Note that the default behavior of Signal has not been changed — event serialization remains the default behavior. MutableProperty is also not gonna be reentrant, because there is no sane reentrancy model that supports access to the current value and multicasting while being compliant to the Property contract.

  2. Fine-grained locking has been scaled down across Signal, SignalProducer and Property. (Unserialized and Reentrant Signals #797)

    Observers are naturally inheriting mutual exclusion from upstream, because the upstream has to serialize the events to be compliant to the Event contract. So for most transformations involving only one event stream (Signal/Producer), the locks for serializing events are redundant. This is also the same premise that RAS 2.0 producer optimizations are built upon.

    This similarly applies to Properties — both the read-only and the mutable versions have their own locks for mutual exclusion. So for them, the only value which the default event serialization in Signal/SignalProducer brings is wasting CPU time.

Checklist

  • Updated CHANGELOG.md.

@andersio andersio marked this pull request as ready for review August 15, 2020 02:59
@andersio andersio force-pushed the unserialized-pipes branch 2 times, most recently from 7e1422a to 924a55b Compare August 15, 2020 12:38
Copy link
Contributor

@sharplet sharplet left a comment

Choose a reason for hiding this comment

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

I think this will be a nice improvement, thanks!

I'd like to take a deeper look at the code, but for now I did a quick scan for API naming. Wondering what you think about using the term unserialized instead of non-serializing, which reads awkwardly to me.

The terms introduced to our public API here a subtle, so I also left a suggestion that we could perhaps document them in a glossary of sorts to get a very clear definition that we can point to for decisions about where one makes more sense than the other. I'd also like to have a think about whether there's a better term to use than "unserialized".

Sources/Signal.swift Outdated Show resolved Hide resolved
Sources/Signal.swift Outdated Show resolved Hide resolved
@@ -200,6 +200,51 @@ class SignalSpec: QuickSpec {
}
}

describe("reentrant") {
#if arch(x86_64) && canImport(Darwin)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for restricting this text to x86?

Copy link
Member Author

Choose a reason for hiding this comment

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

Catching deadlock by intercepting signals is supported only in iOS simulators.

https://github.com/mattgallagher/CwlPreconditionTesting#cwlpreconditiontesting

A Mach exception handler, written in Swift and Objective-C, that allows EXC_BAD_INSTRUCTION (as raised by Swift's assertionFailure/preconditionFailure/fatalError) to be caught and tested.

NOTE: the iOS code runs in the simulator only. It is for logic testing and cannot be deployed to the device due to the Mach exception API being private on iOS.

Copy link

Choose a reason for hiding this comment

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

What about Macs with an M1?

Copy link
Member Author

@andersio andersio Jan 5, 2021

Choose a reason for hiding this comment

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

It is more a question for Nimble/CwlPreconditionSignal. If they support catching signals on arm64/M1, we can lift this.

@andersio andersio changed the title Non-serializing and Reentrant Signals Unserialized and Reentrant Signals Jan 1, 2021
@mluisbrown
Copy link
Contributor

to tackle some legitimate recursive delivery scenarios (e.g. around first responder management)

@andersio any update on when this might get merged and into a release? It would be extremely useful for RAS-TCA, for the same reasons as it would be useful in Loop.

@maximkrouk
Copy link

+1

@mluisbrown
Copy link
Contributor

Any update on when this might get merged @andersio ?

@andersio andersio merged commit 70ad4da into master Jul 23, 2021
@andersio andersio deleted the unserialized-pipes branch July 23, 2021 16:59
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.

5 participants