From 8dddc62aeb636c302de3876607027c7b48a94ba0 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sun, 30 Dec 2018 19:39:59 -0600 Subject: [PATCH] Optimize ASSERT_IS_NOT_FORKED_CHILD() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use `pthread_atfork()` to mark child processes as dirty when `fork()` is invoked rather than needing to call into the kernel each time `ASSERT_IS_NOT_FORKED_CHILD()` is called. This makes simple test cases that hit `ASSERT_IS_NOT_FORKED_CHILD()` 1.8x faster. ------------------------ With a7998c4829a80269bcd6af941812e35c86bf49e2 reverted but before this optimization: ``` mqudsi@ZBOOK ~/r/fish-shell> hyperfine -S build/fish 'for i in (seq 100000); test 1 = 1; end' Benchmark #1: for i in (seq 100000); test 1 = 1; end Time (mean ± σ): 717.8 ms ± 14.9 ms [User: 503.4 ms, System: 216.2 ms] Range (min … max): 692.3 ms … 740.2 ms ``` With a7998c4829a80269bcd6af941812e35c86bf49e2 reverted and with this optimization: ``` mqudsi@ZBOOK ~/r/fish-shell> hyperfine -S build/fish 'for i in (seq 100000); test 1 = 1; end' Benchmark #1: for i in (seq 100000); test 1 = 1; end Time (mean ± σ): 397.2 ms ± 22.3 ms [User: 322.1 ms, System: 79.3 ms] Range (min … max): 376.0 ms … 444.0 ms ``` Without a7998c4829a80269bcd6af941812e35c86bf49e2 reverted and with this optimization: mqudsi@ZBOOK ~/r/fish-shell> hyperfine -S build/fish 'for i in (seq 100000); test 1 = 1; end' Benchmark #1: for i in (seq 100000); test 1 = 1; end Time (mean ± σ): 423.4 ms ± 51.6 ms [User: 363.2 ms, System: 61.3 ms] Range (min … max): 378.4 ms … 541.1 ms ``` --- src/common.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/common.cpp b/src/common.cpp index 688ed30d2..527210a1f 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -54,7 +54,11 @@ constexpr wint_t NOT_A_WCHAR = static_cast(WEOF); struct termios shell_modes; +/// This allows us to determine if we're running on the main thread static std::atomic thread_id { 0 }; +/// This allows us to notice when we've forked. +static bool is_forked_proc = false; +/// This allows us to bypass the main thread checks static bool thread_asserts_cfg_for_testing = false; wchar_t ellipsis_char; @@ -2171,20 +2175,13 @@ void set_main_thread() { void configure_thread_assertions_for_testing() { thread_asserts_cfg_for_testing = true; } bool is_forked_child() { - // Just bail if nobody's called setup_fork_guards, e.g. some of our tools. - if (!initial_pid) return false; - - bool is_child_of_fork = (getpid() != initial_pid); - if (is_child_of_fork) { - debug(0, L"Uh-oh: getpid() != initial_pid: %d != %d\n", getpid(), initial_pid); - while (1) sleep(10000); - } - return is_child_of_fork; + return is_forked_proc; } void setup_fork_guards() { - // Notice when we fork by stashing our pid. This seems simpler than pthread_atfork(). - initial_pid = getpid(); + pthread_atfork(nullptr, nullptr, []() { + is_forked_proc = true; + }); } void save_term_foreground_process_group() {