Conversation
Each worker owns one input list, and SingleProducerSingleConsumerList only lets a consumer take an item once another item has been produced behind it (or the producer has finished). Reader backpressure used a fixed PACK_IN_MEM_LIMIT (32) that is smaller than the worker count on machines with more than 32 cores. With 33+ workers the readers stop after 33 packs, before any worker list holds two items, so no pack is ever consumed and all reader and worker threads wait on the backpressure condition variable forever. The writer-backlog check has the same shape: the writer drains worker lists round-robin and waits on a list until its worker produces a second output, so a fixed limit below the worker count can also block the readers permanently. Allow at least two in-flight packs per worker in both the reader/processor and reader/writer backpressure checks (max(PACK_IN_MEM_LIMIT, 2 * threads)), for PE, interleaved PE and SE readers. The lock-free list semantics are unchanged, so this does not reintroduce OpenGene#695.
dougnukem
marked this pull request as ready for review
September 25, 2026 17:55
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #721.
Summary
With more than 32 worker threads (
-w 33or higher, on a machine with at least 33 cores), fastp 1.3.x can hang forever right after startup / adapter detection. All reader and worker threads sit inmBackpressureCV.wait_for, no output is written, and CPU usage is near idle.This PR makes the reader backpressure limits scale with the worker thread count, so readers always let each worker receive a pack it can consume. It does not touch
SingleProducerSingleConsumerList, so the #695 fix (b402d71) stays as it is.Root cause
PACK_IN_MEM_LIMIT(32) packs ahead of processing (peprocessor.cppreaderTask/interleavedReaderTask,seprocessor.cppreaderTask).SingleProducerSingleConsumerListonly becomes consumable once another item is produced behind it, or once the producer finishes. So each worker needs two packs in its list before it can start.T > PACK_IN_MEM_LIMITworkers, the readers stop after 33 packs, before any list has a second item. Workers wait for input and readers wait for workers, so nothing makes progress.bufferLength() > PACK_IN_MEM_LIMITcan also block the readers permanently whenT > 32.-wathardware_concurrency(), so the hang only shows up on machines with at least 33 cores. That's probably why it looks machine-dependent in the reports.Stack traces of a hung 1.3.6 process (
-w 48, 48-vCPU VM; official binary, with symbols):Fix
packInMemLimit(threads) = max(PACK_IN_MEM_LIMIT, 2 * threads)is used for both the reader/processor and the reader/writer backpressure checks. This applies to the PE reader, the interleaved-PE reader and the SE reader.Memory stays bounded and grows only with the thread count: 2 packs of 1000 reads per worker. For example, at
-w 48about 96k reads can be in flight per reader, up from 32k. At 16 threads or fewer the limits are unchanged; between 17 and 32 threads they grow as 2 × threads.Validation
Tested on a 48-vCPU AMD EPYC 7B13 VM running Ubuntu 24.04. Both builds came from source at 8a2397b, with and without this patch. Runs were killed if not finished after 120 s; normal runs take 3–16 s.
--detect_adapter_for_pe-m -c), uncompressed output,--stdout--interleaved_in)Output equivalence: for every mode and dataset, the sorted decompressed output records (md5 per output file) and the JSON after-filtering read/base counts are identical across this PR at
-w 1,-w 16and-w 48, master at-w 16, and official 1.3.2 and 1.1.0.Performance (SRR891268 subset,
-w 48, median of 3): this PR 4.4 s, 1.3.2 4.0 s, 1.1.0 5.9 s.ThreadSanitizer (
-O1 -fsanitize=thread, 200k-pair synthetic input, run undersetarch -Rbecause TSan aborts with "unexpected memory mapping" on high-entropy ASLR kernels): this PR adds no new reports.-w 16and-w 32completes with the same set of warnings as this PR at-w 16/-w 32/-w 33/-w 48, all in existing code:ReadPool::input/updateFullStatus(readpool.cpp:23/27/53),SingleProducerSingleConsumerListsize()/produce()/consume()(singleproducersingleconsumerlist.h:90–121), andWriterThread::setInputCompletedPwrite(writerthread.cpp:82).-w 33or higher because it deadlocks.:121) only shows up at 33+ threads, i.e. on the code path that the deadlock currently prevents from running.Reproduce
Any machine with at least 33 cores:
Single-end input also hangs at
-w 48(reproduced with synthetic reads).From the code, 1.3.0–1.3.1 (no first-item fix yet) and 1.3.4–1.3.5 (after b402d71) should behave the same; 1.3.2–1.3.3 include 19602ae (1.3.2 completed at
-w 48in my tests; 1.3.3 wasn't tested). Workaround for affected releases:-w 32or lower.