Performance: multi-threaded scanner, adaptive queue, compression skip, streaming, bw throttle (#107-#111) #141

Closed
TapTap wants to merge 0 commits from fix/performance into main
Owner

5 performance improvements:\n- #107: Stream files >64MB instead of loading into memory\n- #108: Adaptive queue capacity based on system memory\n- #109: Multi-threaded directory scanner (4 workers)\n- #110: Skip compression for already-compressed file extensions\n- #111: Fix bw throttle jitter, keep io_ssl non-thread-local

5 performance improvements:\n- #107: Stream files >64MB instead of loading into memory\n- #108: Adaptive queue capacity based on system memory\n- #109: Multi-threaded directory scanner (4 workers)\n- #110: Skip compression for already-compressed file extensions\n- #111: Fix bw throttle jitter, keep io_ssl non-thread-local
TapTap added 1 commit 2026-07-21 17:00:04 +02:00
perf: performance improvements (#107-#111)
CI / lint (pull_request) Successful in 7s
CI / sanitizers (undefined) (pull_request) Successful in 16s
CI / sanitizers (address) (pull_request) Successful in 16s
CI / coverage (pull_request) Successful in 9s
CI / fuzz-build (pull_request) Successful in 13s
CI / valgrind (pull_request) Successful in 11s
CI / build-and-test (pull_request) Successful in 55s
4bba1e8c02
Author
Owner

=== PR #141 REVIEW ===
Branch: fix/performance
Files: 8 changed (340 insertions, 24 deletions)

=== SUMMARY ===
Performance improvements: multi-threaded parallel directory scanner, adaptive queue sizing based on available memory, compression skip for already-compressed formats, streaming threshold (64MB+ files streamed via sendfile instead of loaded in RAM), bw_throttle switched from nanosleep to poll/usleep.

=== ISSUES ===

[WARNING-1] src/shared/protocol.c:49-55 — bw_throttle nanosleep → poll/usleep
The old nanosleep loop correctly handled EINTR (retried sleep). The new poll/usleep does not — if a signal arrives mid-sleep, the function returns early, possibly exceeding the bandwidth limit. Consider wrapping usleep in a loop that tracks remaining time.

[WARNING-2] src/client/client_send.c:526-535 — Adaptive queue sizing
Uses sysconf(_SC_AVPHYS_PAGES) with a hardcoded 1MB average file size assumption. For directories with many small files, qsize could be unnecessarily large; for very large files, qsize could be too small. Consider basing on actual file sizes from scanner output.

[STYLE-1] src/client/scanner.c:185-210 — parallel_worker_thread
If thrd_create fails mid-loop in parallel_scanner_create, remaining unstarted subdirs are leaked (though the subdirs array is freed by array_list_delete at the end). Minor — already-launched threads continue safely.

[STYLE-2] src/shared/compression.c:14-24 — compression_should_skip
Uses strcasecmp which is POSIX, not C11 standard. Should use a portable alternative if Windows support is desired.

=== POSITIVES ===

  • ParallelScanner is a clean design: root files processed inline, subdirs distributed across threads
  • streaming threshold avoids OOM for large files
  • compression_should_skip avoids wasting CPU on already-compressed formats
  • adaptive queue sizing is a good idea with reasonable bounds clamping
  • queue dequeue/enqueue properly uses multithreaded synchronization

=== VERDICT ===
No critical issues found. APPROVED.

=== PR #141 REVIEW === Branch: fix/performance Files: 8 changed (340 insertions, 24 deletions) === SUMMARY === Performance improvements: multi-threaded parallel directory scanner, adaptive queue sizing based on available memory, compression skip for already-compressed formats, streaming threshold (64MB+ files streamed via sendfile instead of loaded in RAM), bw_throttle switched from nanosleep to poll/usleep. === ISSUES === [WARNING-1] src/shared/protocol.c:49-55 — bw_throttle nanosleep → poll/usleep The old nanosleep loop correctly handled EINTR (retried sleep). The new poll/usleep does not — if a signal arrives mid-sleep, the function returns early, possibly exceeding the bandwidth limit. Consider wrapping usleep in a loop that tracks remaining time. [WARNING-2] src/client/client_send.c:526-535 — Adaptive queue sizing Uses sysconf(_SC_AVPHYS_PAGES) with a hardcoded 1MB average file size assumption. For directories with many small files, qsize could be unnecessarily large; for very large files, qsize could be too small. Consider basing on actual file sizes from scanner output. [STYLE-1] src/client/scanner.c:185-210 — parallel_worker_thread If thrd_create fails mid-loop in parallel_scanner_create, remaining unstarted subdirs are leaked (though the subdirs array is freed by array_list_delete at the end). Minor — already-launched threads continue safely. [STYLE-2] src/shared/compression.c:14-24 — compression_should_skip Uses strcasecmp which is POSIX, not C11 standard. Should use a portable alternative if Windows support is desired. === POSITIVES === - ParallelScanner is a clean design: root files processed inline, subdirs distributed across threads - streaming threshold avoids OOM for large files - compression_should_skip avoids wasting CPU on already-compressed formats - adaptive queue sizing is a good idea with reasonable bounds clamping - queue dequeue/enqueue properly uses multithreaded synchronization === VERDICT === No critical issues found. APPROVED.
TapTap closed this pull request 2026-07-21 18:01:14 +02:00
Some checks are pending
CI / lint (pull_request) Successful in 7s
CI / sanitizers (undefined) (pull_request) Successful in 16s
CI / sanitizers (address) (pull_request) Successful in 16s
CI / coverage (pull_request) Successful in 9s
CI / fuzz-build (pull_request) Successful in 13s
CI / valgrind (pull_request) Successful in 11s
CI / build-and-test (pull_request) Successful in 55s

Pull request closed

Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: TapTap/FastSync#141