Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

Locks on UI thread or async dispatches, can't have neither #5

Open
vittoriom opened this issue May 16, 2016 · 3 comments
Open

Locks on UI thread or async dispatches, can't have neither #5

vittoriom opened this issue May 16, 2016 · 3 comments
Assignees
Labels

Comments

@vittoriom
Copy link
Contributor

From @bkase on April 7, 2016 0:6

Due to the use of ReadWriteLocks in many places (notably Promises), it's impossible to both avoid touching locks on the MainThread (which is very important esp. if you're using a lot of Promises) and also avoid any async dispatches (which is important when loading UIImages from a MemoryCacheLevel for example to avoid blinks)

One potential solution could be creating two different Promise types both conforming to some protocol PromiseType (and have Future's constructor take PromiseType rather than Promise).

where

  1. MainThreadPromise has precondition(NSThread.isMainThread()) everywhere and has no locks
  2. AtomicPromise is current Promise with the locks (but acquires locks)

Any thoughts?

Copied from original issue: spring-media/Carlos#148

@vittoriom vittoriom self-assigned this May 16, 2016
@vittoriom
Copy link
Contributor Author

Hi @bkase, thanks for your suggestion!
I'm aware of this issue, and you're right that we should adopt some solution.
My only concern is that even with this supposed PromiseType, every CacheLevel decides internally what to use, and you can not force it unless you create your own, replicating its functionality but using another Promise (that you can do even now).

How would you proceed in this case?

@vittoriom
Copy link
Contributor Author

From @bkase on April 8, 2016 1:51

I wonder if there exists an easy, clean way to configure the built-in caches to either be threadsafe (use the "AtomicPromise") or fast (use the "MainThreadPromise").

I can imagine there may be something to using protocols to mixin the appropriate promise behavior.
This would require a huge refactoring, but imagine something like this:

protocol Asynchronous {
  associatedtype PromiseKind

  func makePromise() -> Self.PromiseKind
}
extension Asynchronous where Self.PromiseType == MainThreadPromise {
  func makePromise() -> MainThreadPromise {
    return MainThreadPromise()
  }
}
extension Asynchronous where Self.PromiseType == AtomicPromise {
  func makePromise() -> MainThreadPromise {
    return AtomicPromise()
  }
}

protocol MemoryCacheLevel: CacheLevel, Asynchronous {
   /*...*/
}
extension MemoryCacheLevel where Self.KeyType == Hashable {
  /* basically all of the current memory cache, but use `makePromise` */
}

class UIMemoryCache<K: Hashable, V/*...*/>: MemoryCacheLevel {
   typealias PromiseKind = MainThreadPromise
}
class AtomicMemoryCache: MemoryCacheLevel {
  typealias PromiseKind = AtomicPromise
}

What do you think about something like this?

Also:

you can not force it unless you create your own, replicating its functionality but using another Promise (that you can do even now).

Can you do this now? Future has a dependency on Promise, so even though Promise isn't exposed in any public APIs, there isn't a way to use a different Promise implementation and still use the CacheLevel protocol.

@vittoriom
Copy link
Contributor Author

Hi @bkase, I'm sorry you're right about the dependency Future has on Promise, I overlooked that. You can't effectively use another Promise implementation because of this.
Nevertheless, Promise is not a final class, so in theory one could subclass it to make it "main thread-safe" and produce that in your own CacheLevel implementation.

Here comes the catch, though (that is also the reason why I think having two implementations of MemoryCacheLevel would not solve the issue):
In basically all the transformation and composition functions, we always create new Promises that differ in some way from the original ones. We would have to determine which Promise to create everywhere in Carlos. This would require probably a "global setting" to define what type of Promise to create.

I have to think about it a little bit more but I'm afraid the complication would be quite big, and adding a "setting" to determine which Promise to create everywhere in the framework doesn't sound like a nice design solution either.

If you have any other idea, just write it here!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant