Git Product home page Git Product logo

Comments (18)

shoumikhin avatar shoumikhin commented on April 28, 2024

Hi Vladimir, is this a duplicate of #5 and #64?

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin It seems so, but availability of CancelToken is must-have anyway for promises to be usable in any UI code (for UITableView cells, for example), because iOS uses GC and we need reliable way to deallocate promise listeners. Otherwise common usage of promises by average programmer will lead to deadlocks and/or excessive memory consumption and/or race conditions.

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

Yep, I know that's a long-awaited feature. The good news is once a promise gets resolved, all its listeners get scheduled on GCD, so the latter controls their lifespan further on. And currently, as we've figured out in #74, you can directly reject any promise you're willing to cancel beforehand with a special error and skip handling it in catch, for example. By the way, I believe that's approximately the way how it's gonna be implemented eventually (plus, an important feature of cancelling a parent promise whose listeners have all bailed early, and providing a corresponding callback in the work block itself). Meanwhile, you can also check out if a custom token would come handy by any chance.

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin That custom token cancels the underlying task, not the promise callbacks registration.

Just try to implement UITableView controller with your promises sdk, where each cell will asynchronously load its content from network or background database like Realm.

Each cell object can be reused by UITableView, so before cell promise resolves, cell can be used to display different content entirely.

Without CancelToken bundled in your SDK, it means that your SDK is not usable for most common of asynchronous UI loading use cases.

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

Vladimir, I'm sorry for confusion. I do understand how important and convenient the cancellation story can be. I guess what I'm trying to say is that there's probably a way to achieve what you want, although maybe not the most elegant one, of course.

Say, for your example, the table view controller is probably the one initiating an async operation per each cell after dequeueing it from cache using some network layer, which apparently also caches responses. Such a network layer would have an API returning a promise containing data needed to setup the cell. The controller can become a delegate of each cell and be notified when a cell gets reused in order to tell the network layer to cancel a particular request, which, in turn, could use something like the custom token I referred in the previous reply. Also, if we can afford to disable prefetching, we can then use cellForRow(at:) inside the then callback chained to the network call inside dequeueReusableCell(withIdentifier:for:) to check against nil, and so learn if the cell with a particular indexPath has already been reused for free and skip the update.

Anyhow, I realize there can be more sophisticated use cases where the lack of proper cancellation may appear frustrating. So, thank you again for raising that concern!

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin

The controller can become a delegate of each cell and be notified when a cell gets reused in order to tell the network layer to cancel a particular request

No, it cannot, because telling the network layer to cancel the request doesn't mean that it will be cancelled — it can be fulfilled before the cancellation event is processed.
And cancellation of underlying process may not be possible anyway in underlying API (like DB API).

Also, if we can afford to disable prefetching, we can then use cellForRow(at:) inside the then callback chained to the network call inside dequeueReusableCell(withIdentifier:for:) to check against nil

No, we cannot, because cell object can already be reused for different indexPath entirely.

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

No, it cannot, because telling the network layer to cancel the request doesn't mean that it will be cancelled — it can be fulfilled before the cancellation event is processed.
And cancellation of underlying process may not be possible anyway in underlying API (like DB API).

How would cancelling a promise be helpful in that situation, if it's been already fulfilled with the response and then block is already scheduled on GCD? As I understand, a promise can't change its state after it's been resolved, even if cancellation is requested. So we still need to find a way to check if the cell has been reused inside the then block, even if we tried to cancel the promise somewhere at prepareForReuse moment.

No, we cannot, because cell object can already be reused for different indexPath entirely.

Given prefetching is turned off, I basically meant something like:

 func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
  guard let cell = collectionView.dequeueReusableCell( // ...
  cell.text = "hello world"
  // ...
  client.fetchImage(at: photoURLs[indexPath]).then { [weak collectionView] image in
    if let cell = collectionView?.cellForItem(at: indexPath) as? ImageCell {
      cell.image = image
    }
  }
  return cell
}

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin

How would cancelling a promise be helpful in that situation, if it's been already fulfilled with the response and then block is already scheduled on GCD?

CancelToken in that UI case is used for cancelling not the promise, but its observation callback.

So we still need to find a way to check if the cell has been reused inside the then block, even if we tried to cancel the promise somewhere at prepareForReuse moment

No, upon cell reuse we just cancel the CancelToken supplied when registering observation listener in promise. For new promise we just register new observation listener with new CancelToken.

That way entire cell reuse usecase is handled via single CancelToken field inside cell object.

Given prefetching is turned off, I basically meant something like:

That code is not usable in any performant app (and that's kind of app where async promises are needed in the first place), because cellForItem() works only for visible cells. But we need to start loading content before cells appear on the screen.

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

CancelToken in that UI case is used for cancelling not the promise, but its observation callback.

Sorry, not sure I get that. How do you cancel an observation callback w/o cancelling the promise? The callback is something which is invoked passively based on the promise state changes. We can't fulfill a promise, and meanwhile skip invoking its listeners' success callbacks.

No, upon cell reuse we just cancel the CancelToken supplied when registering observation listener in promise. For new promise we just register new observation listener with new CancelToken.

Would be awesome to see a pseudocode example, if you don't mind.

That way entire cell reuse use case is handled via single CancelToken field inside cell object.

I have some concerns regarding having such a cancellation token stored in a cell. Specifically, that may potentially break MVC badly.

But we need to start loading content before cells appear on the screen.

Yup, exactly why I mentioned the prefetching clause.

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin

How do you cancel an observation callback w/o cancelling the promise?

Easily — with object called CancelToken that is checked on the callback's queue by the Promise before calling listener callback. If it is cancelled, callback will not be called.

Also CancelToken itself have .cancelledPromise field, which triggers when token is cancelled. Via that field the Promise can register listener to unregister user's listener when supplied cancel token is cancelled.

It is important because iOS doesn't have GC and we need to provide the way to cancel Promise listener callback with its full deallocation/release.

The callback is something which is invoked passively based on the promise state changes. We can't fulfill a promise, and meanwhile skip invoking its listeners' success callbacks.

We can. CancelToken is exactly for that, altought itself it should not be bound to only this use case.

In BrightFutures and similar frameworks it is often called InvalidationToken.

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin

MyCell : UITableViewCell
{
	var image: UIImage? = nil
	var cancelTokenSource = CancelTokenSource()

	func onCellReuse(_ contentId: String)
	{
		self.cancelTokenSource.cancel()
		self.cancelTokenSource = CancelTokenSource()

		let ltoken = self.cancelTokenSource.token

		self.image = nil

		network.loadContent(contentId) { [weak self] content in
			guard let sself = self else {
				return
			}

			guard !ltoken.isCancelled else {
				return
			}

			sself.image = content
		}

	}
}
MyCell : UITableViewCell
{
	var image: UIImage? = nil
	var cancelTokenSource = CancelTokenSource()

	func onCellReuse(_ contentId: String)
	{
		self.cancelTokenSource.cancel()
		self.cancelTokenSource = CancelTokenSource()

		let contentPromise = network.loadContent(contentId)

		self.image = nil

		contentPromise.onFulfill(self.cancelTokenSource.token) { [weak self] value in
			guard let sself = self else {
				return
			}

			sself.image = value
		}
	}
}

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

Great, thank you for the examples!

Although, I have a few concerns now. Looks like MyCell, being a view, plays a role of a controller. It keeps refs to model objects and has an important chunk of business logic for updating them. Hope, that's made for brevity only, and in reality that piece is implemented in a controller which owns the table view, or elsewhere. So, let's ignore that.

Easily — with object called CancelToken that is checked on the callback's queue by the Promise before calling listener callback. If it is cancelled, callback will not be called.

That's an interesting idea. By looking at the example, I imagine a promise could just provide a cancel() method with the same success (since CancelTokenSource likely has a ref to the promise) to set that cancelledPromise kept internally and check it before invoking success callbacks.

Anyhow, I'm not convinced yet we should cancel already scheduled success callbacks and disregard already sealed promise's state just because the cancellation was requested after the promise was fulfilled, but before the callbacks had a chance to execute. Sounds like part of the callbacks can potentially be already executed, and the rest got skipped because of such cancellation, whereas the promise is still considered fulfilled when tested explicitly, which may lead to tricky unpredictable errors. A promise must be either rejected with cancellation reason, or finally fulfilled and notify everyone about that. If it's already fulfilled, we can't just retain it from invoking callbacks.

It seems safer to simply pass a custom token inside the block and check if it's set there instead. Which is essentially what you propose, I guess, but without any promises-related APIs.

It is important because iOS doesn't have GC and we need to provide the way to cancel Promise listener callback with its full deallocation/release.

In our implementation a promise schedules all callbacks on GCD (thus, passes the ownership) or deallocates them once it's resolved. And cancellation means resolution.

from promises.

virl avatar virl commented on April 28, 2024

By looking at the example, I imagine a promise could just provide a cancel() method with the same success (since CancelTokenSource likely has a ref to the promise)

No, CancelTokenSource doesn’t have ref to promise.
No, promise cannot provide cancel() method, because we need to cancel (unregister) specific callback, not promise.

cancelledPromise is entirely different promise instance that is needed to receive async notifications about cancel token cancellations.

Anyhow, I'm not convinced yet we should cancel already scheduled success callbacks and disregard already sealed promise's state just because the cancellation was requested after the promise was fulfilled, but before the callbacks had a chance to execute.

Why? I’ve registered listener on Promise. Now I want to unregister it without waiting for promise completion.

How can I do that in your library?

Sounds like part of the callbacks can potentially be already executed, and the rest got skipped because of such cancellation, whereas the promise is still considered fulfilled when tested explicitly, which may lead to tricky unpredictable errors.

No, it is completely normal, because each callback has its own owner that manages it and knows when it cancels it and for what.

If it's already fulfilled, we can't just retain it from invoking callbacks.

You’re not retaining it. You’re unregistering callbacks from it.

Just like you would do .delegate = nil in iOS or unregisterListener() in Android.

It seems safer to simply pass a custom token inside the block and check if it's set there instead. Which is essentially what you propose, I guess, but without any promises-related APIs.

Yes, but that way the block itself will not be deallocated upon token cancellation and all captured block variables will remain retained until promise completes.

And cancellation means resolution.

No, cancellation means unregistering observation of promise state.

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

No, CancelTokenSource doesn’t have ref to promise.

Gotcha, the promise just checks token's state synchronously. Well, a custom token would serve a bit better, since we could skip synchronization and assume it's always accessed on the same thread. But it's not that important, of course.

I’ve registered listener on Promise. Now I want to unregister it without waiting for promise completion.

The thing is by "registering a callback" on a promise you actually create another promise chained on it, and that others can chain onto later, which is waiting for the original promise to resolve. So, technically you can't "unregister a callback", but you can resolve the root promise and influence the state of all chained ones, i.e. which callbacks (if any) will be invoked as the result of such resolution. Or resolve any chained promise directly, of course. That's why I said you can reject the chained promise (influence its state) and so skip executing the success block entirely. Just be sure to do that before that promise got fulfilled. Otherwise, there's no way to change its state anymore and the block will be executed. Or just introduce a custom flag to check it in the success block and bail early. That's even more flexible, IMO.

How can I do that in your library?

For the example above, you do approximately the following:

func onCellReuse() {
  promise.reject(/* predefined erro for cancellation */)
  promise = network.loadContent(...).then { value in 
    // do the work...
  }
}

Which will try to avoid doing the work, unless the content has already loaded.
Or you go the token way, i.e.:

func onCellReuse() {
  cancelToken.cancel()
  cancelToken = CancelToken()
  network.loadContent(...).then { [cancelToken] value in 
    if (cancelToken.isCancelled) { return }
    // do the work...
  }
}

Yes, but that way the block itself will not be deallocated upon token cancellation and all captured block variables will remain retained until promise completes.

In case a promise was fulfilled, all error callbacks get deallocated immediately. And vice versa.
All success callbacks get scheduled on GCD and so are solely owned by it until executed and deallocated.

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin Ah, you need support for promise chaining. Then yes, you're right — promise in your lib cannot use CancelToken directly.

from promises.

shoumikhin avatar shoumikhin commented on April 28, 2024

Hi Vladimir, If I'm not mistaken, you suggested to have something similar to InvalidationToken?

Its implementation seems pretty trivial and it has nothing to do with promise chaining. It also never breaks the premise that a promise cannot change its state after it's been resolved.

The only convenient part there is that you get an early return for free.

With the same success you could implement a simple InvalidationToken like so:

class InvalidationToken {
  public var isValid: Bool {
    pthread_mutex_lock(&mutex)
    let isValid = valid
    pthread_mutex_unlock(&mutex)
    return isValid
  }
  
  public func invalidate() {
    pthread_mutex_lock(&mutex)
    valid = false
    pthread_mutex_unlock(&mutex)
  }

  private var valid = true  

  private var mutex: pthread_mutex_t = {
    var mutex = pthread_mutex_t()
    pthread_mutex_init(&mutex, nil)
    return mutex
  }()
}

And rewrite the example using Google Promises as follows:

class MyCell : UICollectionViewCell {
  var token = InvalidationToken()
  
  public override func prepareForReuse() {
    super.prepareForReuse()
    token.invalidate()
    token = InvalidationToken()
  }
  
  public func setModel(model: Model) {
    let token = self.token
    ImageLoader.loadImage(model.image).then { [weak self] UIImage in
      if (token.isValid) {
        self?.imageView.image = UIImage
      }
    }
  }
}

(again, ignoring the error-prone design when a cell performs functions of a controller)

As you see, it doesn't seem like too much work, and can be implemented using pretty much any promises library out there.

You can even argue having an explicit isValid check inside the success callback may look cleaner for people not familiar with specific promises APIs and also come handy when you want to do something before an early return, e.g. log the cancelled event first, etc.

from promises.

virl avatar virl commented on April 28, 2024

@shoumikhin Yes, I know. But thanks for the example anyway.

from promises.

 avatar commented on April 28, 2024

Thank you for the discussion. Please feel free to reopen if there are anymore questions/comments regarding this topic.

from promises.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.