From 60cd1202d9064971f7eeaecb8999b55377f95e6d Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Tue, 10 Mar 2020 10:24:17 -0700 Subject: [PATCH 1/3] [vcpkg] Introduce Job Objects to improve ctrl-c performance on Windows --- toolsrc/include/vcpkg/base/system.process.h | 5 +++ toolsrc/src/vcpkg.cpp | 3 +- toolsrc/src/vcpkg/base/system.process.cpp | 38 ++++++++++++++++++--- toolsrc/src/vcpkg/commands.env.cpp | 24 +++++++++---- 4 files changed, 58 insertions(+), 12 deletions(-) diff --git a/toolsrc/include/vcpkg/base/system.process.h b/toolsrc/include/vcpkg/base/system.process.h index b2e76ff7452..51ea728c346 100644 --- a/toolsrc/include/vcpkg/base/system.process.h +++ b/toolsrc/include/vcpkg/base/system.process.h @@ -61,4 +61,9 @@ namespace vcpkg::System std::function data_cb, const Environment& env = {}); void register_console_ctrl_handler(); +#if defined(_WIN32) + void initialize_global_job_object(); + void enter_interactive_subprocess(); + void exit_interactive_subprocess(); +#endif } diff --git a/toolsrc/src/vcpkg.cpp b/toolsrc/src/vcpkg.cpp index 1f212dd9487..c2ee422005b 100644 --- a/toolsrc/src/vcpkg.cpp +++ b/toolsrc/src/vcpkg.cpp @@ -269,7 +269,7 @@ int wmain(int, const wchar_t* const* const); #include int main(int argc, const char* const* const /*argv*/) { - wchar_t **wargv; + wchar_t** wargv; wargv = CommandLineToArgvW(GetCommandLineW(), &argc); return wmain(argc, wargv); } @@ -292,6 +292,7 @@ int main(const int argc, const char* const* const argv) SetConsoleCP(CP_UTF8); SetConsoleOutputCP(CP_UTF8); + System::initialize_global_job_object(); #endif Checks::register_global_shutdown_handler([]() { diff --git a/toolsrc/src/vcpkg/base/system.process.cpp b/toolsrc/src/vcpkg/base/system.process.cpp index 076a1edbabf..bba29a2d738 100644 --- a/toolsrc/src/vcpkg/base/system.process.cpp +++ b/toolsrc/src/vcpkg/base/system.process.cpp @@ -30,7 +30,7 @@ namespace vcpkg { struct CtrlCStateMachine { - CtrlCStateMachine() : m_number_of_external_processes(0) {} + CtrlCStateMachine() : m_number_of_external_processes(0), m_global_job(NULL), m_in_interactive(0) {} void transition_to_spawn_process() noexcept { @@ -91,17 +91,47 @@ namespace vcpkg } else { - // We are currently blocked on a child process. Upon return, transition_from_spawn_process() - // will be called and exit. + // We are currently blocked on a child process. + // If none of the child processes are interactive, use the Job Object to terminate the tree. + if (m_in_interactive.load() == 0) + { + auto job = m_global_job.exchange(NULL); + if (job != NULL) + { + ::CloseHandle(job); + } + } } } + void initialize_job() + { + m_global_job = CreateJobObjectW(NULL, NULL); + if (m_global_job != NULL) + { + JOBOBJECT_EXTENDED_LIMIT_INFORMATION info = {}; + info.BasicLimitInformation.LimitFlags = + JOB_OBJECT_LIMIT_BREAKAWAY_OK | JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + ::SetInformationJobObject(m_global_job, JobObjectExtendedLimitInformation, &info, sizeof(info)); + ::AssignProcessToJobObject(m_global_job, ::GetCurrentProcess()); + } + } + + void enter_interactive() { ++m_in_interactive; } + void exit_interactive() { --m_in_interactive; } + private: std::atomic m_number_of_external_processes; + std::atomic m_global_job; + std::atomic m_in_interactive; }; static CtrlCStateMachine g_ctrl_c_state; } + + void System::initialize_global_job_object() { g_ctrl_c_state.initialize_job(); } + void System::enter_interactive_subprocess() { g_ctrl_c_state.enter_interactive(); } + void System::exit_interactive_subprocess() { g_ctrl_c_state.exit_interactive(); } #endif fs::path System::get_exe_path_of_current_process() @@ -428,7 +458,7 @@ namespace vcpkg { auto timer = Chrono::ElapsedTimer::create_started(); - auto process_info = windows_create_process(cmd_line, {}, DETACHED_PROCESS); + auto process_info = windows_create_process(cmd_line, {}, DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB); if (auto p = process_info.get()) { p->close_handles(); diff --git a/toolsrc/src/vcpkg/commands.env.cpp b/toolsrc/src/vcpkg/commands.env.cpp index 5b673b965bf..8d70ac1447c 100644 --- a/toolsrc/src/vcpkg/commands.env.cpp +++ b/toolsrc/src/vcpkg/commands.env.cpp @@ -48,7 +48,7 @@ namespace vcpkg::Commands::Env const Build::PreBuildInfo pre_build_info( paths, triplet, var_provider.get_generic_triplet_vars(triplet).value_or_exit(VCPKG_LINE_INFO)); const Toolset& toolset = paths.get_toolset(pre_build_info); - auto env_cmd = Build::make_build_env_cmd(pre_build_info, toolset); + auto build_env_cmd = Build::make_build_env_cmd(pre_build_info, toolset); std::unordered_map extra_env = {}; const bool add_bin = Util::Sets::contains(options.switches, OPTION_BIN); @@ -74,12 +74,22 @@ namespace vcpkg::Commands::Env if (add_python) extra_env.emplace("PYTHONPATH", (paths.installed / triplet.to_string() / "python").u8string()); if (path_vars.size() > 0) extra_env.emplace("PATH", Strings::join(";", path_vars)); - std::string env_cmd_prefix = env_cmd.empty() ? "" : Strings::format("%s && ", env_cmd); - std::string env_cmd_suffix = - args.command_arguments.empty() ? "cmd" : Strings::format("cmd /c %s", args.command_arguments.at(0)); + auto env = [&] { + auto clean_env = System::get_modified_clean_environment(extra_env); + if (build_env_cmd.empty()) + return clean_env; + else + return System::cmd_execute_modify_env(build_env_cmd, clean_env); + }(); - const std::string cmd = Strings::format("%s%s", env_cmd_prefix, env_cmd_suffix); - System::cmd_execute(cmd, System::get_modified_clean_environment(extra_env)); - Checks::exit_success(VCPKG_LINE_INFO); + std::string cmd = args.command_arguments.empty() ? "cmd" : args.command_arguments.at(0); +#ifdef _WIN32 + System::enter_interactive_subprocess(); +#endif + auto rc = System::cmd_execute(cmd, env); +#ifdef _WIN32 + System::exit_interactive_subprocess(); +#endif + Checks::exit_with_code(VCPKG_LINE_INFO, rc); } } From e2efb82aa8cf2f250bfe97386eb15abb0b71cbce Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Fri, 13 Mar 2020 21:00:21 -0700 Subject: [PATCH 2/3] [vcpkg] Fix build for non-windows --- toolsrc/src/vcpkg/commands.env.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/toolsrc/src/vcpkg/commands.env.cpp b/toolsrc/src/vcpkg/commands.env.cpp index 8d70ac1447c..c5cab46f462 100644 --- a/toolsrc/src/vcpkg/commands.env.cpp +++ b/toolsrc/src/vcpkg/commands.env.cpp @@ -79,7 +79,14 @@ namespace vcpkg::Commands::Env if (build_env_cmd.empty()) return clean_env; else + { +#ifdef _WIN32 return System::cmd_execute_modify_env(build_env_cmd, clean_env); +#else + Checks::exit_with_message(VCPKG_LINE_INFO, + "Build environment commands are not supported on this platform"); +#endif + } }(); std::string cmd = args.command_arguments.empty() ? "cmd" : args.command_arguments.at(0); From 26949af8f8ae311e94198c8c1bd2d93578809630 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Tue, 17 Mar 2020 13:11:15 -0700 Subject: [PATCH 3/3] [vcpkg-env] Restore previous behavior of `cmd /c ` --- toolsrc/src/vcpkg/commands.env.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolsrc/src/vcpkg/commands.env.cpp b/toolsrc/src/vcpkg/commands.env.cpp index c5cab46f462..ca398068f32 100644 --- a/toolsrc/src/vcpkg/commands.env.cpp +++ b/toolsrc/src/vcpkg/commands.env.cpp @@ -89,7 +89,7 @@ namespace vcpkg::Commands::Env } }(); - std::string cmd = args.command_arguments.empty() ? "cmd" : args.command_arguments.at(0); + std::string cmd = args.command_arguments.empty() ? "cmd" : ("cmd /c " + args.command_arguments.at(0)); #ifdef _WIN32 System::enter_interactive_subprocess(); #endif