GoogleChrome / GoogleChrome/ripunzip

Getting no read parallelism on macOS (kernel mutex reading same file in parallel threads)

Open
#126 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
294
Forks
30
PR merge metrics
No merged PRs in 30d

Description

On https://github.com/GoogleChrome/ripunzip/pull/118 I removed a read mutex from ripunzip. From the benchmarking I did on Linux, it made a big difference. But I just tried it at home on macOS on my external SSD formatted with APFS and I found I wasn't getting parallelism.

If I sample from Activity Monitor, I clearly see the 10 threads are spending >90% of their time in `read`. We can't see kernel stacks in the `sample` app so I switched to Instruments.app

Image

Image

I don't have full kernel symbols but it seems somewhat clear what's going on from the symbols we do have:
- We are calling `read`
- The kernel delegates through a few functions (probably generic syscall handlers and then routers to figure out what type of filesystem this is)
- `apfs_vnop_read` is some apfs read function; probably the APFS implementation of read
- `cluster_read_ext` is "documented" here: https://developer.apple.com/documentation/kernel/1463712-cluster_read_ext
- through a few more unsymbolised functions we end up at `lck_mutex_sleep`.

It's surprising. We've opened the source file 10x for read-only, so I thought APFS might use a shared-reader/exclusive-writer lock, but it seems to be assuming all these reads can conflict and using a regular mutex.

There's a 2018 blogpost about APFS not having read parallelism in the specific context of `apfs_vnop_readdir`: https://gregoryszorc.com/blog/2018/10/29/global-kernel-locks-in-apfs/. It's worth a read. They lament that OS filesystem abstractions are holding back SSDs which can do ludicrous IOPS.

> At this time, I haven't conducted a comprehensive analysis of APFS to determine what other filesystem operations seem to acquire global kernel locks: all I know is readdir() does.

Well, I think my profile indicates that `read` holds a lock too. Though I'd guess it's probably not a global kernel lock, but rather a per-file lock.

What options do we have to "fix" this?
1. Make a benchmark to investigate more where this happens (any reads on the same file? overlapping reads only?) and send it to Apple and hope they allow parallel reads in some future macOS version
1. Use mmap? Use memmap2 crate and wrap a Reader around it. The story about mmap in rust is complicated, due to it being unsafe if other apps change the file under you.
1. Try sharing a single file handle, but don't mutate the seek position, use `pread`. I'm not sure if this would also hit a lock
1. Triple-check we're opening the file for 'read' not 'read+write'. I think we are: https://github.com/GoogleChrome/ripunzip/blob/236ed2526e686a46ff6574482195fdf2544e72c8/src/unzip/multi_file_seeker.rs#L25, and https://doc.rust-lang.org/std/fs/struct.File.html#method.open "Attempts to open a file in read-only mode."
1. Disable some parallelism in macOS? Maybe set it to '2' so we can both read and write in parallel? But macOS does support other filesystems, so maybe that's not right. Can we detect APFS?
1. Investigate some more about where the lock is held; perhaps it's held only for overlapping reads, which would only be reading the index metadata; perhaps we could read the zip index metadata once? See https://docs.rs/zip/latest/zip/read/struct.ZipArchive.html#method.unsafe_new_with_metadata

Contributor guide

Open the contributing guide

Research direction

Start at src/unzip/multi_file_seeker.rs and inspect how files are opened and read by the parallel workers. Reproduce the reported behavior on macOS/APFS with the existing benchmark setup, then compare the read strategies listed in the issue. Done means identifying the limiting operation and documenting or implementing a tested direction for restoring useful read parallelism.

Written by the indexing model from the issue text.

Assessment

Tech stack
macos, rust
Domain
operating-systems, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.