Re: [PATCH v2 5/7] trace2: report peak memory usage of the process

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Mar 29 2019, Jeff Hostetler via GitGitGadget wrote:

> 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)
>  #endif

FWIW 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).