From: Derrick Stolee <stolee@gmail•com>
To: Matthew John Cheetham via GitGitGadget <gitgitgadget@gmail•com>,
git@vger•kernel.org
Cc: gitster@pobox•com, johannes.schindelin@gmx•de,
Matthew John Cheetham <mjcheetham@outlook•com>
Subject: Re: [PATCH 1/4] trace2: add macOS process ancestry tracing
Date: Mon, 9 Feb 2026 09:36:06 -0500 [thread overview]
Message-ID: <7390e189-16ac-43b3-a63c-a8b942d5934b@gmail.com> (raw)
In-Reply-To: <d99a30a1a77f0f23468dba987da08b32dd9a92fa.1770307510.git.gitgitgadget@gmail.com>
On 2/5/2026 11:05 AM, Matthew John Cheetham via GitGitGadget wrote:
> Teach Git to also log process ancestry on macOS using the sysctl with
> KERN_PROC to get process information (PPID and process name).
> Like the Linux implementation, we use the cmd_ancestry TRACE2 event
> rather than using a data_json event and creating another custom data
> point.
> +#define USE_THE_REPOSITORY_VARIABLE
If we are creating a new file, then it would be best if we avoid this
macro, which is intended for older code to still work until it can be
fixed.
But also it seems that you don't use the_repository anywhere, so this
can be deleted without consequence!
> +/*
> + * Recursively push process names onto the ancestry array.
> + * We guard against cycles by limiting the depth to NR_PIDS_LIMIT.
> + */
> +static void push_ancestry_name(struct strvec *names, pid_t pid, int depth)
> +{
> + struct strbuf name = STRBUF_INIT;
> + pid_t ppid;
> +
> + if (depth >= NR_PIDS_LIMIT)
> + return;
Here is the recursion limit check.
> + if (pid <= 0)
> + return;
> +
> + if (get_proc_info(pid, &name, &ppid) < 0)
> + goto cleanup;
> +
> + strvec_push(names, name.buf);
This is copying the buffer, which is why you release it later.
Question: could we stop copying here and use strbuf_detach() at this
point? That would be a very minor improvement, so feel free to ignore!
I took a look and rediscovered that strvecs do not have an option to not
copy. I'm thinking about string_list. I'm not sure if there is any value
in converting your code just to avoid some string duplication at this
scale.
> + /*
> + * Recurse to the parent process. Stop if ppid is 0 or 1
> + * (init/launchd) or if we've reached ourselves (cycle).
> + */
> + if (ppid > 1 && ppid != pid)
> + push_ancestry_name(names, ppid, depth + 1);
This kind of tail recursion could be easily converted into a loop. I
usually prefer loops to recursion when possible, in case we want to allow
an unlimited number of parents in the future.
> +cleanup:
> + strbuf_release(&name);
> +}
I got a little confused by the lack of a .h file, but that's probably due
to the extra magic being done at compile time to pick this file on a per-
platform basis.
Indeed, trace2_collect_process_info() is defined in trace2.h.
Thanks,
-Stolee
next prev parent reply other threads:[~2026-02-09 14:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-05 16:05 [PATCH 0/4] trace2: add macOS and Windows process ancestry tracing Matthew John Cheetham via GitGitGadget
2026-02-05 16:05 ` [PATCH 1/4] trace2: add macOS " Matthew John Cheetham via GitGitGadget
2026-02-09 14:36 ` Derrick Stolee [this message]
2026-02-09 15:13 ` Matthew John Cheetham
2026-02-10 4:15 ` Derrick Stolee
2026-02-05 16:05 ` [PATCH 2/4] build: include procinfo.c impl for macOS Matthew John Cheetham via GitGitGadget
2026-02-09 14:37 ` Derrick Stolee
2026-02-05 16:05 ` [PATCH 3/4] trace2: refactor Windows process ancestry trace2 event Matthew John Cheetham via GitGitGadget
2026-02-09 14:41 ` Derrick Stolee
2026-02-05 16:05 ` [PATCH 4/4] trace2: emit cmd_ancestry data for Windows Matthew John Cheetham via GitGitGadget
2026-02-05 16:19 ` Kristoffer Haugsbakk
2026-02-09 14:42 ` Derrick Stolee
2026-02-09 14:48 ` [PATCH 0/4] trace2: add macOS and Windows process ancestry tracing Derrick Stolee
2026-02-09 17:05 ` Junio C Hamano
2026-02-13 19:54 ` [PATCH v2 0/6] " Matthew John Cheetham via GitGitGadget
2026-02-13 19:54 ` [PATCH v2 1/6] trace2: add macOS " Matthew John Cheetham via GitGitGadget
2026-02-13 19:54 ` [PATCH v2 2/6] build: include procinfo.c impl for macOS Matthew John Cheetham via GitGitGadget
2026-02-13 20:34 ` Junio C Hamano
2026-02-13 19:54 ` [PATCH v2 3/6] trace2: refactor Windows process ancestry trace2 event Matthew John Cheetham via GitGitGadget
2026-02-13 20:36 ` Junio C Hamano
2026-02-13 19:54 ` [PATCH v2 4/6] trace2: emit cmd_ancestry data for Windows Matthew John Cheetham via GitGitGadget
2026-02-13 20:52 ` Junio C Hamano
2026-02-13 19:54 ` [PATCH v2 5/6] test-tool: extend trace2 helper with 400ancestry Matthew John Cheetham via GitGitGadget
2026-02-13 19:55 ` [PATCH v2 6/6] t0213: add trace2 cmd_ancestry tests Matthew John Cheetham via GitGitGadget
2026-02-14 0:30 ` [PATCH v2 0/6] trace2: add macOS and Windows process ancestry tracing Derrick Stolee
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=7390e189-16ac-43b3-a63c-a8b942d5934b@gmail.com \
--to=stolee@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=gitster@pobox$(echo .)com \
--cc=johannes.schindelin@gmx$(echo .)de \
--cc=mjcheetham@outlook$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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