Skip to content
New issue

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

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

Already on GitHub? # to your account

When.BarrierQueue #26

Closed
MontakOleg opened this issue Jan 19, 2018 · 5 comments
Closed

When.BarrierQueue #26

MontakOleg opened this issue Jan 19, 2018 · 5 comments

Comments

@MontakOleg
Copy link
Contributor

Hello! I found what barrier queue is concurrent:

Functions.swift:

let barrierQueue = DispatchQueue(label: "When.BarrierQueue", attributes: DispatchQueue.Attributes.concurrent)

But it must be serial to behave as real barrier. Misprint?

@vadymmarkov
Copy link
Owner

Yeah, I think concurrentQueue would be more appropriate name in this case. Because in order to use concurrent queue with barrier we use flags like concurrentQueue.async(flags: .barrier). But as I see this constant is not even used anywhere in code, so I might end up removing it. Good catch anyway @MontakOleg 👍

@MontakOleg
Copy link
Contributor Author

This queue used in when() function (https://github.com/vadymmarkov/When/blob/master/Sources/When/Functions.swift#L30) to synchronize access to resolved counter.
So now with concurrent queue access is not synchronized.

Try run this test:

override func spec() {
  it("resolves the promise") {

    let promises: [Promise<Int>] = (0..<100000).map { _ in
        let promise = Promise<Int>(queue: DispatchQueue.global(), { resolve in
            // +3 needed to avoid https://github.com/vadymmarkov/When/issues/27 issue
            DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + 3) {
               resolve(1)
            }
        })
        return promise
    }

    let doneExpectation = self.expectation(description: "Done expectation")

    when(promises)
      .done({ result in
        doneExpectation.fulfill()
      })

    self.waitForExpectations(timeout: 5.0, handler:nil)
  }
}

It fails. But if you change

let barrierQueue = DispatchQueue(label: "When.BarrierQueue", attributes: DispatchQueue.Attributes.concurrent)

to

let barrierQueue = DispatchQueue(label: "When.BarrierQueue")

test will success.

@vadymmarkov
Copy link
Owner

@MontakOleg It makes sense I guess. How about if we change barrierQueue.sync(flags: .barrier) in https://github.com/vadymmarkov/When/blob/master/Sources/When/Functions.swift#L30

@MontakOleg
Copy link
Contributor Author

MontakOleg commented Jan 21, 2018

@vadymmarkov

Yeah, if we add flags: .barrier on
https://github.com/vadymmarkov/When/blob/master/Sources/When/Functions.swift#L30
and https://github.com/vadymmarkov/When/blob/master/Sources/When/Functions.swift#L38 it should work fine.

But if all calls to queue uses .barrier flag there is no difference with serial queue.
.barrier flag is useful for implementing "multi-readers, one writer" pattern: for "reader" blocks, you use queue.[a]sync, for "writer" blocks, you use queue.[a]sync(flags: .barrier)

@vadymmarkov
Copy link
Owner

@MontakOleg Agree, good point 👍 Would you mind making a PR to change the queue to be not concurrent?

MontakOleg pushed a commit to MontakOleg/When that referenced this issue Jan 22, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants