Repository navigation
perf(log): opt-in background writer for logger and access-log lines - #370
Merged
Merged
Conversation
cwist_logger_* wrote each line with three fprintf calls and an fflush, and the access-log middleware printf'd under a global mutex, both on the request thread. With 8 threads logging, a line cost the caller ~12.5 us. cwist_log_async_start() starts one writer thread with a byte ring per stream (stdout, stderr). A caller formats the whole line on its stack, copies it into the ring under one mutex and returns; the writer does the write() calls. Lines stay whole and in order per stream, a full ring makes the caller wait (nothing is dropped), and a line larger than the ring, or any line while the writer is off, is written synchronously as before. pthread_atfork gives forked workers their own writer, an atexit hook writes what is still queued, and WASM builds get stubs that never start. The logger also caches its timestamp per thread per second: localtime_r() takes glibc's timezone lock on every call and was the largest cost left once many threads log. Logging stays synchronous unless cwist_log_async_start() is called. New API: cwist_log_async_start/flush/stop/active (baseline regenerated).
3 tasks done
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.
Summary
Logging on the request path cost a lot once several threads logged at once.
cwist_logger_*wrote each line with threefprintfcalls and anfflush, so every line was at least one syscall.printfunder a global mutex.This PR adds an opt-in background writer. Logging stays synchronous unless the app calls
cwist_log_async_start().Changes
include/cwist/sys/app/logger.h):cwist_log_async_start(size_t),cwist_log_async_flush(),cwist_log_async_stop(),cwist_log_async_active(). The API baseline is regenerated with additions only; no existing struct or signature changes.src/sys/app/log_async.c:write()calls.pthread_atfork: a forked child (CWIST_WORKERS) drops its copy of the parent's queue and starts its own writer on its first line.atexitwrites whatever is still queued.startreturns false.logger.c: builds the whole line once, then queues it or writes it with onefwrite. The output format is unchanged. The timestamp is cached per thread per second:localtime_r()takes glibc's timezone lock on every call and was the largest cost left with 8 threads.middleware.c: the three access-log formats go through one helper,access_log_printf(), which queues the line orfwrites it under the existing mutex.docs/api/middleware.mdanddocs/ko/api/middleware.mdget an "Async log output" section.Measurements
Local machine, 12 cores. Each thread logs one ~70-byte line repeatedly to a file. Per-line cost is wall time per line as each calling thread sees it.
Tests
tests/test_log_async.c, wired intoTEST_TARGETS. It covers:stopdrains the queue, and logging afterstopis synchronous again.exit(), and the parent's queued line is written exactly once.die_after_fork=0, because TSan otherwise refuses to start a thread after a multi-threaded fork, which is exactly the worker case this tests.libcwist.awith the three changed files linked first):test_log_async: 50 runs, all passed.test_log_asyncunder TSan (changed files and test instrumented): 30 runs, no failures and no race reports.test_logger_configandtest_access_log: pass.check_api_baseline,check_api_test_coverage,check_env_knobs,check_test_wiring,malloc_lintandclang-formatall pass.make testsuite. CI covers it.