git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 8/9] trace2: add counter events to perf and event target formats

From
JHJeff Hostetler <git@jeffhostetler.com>
Date
Dec 22, 2021, 22:56 UTC
Message-ID
<fe3e9ceb-428e-1f20-9f90-37ad378f50b8@jeffhostetler.com>
In-Reply-To
<211220.86czlrurm6.gmgdl@evledraar.gmail.com>
On 12/20/21 11:51 AM, Ævar Arnfjörð Bjarmason wrote:
Show 19 quoted lines
> 
> On Mon, Dec 20 2021, Jeff Hostetler via GitGitGadget wrote:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>> [...]
> 
>> +static void fn_counter(uint64_t us_elapsed_absolute,
>> +		       const char *thread_name,
>> +		       const char *category,
>> +		       const char *counter_name,
>> +		       uint64_t value)
>> +{
>> +	const char *event_name = "counter";
>> +	struct strbuf buf_payload = STRBUF_INIT;
>> +
>> +	strbuf_addf(&buf_payload, "name:%s", counter_name);
>> +	strbuf_addf(&buf_payload, " value:%"PRIu64, value);
> 
> Odd to have these be two seperate strbuf_addf()...
yeah, i'll combine.  and in the body of fn_timer in 6/9.
> ....but more generally, and I see from e.g. the existing fn_version_fl
> that you're just using existing patterns, but it seems odd not to have a
> trivial varargs fmt helper for perf_io_write_fl that would avoid the
> whole strbuf/addf/release dance.
[...]

yeah, cut-n-paste was used here and i was maintaining consistency with the other functions -- rather than inventing something new and refactoring stuff that didn't need be refactored in the middle of an on-going patch series.

Show 6 quoted lines
> I did a quick experiment to do that, patch on "master" below. A lot of
> the boilerplate could be simplified by factoring out the
> sq_quote_buf_pretty() case, and even this approach (re)allocs in a way
> that looks avoidable in many cases if perf_fmt_prepare() were improved
> (but it looks like it nedes its if/while loops in some cases still):
> 
[...]
Show 35 quoted lines
>   
> +__attribute__((format (printf, 8, 9)))
> +static void perf_io_write_fl_fmt(const char *file, int line, const char *event_name,
> +				 const struct repository *repo,
> +				 uint64_t *p_us_elapsed_absolute,
> +				 uint64_t *p_us_elapsed_relative,
> +				 const char *category,
> +				 const char *fmt, ...)
> +{
> +	va_list ap;
> +	struct strbuf sb = STRBUF_INIT;
> +
> +	va_start(ap, fmt);
> +	strbuf_vaddf(&sb, fmt, ap);
> +	va_end(ap);
> +
> +	perf_io_write_fl(file, line, event_name, repo, p_us_elapsed_absolute,
> +			 p_us_elapsed_relative, category, &sb);
> +
> +	strbuf_release(&sb);
> +}
> +
>   static void fn_version_fl(const char *file, int line)
>   {
>   	const char *event_name = "version";
> -	struct strbuf buf_payload = STRBUF_INIT;
> -
> -	strbuf_addstr(&buf_payload, git_version_string);
>   
> -	perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,
> -			 &buf_payload);
> -	strbuf_release(&buf_payload);
> +	perf_io_write_fl_fmt(file, line, event_name, NULL, NULL, NULL, NULL,
> +			     "%s", git_version_string);
>   }
[...]

Yes, it might be nice to have a _fmt() version as you suggest and simplify many of the existing fn_*() function bodies.

It seems like I keep saying this today, but can we discuss that in a new top-level topic and not down inside commit 8/9 of this series?

Thanks, Jeff

Previous: Ævar Arnfjörð BjarmasonNext: Jeff Hostetler via GitGitGadget
Message 28 of 55 in “Trace2 stopwatch timers and global counters”
  1. 0/9 Trace2 stopwatch timers and global countersJeff Hostetler via GitGitGadget, Dec 20, 2021
  2. 1/9 trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctxJeff Hostetler via GitGitGadget, Dec 20, 2021
  3. 2/9 trace2: convert tr2tls_thread_ctx.thread_name from strbuf to char*Jeff Hostetler via GitGitGadget, Dec 20, 2021
  4. Ævar Arnfjörð BjarmasonDec 20, 2021
  5. Jeff HostetlerDec 20, 2021
  6. Ævar Arnfjörð BjarmasonDec 20, 2021
  7. Jeff HostetlerDec 22, 2021
  8. Junio C HamanoDec 21, 2021
  9. Junio C HamanoDec 21, 2021
  10. Jeff HostetlerDec 22, 2021
  11. Junio C HamanoDec 22, 2021
  12. 3/9 trace2: defer free of TLS CTX until program exit.Jeff Hostetler via GitGitGadget, Dec 20, 2021
  13. Junio C HamanoDec 21, 2021
  14. Jeff HostetlerDec 22, 2021
  15. Junio C HamanoDec 22, 2021
  16. Jeff HostetlerDec 22, 2021
  17. Johannes SixtDec 23, 2021
  18. Junio C HamanoDec 23, 2021
  19. Jeff HostetlerDec 27, 2021
  20. 4/9 trace2: add thread-name override to event targetJeff Hostetler via GitGitGadget, Dec 20, 2021
  21. 5/9 trace2: add thread-name override to perf targetJeff Hostetler via GitGitGadget, Dec 20, 2021
  22. 6/9 trace2: add timer events to perf and event target formatsJeff Hostetler via GitGitGadget, Dec 20, 2021
  23. Ævar Arnfjörð BjarmasonDec 20, 2021
  24. Jeff HostetlerDec 20, 2021
  25. Derrick StoleeDec 21, 2021
  26. 8/9 trace2: add counter events to perf and event target formatsJeff Hostetler via GitGitGadget, Dec 20, 2021
  27. Ævar Arnfjörð BjarmasonDec 20, 2021
  28. Jeff HostetlerDec 22, 2021
  29. 7/9 trace2: add stopwatch timersJeff Hostetler via GitGitGadget, Dec 20, 2021
  30. Ævar Arnfjörð BjarmasonDec 20, 2021
  31. Jeff HostetlerDec 22, 2021
  32. Derrick StoleeDec 21, 2021
  33. Jeff HostetlerDec 22, 2021
  34. 9/9 trace2: add global countersJeff Hostetler via GitGitGadget, Dec 20, 2021
  35. Ævar Arnfjörð BjarmasonDec 20, 2021
  36. Jeff HostetlerDec 22, 2021
  37. Derrick StoleeDec 21, 2021
  38. Matheus TavaresDec 21, 2021
  39. 0/9 Trace2 stopwatch timers and global countersJeff Hostetler via GitGitGadget, Dec 28, 2021
  40. 1/9 trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctxJeff Hostetler via GitGitGadget, Dec 28, 2021
  41. Ævar Arnfjörð BjarmasonDec 29, 2021
  42. 2/9 trace2: convert tr2tls_thread_ctx.thread_name from strbuf to flex arrayJeff Hostetler via GitGitGadget, Dec 28, 2021
  43. Ævar Arnfjörð BjarmasonDec 29, 2021
  44. Jeff HostetlerDec 29, 2021
  45. 3/9 trace2: defer free of thread local storage until program exit.Jeff Hostetler via GitGitGadget, Dec 28, 2021
  46. 4/9 trace2: add thread-name override to event targetJeff Hostetler via GitGitGadget, Dec 28, 2021
  47. 7/9 trace2: add stopwatch timersJeff Hostetler via GitGitGadget, Dec 28, 2021
  48. 5/9 trace2: add thread-name override to perf targetJeff Hostetler via GitGitGadget, Dec 28, 2021
  49. Ævar Arnfjörð BjarmasonDec 29, 2021
  50. Jeff HostetlerDec 29, 2021
  51. 6/9 trace2: add timer events to perf and event target formatsJeff Hostetler via GitGitGadget, Dec 28, 2021
  52. 8/9 trace2: add counter events to perf and event target formatsJeff Hostetler via GitGitGadget, Dec 28, 2021
  53. 9/9 trace2: add global countersJeff Hostetler via GitGitGadget, Dec 28, 2021
  54. Ævar Arnfjörð BjarmasonDec 29, 2021
  55. Jeff HostetlerDec 30, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.