Typechecking timeout compiling tests in v7

[GarthSnyder]

With the cancellation code merged into v7, my hamster-wheel-powered Mac Mini is timing out while compiling Tests/Cancel/DispatcherTests.swift -> testMapValues().

    func testMapValues() {
        let ex1 = expectation(description: "DispatchQueue MapValues compatibility")
        Promise.value([42, 52]).cancellize().then(on: .global(qos: .background), flags: .barrier) { v -> Promise<[Int]> in
            Promise.value(v)
        }.compactMap(on: .global(qos: .background), flags: .barrier) {
            $0
        }.mapValues(on: .global(qos: .background), flags: .barrier) {
            $0 + 10
        }.flatMapValues(on: .global(qos: .background), flags: .barrier) {
            [$0 + 10]
        }.compactMapValues(on: .global(qos: .background), flags: .barrier) {
            $0 + 10
        }.thenMap(on: .global(qos: .background), flags: .barrier) { v -> CancellablePromise<Int> in
            Promise.value(v + 10).cancellize()
        }.thenMap(on: .global(qos: .background), flags: .barrier) { v -> Promise<Int> in
            Promise.value(v + 10)
        }.thenFlatMap(on: .global(qos: .background), flags: .barrier) {
            Promise.value([$0 + 10]).cancellize()
        }.thenFlatMap(on: .global(qos: .background), flags: .barrier) { v -> Promise<[Int]> in
            Promise.value([v + 10])
        }.filterValues(on: .global(qos: .background), flags: .barrier) { _ in
            true
        }.sortedValues(on: .global(qos: .background), flags: .barrier).firstValue(on: .global(qos: .background), flags: .barrier) { _ in
            true
        }.done(on: .global(qos: .background), flags: .barrier) {
            XCTAssertEqual($0, 112)
            ex1.fulfill()
        }.catch(on: .global(qos: .background), flags: .barrier) { _ in
            XCTFail()
        }

The error is the usual "The compiler is unable to type-check this expression in reasonable time; try breaking up the expression into distinct sub-expressions".

This is a pretty complicated expression to resolve (259 separate type decisions, according to swift -debug-constraints), but I'm a bit suspicious that the complexity may derive from the use of cancellable promises. If you edit out the calls to cancellize(), it compiles fine.

@dougzilla32, is there some reason why cancellable promises might have a broader API than regular promises?

[dougzilla32]

I cannot reproduce this as is, however I'm able to easily reproduce this if I remove the closure return type from 2 or more of the 'thenMap' and 'thenFlatMap' calls. I believe the workaround for this specific test case is to specify the closure return type for both of the calls to thenFlatMap -- Garth does this workaround solve the problem for you?

If so we should check in the workaround for now to unblock everyone. But this is a general problem that needs to be solved for cancellable promises.

After some analysis my initial take is that this test case is triggering an n-squared problem in the compiler, which is worsened by the number of possible combinations of parameters for methods that share the same name. It may only be triggered when at least one of the parameters is a closure but I haven't confirmed this.

I tried to resolve this by adding a namespacer parameter to all the 'thenXXX' methods on CancellableThenable where the closure returns a standard promise. For example:

public enum CancelNamespacer {
    case cancellize
}

func thenMap<V: CancellableThenable>(
    on: Dispatcher = conf.D.map,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.U.T]> 

func thenMap<V: Thenable>(
    _: CancelNamespacer,
    on: Dispatcher = conf.D.map,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.T]>

AND

...

}.thenMap(on: .global(qos: .background), flags: .barrier) {
    Promise.value($0 + 10).cancellize()
}.thenMap(.cancellize, on: .global(qos: .background), flags: .barrier) {
    Promise.value($0 + 10)

...

But this just makes the problem worse!! Which leads me to believe that the problem is worsened by increasing the possible combinations of parameters.

I'll continue to look at this and give updates as I make discoveries or find solutions. If anyone has insights or suggestions I'd love to hear them!

This problem likely affects all of the following methods in the cancellable classes:

/// CancellableThenable.swift

public extension CancellableThenable {
  func then<V: CancellableThenable>(
    on: Dispatcher = conf.D.map,
    _ body: @escaping (U.T) throws -> V) -> CancellablePromise<V.U.T>

  func then<V: Thenable>(
    on: Dispatcher = conf.D.map,
    _ body: @escaping (U.T) throws -> V) -> CancellablePromise<V.T>
}

public extension CancellableThenable where U.T: Sequence {
  func thenMap<V: CancellableThenable>(
    on: Dispatcher = conf.D.map,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.U.T]>

  func thenMap<V: Thenable>(
    on: Dispatcher = conf.D.map,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.T]>


  func thenFlatMap<V: CancellableThenable>(
    on: Dispatcher = conf.D.map,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.U.T.Iterator.Element]> where V.U.T: Sequence

  func thenFlatMap<V: Thenable>(
    on: Dispatcher = conf.D.map,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.T.Iterator.Element]> where V.T: Sequence
}


/// CancellableCatchable.swift:

public extension CancellableCatchMixin {
  func recover<V: CancellableThenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    policy: CatchPolicy = conf.catchPolicy,
    _ body: (Error) throws -> V) -> CancellablePromise<C.T> where V.U.T == C.T

  func recover<V: Thenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    policy: CatchPolicy = conf.catchPolicy,
    _ body: (Error) throws -> V) -> CancellablePromise<C.T> where V.T == C.T
}


/// Dispatcher.swift:

public extension CancellableThenable {
  func then<V: CancellableThenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    _ body: (U.T) throws -> V) -> CancellablePromise<V.U.T>

  func then<V: Thenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    _ body: (U.T) throws -> V) -> CancellablePromise<V.T>
}

public extension CancellableThenable where U.T: Sequence {
  func thenMap<V: CancellableThenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.U.T]>

  func thenMap<V: Thenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.T]>


  func thenFlatMap<V: CancellableThenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.U.T.Iterator.Element]> where V.U.T: Sequence

  func thenFlatMap<V: Thenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    _ transform: (U.T.Iterator.Element) throws -> V) -> CancellablePromise<[V.T.Iterator.Element]> where V.T: Sequence
}

public extension CancellableCatchMixin {
  func recover<V: CancellableThenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    policy: CatchPolicy = conf.catchPolicy,
    _ body: (Error) throws -> V) -> CancellablePromise<C.T> where V.U.T == C.T

  func recover<V: Thenable>(
    on: DispatchQueue? = conf.Q.map,
    flags: DispatchWorkItemFlags? = nil,
    policy: CatchPolicy = conf.catchPolicy,
    _ body: (Error) throws -> V) -> CancellablePromise<C.T> where V.T == C.T
}

[mxcl]

We should be able to drop the API surface with a refactor or two. I'm not fussed about backwards compatibility, since PMKv6 is stable and will be maintained, I feel upgrading is a choice that will require some gardening.

Asidedly, the test is absurdly long chain, IME they don't get this long in read code. Please correct me if I'm wrong in your experience.

So if we refactor, and we make this test specify return types for all closures, we should be good IMO. But something to keep in mind. We don’t want to cause horrible compile performance and PMK has a lot of type-checking.

[dougzilla32]

After further investigation, I've discovered that unfortunately this is a more basic compiler problem than I first thought.

I was able to reproduce this in a simplified way. Both of the following standalone examples demonstrate the issue "The compiler is unable to type-check this expression in reasonable time; try breaking up the expression into distinct sub-expressions". The first simulates CancellablePromise,
Promise and Thenable and the second example does the same test with Thenable removed.

NOTE that If you specify the return types for all the closures (uncomment) then everything compiles fine.

You can copy/paste these examples into a new project and attempt a build to see the issue. On my machine I am able to compile (but only just barely) if I comment out all the thenFlatMap calls.

The second example demonstrates that refactoring PromiseKit 7 to eliminate Thenable would not solve this particular issue.

The compiler initially does ok with methods that have the same names and closure parameters with different return types. But it runs into trouble when Sequence qualifiers are added to the mix.

The only solutions I've come up with so far are to either require the user to specify return types for all the closures or to have different names for the two variants of thenMap and thenFlatMap.

Perhaps requiring the return types is acceptable? Requiring the return type makes things tricky for the user because if you mess up the return type it is slow going trying to correct it (have to wait for compiler timeouts or keep killing the build).

Having different names does not look nice for the for cancellable promise chains.

Test 1:

protocol CTh {
    associatedtype U: Th
}

extension CTh {
    func then<V: CTh>(_ body: @escaping (U.BaseType) throws -> V) -> CP<V.U.BaseType> {
        return CP()
    }

    func then<V: Th>(_ body: @escaping (U.BaseType) throws -> V) -> CP<V.BaseType> {
        return CP()
    }
}

extension CTh where U.BaseType: Sequence {
    func thenMap<V: CTh>(_ transform: @escaping(U.BaseType.Iterator.Element) throws -> V) -> CP<[V.U.BaseType]> {
        return CP()
    }

    func thenMap<V: Th>(_ transform: @escaping(U.BaseType.Iterator.Element) throws -> V) -> CP<[V.BaseType]> {
        return CP()
    }

    func thenFlatMap<V: CTh>(_ transform: @escaping(U.BaseType.Iterator.Element) throws -> V) -> CP<[V.U.BaseType.Iterator.Element]> where V.U.BaseType: Sequence {
        return CP()
    }

    func thenFlatMap<V: Th>(_ transform: @escaping(U.BaseType.Iterator.Element) throws -> V) -> CP<[V.BaseType.Iterator.Element]> where V.BaseType: Sequence {
        return CP()
    }
}

protocol Th {
    associatedtype BaseType
}

class CP<BaseType>: CTh {
    typealias U = P<BaseType>
}

class P<BaseType>: Th {
    class func value(_ value: BaseType) -> P<BaseType> {
        return P<BaseType>()
    }
}

extension Th {
    func cancellize() -> CP<BaseType> {
        return CP()
    }
}

class test {
    func testMapValues() {
        P.value([42, 52]).cancellize().then { // v -> P<[Int]> in
            P.value($0)
        }.then { // v -> P<[Int]> in
            return P.value($0)
        }.thenMap { // v -> CP<Int> in
            P.value($0 + 10).cancellize()
        }.thenMap { // v -> P<Int> in
            P.value($0 + 10)
        }.thenFlatMap { // v -> CP<[Int]> in
            P.value([$0 + 10]).cancellize()
        }.thenFlatMap { // v -> P<[Int]> in
            P.value([$0 + 10])
        }
    }
}

Test 2:

class SimpleCP<BaseType> {
    func then<NewT>(_ body: @escaping (BaseType) throws -> SimpleCP<NewT>) -> SimpleCP<NewT> {
        return SimpleCP<NewT>()
    }

    func then<NewT>(_ body: @escaping (BaseType) throws -> SimpleP<NewT>) -> SimpleCP<NewT> {
        return SimpleCP<NewT>()
    }
}

extension SimpleCP where BaseType: Sequence {
    func thenMap<NewT>(_ transform: @escaping(BaseType.Iterator.Element) throws -> SimpleCP<NewT>) -> SimpleCP<[NewT]> {
        return SimpleCP<[NewT]>()
    }

    func thenMap<NewT>(_ transform: @escaping(BaseType.Iterator.Element) throws -> SimpleP<NewT>) -> SimpleCP<[NewT]> {
        return SimpleCP<[NewT]>()
    }

    func thenFlatMap<NewT>(_ transform: @escaping(BaseType.Iterator.Element) throws -> SimpleCP<NewT>) -> SimpleCP<[NewT.Iterator.Element]> where NewT: Sequence {
        return SimpleCP<[NewT.Iterator.Element]>()
    }

    func thenFlatMap<NewT>(_ transform: @escaping(BaseType.Iterator.Element) throws -> SimpleP<NewT>) -> SimpleCP<[NewT.Iterator.Element]> where NewT: Sequence {
        return SimpleCP<[NewT.Iterator.Element]>()
    }
}

class SimpleP<BaseType> {
    class func value(_ value: BaseType) -> SimpleP<BaseType> {
        return SimpleP<BaseType>()
    }

    func cancellize() -> SimpleCP<BaseType> {
        return SimpleCP()
    }
}

class testSimple {
    func testMapValues() {
        SimpleP.value([42, 52]).cancellize().then { // v -> SimpleP<[Int]> in
            SimpleP.value($0)
        }.then { // v -> SimpleP<[Int]> in
            return SimpleP.value($0)
        }.thenMap { // v -> SimpleCP<Int> in
            SimpleP.value($0 + 10).cancellize()
        }.thenMap { // v -> SimpleP<Int> in
            SimpleP.value($0 + 10)
        }.thenFlatMap { // v -> SimpleCP<[Int]> in
            SimpleP.value([$0 + 10]).cancellize()
        }.thenFlatMap { // v -> SimpleP<[Int]> in
            SimpleP.value([$0 + 10])
        }
    }
}

[GarthSnyder]

Great work, Doug - really nice mini example.

I didn't play around with test 1 since 2 is simpler. But here's what's confusing to me about 2: if you take out the instances of + 10, or even just a couple of them, it compiles with no problem.

Just to simplify the types further, I replaced the start of the chain with

let numbers = [42, 52]
SimpleP.value(numbers)...

which I would assume would fix the BaseType for everything at either [Int] or Int. (Without that, I guess the initial type could theoretically be an array of anything that's ExpressibleByIntegerLiteral.) But this doesn't help; type checking still times out for me.

Int + 10 seems like it should be a fixed type. If you compile that on its own, the compiler only considers 3 scopes (why it's more than 1, I'm not sure). It must be the case that the compiler isn't doing a very efficient job of detecting that $0 can only be Int in most of these closures.

In any event, this seems like a pretty clear demonstration that the PromiseKit code isn't doing anything unreasonable.

As far as compiling the library, yes, any of the usual fixes works for me - splitting up the chain, annotating types, etc.

[dougzilla32]

Whoa that's funky!! I didn't know the + 10 was making the difference! Great observation! That'll definitely help if/when I get around to filing a report against the Swift compiler.

I created pull request #1027 to add a return type for the thenFlatMap closure, that should do the trick for the test case.

But the compiler isn't so nice if you get the return type wrong -- it just gives you the same timeout error as if you'd left it out entirely.

Thanks for the help!!

[GarthSnyder]

Max: Asidedly, the test is absurdly long chain, IME they don't get this long in read code. Please correct me if I'm wrong in your experience.

Only in RxSwift 😁

[mxcl]

I removed this test for now even after trying a bit to get it to work.