hmlongco / Resolver

Swift Ultralight Dependency Injection / Service Locator framework
MIT License
2.14k stars 188 forks source link

[QUESTION] Locking Mechanism #109

Closed gmogames closed 3 years ago

gmogames commented 3 years ago

Hi There,

First, thanks for the great lib! I just had a question relating to the thread locking, you are using pthread_mutex_lock and not NSLock/NSRecursiveLock or a Dispatch Semaphore and I was just wondering if there was a specific reason and what were the benefits.

EDIT: Just wanted to add more info on the question, so for instance, cant we change the static property private static let lock = ResolverRecursiveLock() to private static let lock = NSRecursiveLock() and be done with it (keeping the lock.lock() and lock.unlock()) without the use of the POSIX Mutex lock?

Cheers!

hmlongco commented 3 years ago

It's primarily an attempt to eek out just a bit more performance in something that's done on every registration/resolution.

NSLock and NSRecursiveLock are built on pthread_mutex anyway, but the biggest difference is NSLock/NSRecursiveLock are NSObject-based, which means using msg_send which incurs a performance hit. The lock object used in Resolver is a final Swift class with a recursive mutex and with the lock/unlock functions marked as @inline(__always).

private final class ResolverRecursiveLock {
    init() {
        pthread_mutexattr_init(&recursiveMutexAttr)
        pthread_mutexattr_settype(&recursiveMutexAttr, PTHREAD_MUTEX_RECURSIVE)
        pthread_mutex_init(&recursiveMutex, &recursiveMutexAttr)
    }
    @inline(__always)
    func lock() {
        pthread_mutex_lock(&recursiveMutex)
    }
    @inline(__always)
    func unlock() {
        pthread_mutex_unlock(&recursiveMutex)
    }
    private var recursiveMutex = pthread_mutex_t()
    private var recursiveMutexAttr = pthread_mutexattr_t()
}
gmogames commented 3 years ago

Thank you for the clarification. I've been reading more about it and yes, it does seem the using NSRecursiveLock has a small increase in performance, but it seems a very small amount. I personally changed in my code to use the NSRecursiveLock mostly for also removing the @inline(__always) as this tweet from Jordan Rose always comes to mind: image

I do appreciate your work and your response to my question, and you have helped my project a lot already.

Thank you once again.

hmlongco commented 3 years ago

Just noting that I replaced my lock with NSRecursiveLock and ran my multi-threading test w/o any issues or failures, so you should be good to go.