Re: [PATCH v2 5/7] trace2: report peak memory usage of the process
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 29, 2019, 22:16 UTC
- Message-ID
- <87k1gh9vii.fsf@evledraar.gmail.com>
- In-Reply-To
- <86d8fe28cf2f51a494163f9cec3e3b0541288435.1553879063.git.gitgitgadget@gmail.com>
On Fri, Mar 29 2019, Jeff Hostetler via GitGitGadget wrote:
Show 135 quoted lines
> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Teach Windows version of git to report peak memory usage
> during exit() processing.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
> common-main.c | 2 +-
> compat/win32/trace2_win32_process_info.c | 50 ++++++++++++++++++++++--
> trace2.c | 2 +
> trace2.h | 14 +++++--
> 4 files changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/common-main.c b/common-main.c
> index 299ca62a72..582a7b1886 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -41,7 +41,7 @@ int main(int argc, const char **argv)
>
> trace2_initialize();
> trace2_cmd_start(argv);
> - trace2_collect_process_info();
> + trace2_collect_process_info(TRACE2_PROCESS_INFO_STARTUP);
>
> git_setup_gettext();
>
> diff --git a/compat/win32/trace2_win32_process_info.c b/compat/win32/trace2_win32_process_info.c
> index 52bd62034b..2a514caed9 100644
> --- a/compat/win32/trace2_win32_process_info.c
> +++ b/compat/win32/trace2_win32_process_info.c
> @@ -1,5 +1,6 @@
> #include "../../cache.h"
> #include "../../json-writer.h"
> +#include "lazyload.h"
> #include <Psapi.h>
> #include <tlHelp32.h>
>
> @@ -137,11 +138,54 @@ static void get_is_being_debugged(void)
> "windows/debugger_present", 1);
> }
>
> -void trace2_collect_process_info(void)
> +/*
> + * Emit JSON data with the peak memory usage of the current process.
> + */
> +static void get_peak_memory_info(void)
> +{
> + DECLARE_PROC_ADDR(psapi.dll, BOOL, GetProcessMemoryInfo,
> + HANDLE, PPROCESS_MEMORY_COUNTERS, DWORD);
> +
> + if (INIT_PROC_ADDR(GetProcessMemoryInfo)) {
> + PROCESS_MEMORY_COUNTERS pmc;
> +
> + if (GetProcessMemoryInfo(GetCurrentProcess(), &pmc,
> + sizeof(pmc))) {
> + struct json_writer jw = JSON_WRITER_INIT;
> +
> + jw_object_begin(&jw, 0);
> +
> +#define KV(kv) #kv, (intmax_t)pmc.kv
> +
> + jw_object_intmax(&jw, KV(PageFaultCount));
> + jw_object_intmax(&jw, KV(PeakWorkingSetSize));
> + jw_object_intmax(&jw, KV(PeakPagefileUsage));
> +
> + jw_end(&jw);
> +
> + trace2_data_json("process", the_repository,
> + "windows/memory", &jw);
> + jw_release(&jw);
> + }
> + }
> +}
> +
> +void trace2_collect_process_info(enum trace2_process_info_reason reason)
> {
> if (!trace2_is_enabled())
> return;
>
> - get_is_being_debugged();
> - get_ancestry();
> + switch (reason) {
> + case TRACE2_PROCESS_INFO_STARTUP:
> + get_is_being_debugged();
> + get_ancestry();
> + return;
> +
> + case TRACE2_PROCESS_INFO_EXIT:
> + get_peak_memory_info();
> + return;
> +
> + default:
> + BUG("trace2_collect_process_info: unknown reason '%d'", reason);
> + }
> }
> diff --git a/trace2.c b/trace2.c
> index 490b3f071e..6baa65cdf9 100644
> --- a/trace2.c
> +++ b/trace2.c
> @@ -213,6 +213,8 @@ int trace2_cmd_exit_fl(const char *file, int line, int code)
> if (!trace2_enabled)
> return code;
>
> + trace2_collect_process_info(TRACE2_PROCESS_INFO_EXIT);
> +
> tr2main_exit_code = code;
>
> us_now = getnanotime() / 1000;
> diff --git a/trace2.h b/trace2.h
> index 894bfca7e0..888531eb08 100644
> --- a/trace2.h
> +++ b/trace2.h
> @@ -391,13 +391,19 @@ void trace2_printf(const char *fmt, ...);
> * Optional platform-specific code to dump information about the
> * current and any parent process(es). This is intended to allow
> * post-processors to know who spawned this git instance and anything
> - * else the platform may be able to tell us about the current process.
> + * else that the platform may be able to tell us about the current process.
> */
> +
> +enum trace2_process_info_reason {
> + TRACE2_PROCESS_INFO_STARTUP,
> + TRACE2_PROCESS_INFO_EXIT,
> +};
> +
> #if defined(GIT_WINDOWS_NATIVE)
> -void trace2_collect_process_info(void);
> +void trace2_collect_process_info(enum trace2_process_info_reason reason);
> #else
> -#define trace2_collect_process_info() \
> - do { \
> +#define trace2_collect_process_info(reason) \
> + do { \
> } while (0)
> #endifFWIW this is the "VmPeak" line in /proc/$$/status on Linux. I don't know if we can/should parse that in practice (I've been bitten in the past by things in /proc having high cost on some kernel versions in the past, notably smaps).