git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff Hostetler <git@jeffhostetler.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, peff@peff.net, jrnieder@gmail.com,
	steadmon@google.com, Junio C Hamano <gitster@pobox.com>,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH v2 5/7] trace2: report peak memory usage of the process
Date: Mon, 1 Apr 2019 17:05:09 -0400	[thread overview]
Message-ID: <ded19f10-569a-f6d7-ad77-17a6f9c6b3a5@jeffhostetler.com> (raw)
In-Reply-To: <87k1gh9vii.fsf@evledraar.gmail.com>



On 3/29/2019 6:16 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> 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).
> 

Thanks,

Yeah, I've avoided adding the corresponding platform-specific changes
in for non-Windows versions because I didn't want to venture off into
the weeds and introduce a perf or functional problem (for something
that I'm not going to make use of in the short term).  Whereas, we're
seeing occasional perf problems with our Windows users on the OS repo
where some (yet to be determined) commands will just go out to lunch.

I suspect we'll have similar problems with out Mac users in the near
future, but I didn't want to jump in there just yet.

Jeff




  reply	other threads:[~2019-04-01 21:05 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-28 13:30 [PATCH 0/4] trace2: load trace2 settings from system config Jeff Hostetler via GitGitGadget
2019-03-28 13:30 ` [PATCH 1/4] trace2: refactor setting process starting time Jeff Hostetler via GitGitGadget
2019-03-28 13:31 ` [PATCH 2/4] trace2: add absolute elapsed time to start event Jeff Hostetler via GitGitGadget
2019-03-28 13:31 ` [PATCH 3/4] trace2: find exec-dir before trace2 initialization Jeff Hostetler via GitGitGadget
2019-03-28 13:31 ` [PATCH 4/4] trace2: use system config for default trace2 settings Jeff Hostetler via GitGitGadget
2019-03-28 14:36   ` Ævar Arnfjörð Bjarmason
2019-03-28 18:50     ` Jeff Hostetler
2019-03-28 21:28   ` Josh Steadmon
2019-03-29 17:04 ` [PATCH v2 0/7] trace2: load trace2 settings from system config Jeff Hostetler via GitGitGadget
2019-03-29 17:04   ` [PATCH v2 1/7] trace2: refactor setting process starting time Jeff Hostetler via GitGitGadget
2019-03-29 17:04   ` [PATCH v2 2/7] trace2: add absolute elapsed time to start event Jeff Hostetler via GitGitGadget
2019-03-29 17:04   ` [PATCH v2 3/7] trace2: find exec-dir before trace2 initialization Jeff Hostetler via GitGitGadget
2019-03-29 17:04   ` [PATCH v2 4/7] trace2: use system config for default trace2 settings Jeff Hostetler via GitGitGadget
2019-04-01 21:00     ` Josh Steadmon
2019-04-01 21:06       ` Jeff Hostetler
2019-04-03  0:01       ` Jonathan Nieder
2019-04-03  0:00     ` Jonathan Nieder
2019-04-09 15:58       ` Jeff Hostetler
2019-03-29 17:04   ` [PATCH v2 5/7] trace2: report peak memory usage of the process Jeff Hostetler via GitGitGadget
2019-03-29 22:16     ` Ævar Arnfjörð Bjarmason
2019-04-01 21:05       ` Jeff Hostetler [this message]
2019-03-29 17:04   ` [PATCH v2 7/7] trace2: make SIDs more unique Jeff Hostetler via GitGitGadget
2019-03-29 22:12     ` Ævar Arnfjörð Bjarmason
2019-04-01 21:16       ` Jeff Hostetler
2019-03-29 17:04   ` [PATCH v2 6/7] trace2: clarify UTC datetime formatting Jeff Hostetler via GitGitGadget
2019-04-01 21:02   ` [PATCH v2 0/7] trace2: load trace2 settings from system config Josh Steadmon
2019-04-11 15:18   ` [PATCH v3 00/10] " Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 01/10] config: initialize opts structure in repo_read_config() Jeff Hostetler via GitGitGadget
2019-04-12  3:52       ` Jonathan Nieder
2019-04-15 14:34         ` Johannes Schindelin
2019-04-11 15:18     ` [PATCH v3 03/10] trace2: add absolute elapsed time to start event Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 02/10] trace2: refactor setting process starting time Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 04/10] trace2: find exec-dir before trace2 initialization Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 05/10] config: add read_very_early_config() Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 06/10] trace2: use system/global config for default trace2 settings Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 07/10] trace2: report peak memory usage of the process Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 08/10] trace2: clarify UTC datetime formatting Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 09/10] trace2: make SIDs more unique Jeff Hostetler via GitGitGadget
2019-04-11 15:18     ` [PATCH v3 10/10] trace2: update docs to describe system/global config settings Jeff Hostetler via GitGitGadget
2019-04-12  2:29     ` [PATCH v3 00/10] trace2: load trace2 settings from system config Junio C Hamano
2019-04-12 13:47       ` Jeff Hostetler
2019-04-15 20:39     ` [PATCH v4 " Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 01/10] config: initialize opts structure in repo_read_config() Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 02/10] trace2: refactor setting process starting time Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 03/10] trace2: add absolute elapsed time to start event Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 04/10] trace2: find exec-dir before trace2 initialization Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 05/10] config: add read_very_early_config() Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 06/10] trace2: use system/global config for default trace2 settings Jeff Hostetler via GitGitGadget
2019-04-27 13:43         ` SZEDER Gábor
2019-04-29 19:03           ` Jeff Hostetler
2019-04-15 20:39       ` [PATCH v4 07/10] trace2: report peak memory usage of the process Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 08/10] trace2: clarify UTC datetime formatting Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 09/10] trace2: make SIDs more unique Jeff Hostetler via GitGitGadget
2019-04-15 20:39       ` [PATCH v4 10/10] trace2: update docs to describe system/global config settings Jeff Hostetler via GitGitGadget
2019-04-29 20:14       ` [PATCH v5 00/11] trace2: load trace2 settings from system config Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 02/11] trace2: refactor setting process starting time Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 01/11] config: initialize opts structure in repo_read_config() Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 03/11] trace2: add absolute elapsed time to start event Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 04/11] trace2: find exec-dir before trace2 initialization Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 05/11] config: add read_very_early_config() Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 06/11] trace2: use system/global config for default trace2 settings Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 07/11] trace2: report peak memory usage of the process Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 09/11] trace2: make SIDs more unique Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 08/11] trace2: clarify UTC datetime formatting Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 10/11] trace2: update docs to describe system/global config settings Jeff Hostetler via GitGitGadget
2019-04-29 20:14         ` [PATCH v5 11/11] trace2: fixup access problem on /etc/gitconfig in read_very_early_config Jeff Hostetler via GitGitGadget
2019-04-29 20:21         ` [PATCH v5 00/11] trace2: load trace2 settings from system config Jeff Hostetler
2019-05-07  1:18           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ded19f10-569a-f6d7-ad77-17a6f9c6b3a5@jeffhostetler.com \
    --to=git@jeffhostetler.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).