git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH 6/9] trace2: convert ctx.thread_name to flex array
Date: Wed, 05 Oct 2022 11:03:00 -0700	[thread overview]
Message-ID: <xmqq7d1eqhbf.fsf@gitster.g> (raw)
In-Reply-To: <6492b6d2b989e08bb539fff3ffe5bdf50fa0a195.1664900407.git.gitgitgadget@gmail.com> (Jeff Hostetler via GitGitGadget's message of "Tue, 04 Oct 2022 16:20:04 +0000")

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Convert the `tr2tls_thread_ctx.thread_name` field from a `strbuf`
> to a "flex array" at the end of the context structure.
>
> The `thread_name` field is a constant string that is constructed when
> the context is created.  Using a (non-const) `strbuf` structure for it
> caused some confusion in the past because it implied that someone
> could rename a thread after it was created.  That usage was not
> intended.  Changing it to a "flex array" will hopefully make the
> intent more clear.

Surely, "const struct strbuf name;" member would be an oxymoron, and
I agree that this should follow "use strbuf as an easy-to-work-with
mechanism to come up with a string, and bake the final value into a
struct as a member of type 'const char []'" pattern.

I recall saying why I thought the flex array was overkill, though.

You have been storing an up-to-24-byte human readable name by
embedding a strbuf that has two size_t plus a pointer (i.e. 24-bytes
even on Windows), and as TR2_MAX_THREAD_NAME is capped at 24 bytes
anyway, an embedded fixed-size thread_name[TR2_MAX_THREAD_NAME+1]
member may be the simplest thing to do, I suspect.

If we were to allow arbitrarily long thread_name[], which may not be
a bad thing to do (e.g. we do not have to worry about truncation
making two names ambiguous, for example), then the flex array is the
right direction to go in, though.

  parent reply	other threads:[~2022-10-05 18:03 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-04 16:19 [PATCH 0/9] Trace2 timers and counters and some cleanup Jeff Hostetler via GitGitGadget
2022-10-04 16:19 ` [PATCH 1/9] builtin/merge-file: fix compiler warning on MacOS with clang 11.0.0 Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 2/9] builtin/unpack-objects.c: " Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 3/9] trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 4/9] tr2tls: clarify TLS terminology Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 5/9] trace2: rename trace2 thread_name argument as name_hint Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 6/9] trace2: convert ctx.thread_name to flex array Jeff Hostetler via GitGitGadget
2022-10-05 11:14   ` Ævar Arnfjörð Bjarmason
2022-10-06 16:28     ` Jeff Hostetler
2022-10-10 18:31     ` Jeff Hostetler
2022-10-05 18:03   ` Junio C Hamano [this message]
2022-10-06 21:05     ` Ævar Arnfjörð Bjarmason
2022-10-06 21:50       ` Junio C Hamano
2022-10-07  1:10         ` [RFC PATCH] trace2 API: don't save a copy of constant "thread_name" Ævar Arnfjörð Bjarmason
2022-10-07  1:16           ` Junio C Hamano
2022-10-07 10:03             ` Ævar Arnfjörð Bjarmason
2022-10-10 19:16               ` Jeff Hostetler
2022-10-11 13:31                 ` Ævar Arnfjörð Bjarmason
2022-10-12 13:31                   ` Jeff Hostetler
2022-10-10 19:05           ` Jeff Hostetler
2022-10-11 12:52             ` Ævar Arnfjörð Bjarmason
2022-10-11 14:40               ` Junio C Hamano
2022-10-10 18:39       ` [PATCH 6/9] trace2: convert ctx.thread_name to flex array Jeff Hostetler
2022-10-04 16:20 ` [PATCH 7/9] api-trace2.txt: elminate section describing the public trace2 API Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 8/9] trace2: add stopwatch timers Jeff Hostetler via GitGitGadget
2022-10-04 16:20 ` [PATCH 9/9] trace2: add global counter mechanism Jeff Hostetler via GitGitGadget
2022-10-05 13:04 ` [PATCH 0/9] Trace2 timers and counters and some cleanup Ævar Arnfjörð Bjarmason
2022-10-06 15:45   ` Jeff Hostetler
2022-10-06 18:12 ` Derrick Stolee
2022-10-12 18:52 ` [PATCH v2 0/7] " Jeff Hostetler via GitGitGadget
2022-10-12 18:52   ` [PATCH v2 1/7] trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx Jeff Hostetler via GitGitGadget
2022-10-12 18:52   ` [PATCH v2 2/7] tr2tls: clarify TLS terminology Jeff Hostetler via GitGitGadget
2022-10-13 21:12     ` Junio C Hamano
2022-10-12 18:52   ` [PATCH v2 3/7] api-trace2.txt: elminate section describing the public trace2 API Jeff Hostetler via GitGitGadget
2022-10-12 18:52   ` [PATCH v2 4/7] trace2: rename the thread_name argument to trace2_thread_start Jeff Hostetler via GitGitGadget
2022-10-12 21:06     ` Ævar Arnfjörð Bjarmason
2022-10-20 14:40       ` Jeff Hostetler
2022-10-13 21:12     ` Junio C Hamano
2022-10-12 18:52   ` [PATCH v2 5/7] trace2: convert ctx.thread_name from strbuf to pointer Jeff Hostetler via GitGitGadget
2022-10-13 21:12     ` Junio C Hamano
2022-10-12 18:52   ` [PATCH v2 6/7] trace2: add stopwatch timers Jeff Hostetler via GitGitGadget
2022-10-13 21:12     ` Junio C Hamano
2022-10-20 14:42       ` Jeff Hostetler
2022-10-12 18:52   ` [PATCH v2 7/7] trace2: add global counter mechanism Jeff Hostetler via GitGitGadget
2022-10-20 18:28   ` [PATCH v3 0/8] Trace2 timers and counters and some cleanup Jeff Hostetler via GitGitGadget
2022-10-20 18:28     ` [PATCH v3 1/8] trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx Jeff Hostetler via GitGitGadget
2022-10-20 18:28     ` [PATCH v3 2/8] tr2tls: clarify TLS terminology Jeff Hostetler via GitGitGadget
2022-10-20 18:28     ` [PATCH v3 3/8] api-trace2.txt: elminate section describing the public trace2 API Jeff Hostetler via GitGitGadget
2022-10-20 18:28     ` [PATCH v3 4/8] trace2: rename the thread_name argument to trace2_thread_start Jeff Hostetler via GitGitGadget
2022-10-20 18:28     ` [PATCH v3 5/8] trace2: improve thread-name documentation in the thread-context Jeff Hostetler via GitGitGadget
2022-10-20 18:57       ` Ævar Arnfjörð Bjarmason
2022-10-20 20:15         ` Jeff Hostetler
2022-10-20 18:28     ` [PATCH v3 6/8] trace2: convert ctx.thread_name from strbuf to pointer Jeff Hostetler via GitGitGadget
2022-10-20 18:28     ` [PATCH v3 7/8] trace2: add stopwatch timers Jeff Hostetler via GitGitGadget
2022-10-20 20:25       ` Junio C Hamano
2022-10-20 20:52         ` Jeff Hostetler
2022-10-20 20:55           ` Junio C Hamano
2022-10-21 21:51             ` Jeff Hostetler
2022-10-20 18:28     ` [PATCH v3 8/8] trace2: add global counter mechanism Jeff Hostetler via GitGitGadget
2022-10-24 13:40     ` [PATCH v4 0/8] Trace2 timers and counters and some cleanup Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 1/8] trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx Jeff Hostetler via GitGitGadget
2022-10-24 20:31         ` Junio C Hamano
2022-10-25 12:35           ` Derrick Stolee
2022-10-25 15:40             ` Junio C Hamano
2022-10-24 13:41       ` [PATCH v4 2/8] tr2tls: clarify TLS terminology Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 3/8] api-trace2.txt: elminate section describing the public trace2 API Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 4/8] trace2: rename the thread_name argument to trace2_thread_start Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 5/8] trace2: improve thread-name documentation in the thread-context Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 6/8] trace2: convert ctx.thread_name from strbuf to pointer Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 7/8] trace2: add stopwatch timers Jeff Hostetler via GitGitGadget
2022-10-24 13:41       ` [PATCH v4 8/8] trace2: add global counter mechanism Jeff Hostetler via GitGitGadget
2022-10-25 12:27       ` [PATCH v4 0/8] Trace2 timers and counters and some cleanup Derrick Stolee
2022-10-25 15:36         ` 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

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

  git send-email \
    --in-reply-to=xmqq7d1eqhbf.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jeffhost@microsoft.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.
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 NNTP newsgroup(s).