xzfc / cached-nix-shell

Instant startup time for nix-shell
https://xzfc.github.io/cached-nix-shell/cached-nix-shell.1
The Unlicense
195 stars 16 forks source link

Add macOS support. #25

Closed uri-canva closed 1 year ago

uri-canva commented 1 year ago

Using dyld interposing as documented in: https://opensource.apple.com/source/dyld/dyld-97.1/include/mach-o/dyld-interposing.h.auto.html Following the example here: https://github.com/NixOS/nixpkgs/blob/9d452bdebedb1c47adaabd40e61a742f9f07b54c/pkgs/build-support/libredirect/libredirect.c

I've unconditionally passed both DYLD_INSERT_LIBRARIES and LD_PRELOAD, reasoning that whichever runtime linker we're using will ignore the environment variable it doesn't use, and we'll unset both in the initializer anyway.

uri-canva commented 1 year ago

@lilyball sorry for the unsolicited ping, but I've seen you participate in some of the issues around faster direnv / nix flakes evaluation caching on darwin. Would you be interested in helping review this PR?

uri-canva commented 1 year ago

ping @xzfc

uri-canva commented 1 year ago

ping @xzfc

lilyball commented 1 year ago

I don't use cached-nix-shell (I hadn't heard of it until now, and my normal driver is nix-direnv with flakes which already caches nicely), but I took a quick skim through this. I haven't used DYLD_INSERT_LIBRARIES before, or dyld interposing, but I'm aware of both and the idea looks okay.

However, I don't understand why you're using a dispatch_semaphore_t as a mutex. A dispatch semaphore is not as good as an actual mutex, because it can't do priority donation. macOS's pthread mutex is perfectly fine, and I believe it even defaults to being unfair these days in case this matters (you can set this with pthread_mtuexattr_setpolicy_np(&attr, PTHREAD_MUTEX_POLICY_FIRSTFIT_NP) if you so desire, and <pthread/pthread_spis.h> has the handy-looking PTHREAD_FIRSTFIT_MUTEX_INITIALIZER, but I don't believe this is necessary). macOS also has os_unfair_lock() which is like a spinlock but does priority donation, but since this code was already using a pthread mutex I assume you don't want to replace it with a spinlock-type lock.

The libdispatch equivalent for a mutex is actually a serial dispatch_queue_t, with blocks enqueued using dispatch_sync() (if you can use Apple's C blocks) or dispatch_sync_f(), but that's a bigger change and has no benefit over just using pthread_mutex anyway in the purely synchronous case.

I do think splitting the one mutex into two is a good idea so you can avoid recursion, although the alternative would have been to split print_log() into a separate print_log_locked() that you can call if you already have the mutex. macOS does define PTHREAD_RECURSIVE_MUTEX_INITIALIZER (see the #if conditions though), but avoiding recursive mutex shoudl be a performance win anyway.

xzfc commented 1 year ago

Thanks.

NorthIsUp commented 1 year ago

new release please?