From patchwork Fri Oct 7 01:10:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13000840 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA8A1C433FE for ; Fri, 7 Oct 2022 01:10:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231489AbiJGBKQ (ORCPT ); Thu, 6 Oct 2022 21:10:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43854 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229906AbiJGBKO (ORCPT ); Thu, 6 Oct 2022 21:10:14 -0400 Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 776B13FA30 for ; Thu, 6 Oct 2022 18:10:11 -0700 (PDT) Received: by mail-wm1-x32f.google.com with SMTP id t4so2016974wmj.5 for ; Thu, 06 Oct 2022 18:10:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=aUJ6gA3GrVElkb5+LPIfGlF7hIXeFnZybW4AEOtzqjk=; b=nX8K3MJZvzg6a2cDm2Q9S7uLsPWuNDubQMmIHmyPxRTHfxnRJzwYRz7YQV8Up9HShi F+la7dd6nWt0/O8+0kqzaQRoP1zL/rwgHgHRcv7iVEG0n7avumH8jW8Rre3RhPEP9R5r xCoz8jn5Tte3s1+/6PIJ3tJgmHTFU8AglXA2f4/SIVGXk7NEh2AbhufZz3WKn+PhfMm+ HcmOWBmRokwArEiZ9zr9pczzT28wV26M5HFd3KCUQbSsTh9wSesQmPJRv8x1s0v6MSJQ C4Ywg+fi8lmKrG4Pip/d8n8TYw8FfbJ+X/9K3+M+t2rtaUQagAYgDarW8peFwHVC8mCL ek1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=aUJ6gA3GrVElkb5+LPIfGlF7hIXeFnZybW4AEOtzqjk=; b=CU25cKrsl7/j/9XSeWGJrpZXujbIZXaU2OPSWtDZth35CTysT2clm9tk/9JtyfJhQd he9W791ZbfcLZxLFXS5A4oggRJvxZLLC/g3ehanbkaxQdulvHJkHmY29yuijUojhRvIt 9eTJt2PGRPArfS0NfFdiZj4V/0Yn+FS1osKaKkmMJEFb1HhQgA+GxWRCe3FksBGpsvwT xIgEmo7jxtHTLWPzGOEYTyS3bL3bNmf9hFB6s4++BI5GFJjPSMjL8DM7CX4Qad3qARHX 5WsY1ZS2VOxF9CKe+/0vVmeEvJ+i7U2mDcC4lrkFNP47H9UTlw7ker3QUFuHCUA4ogr8 i7PA== X-Gm-Message-State: ACrzQf1aDncVIWmlDHrqvJKKI6UJOeTMOQqD7xlx3ArOIjIk9YAlw8nn Uh1DXXBhFBW5EcfPHMkwjgaxZkY7iVo= X-Google-Smtp-Source: AMsMyM5yinaouEthvfZJzm9AObh+BSXDMXmMIaRFkrgDYrpXnOOmxndARJb+huuzuduf+Qv2d8FfSQ== X-Received: by 2002:a05:600c:3512:b0:3b4:bcde:26ad with SMTP id h18-20020a05600c351200b003b4bcde26admr8953745wmq.164.1665105009517; Thu, 06 Oct 2022 18:10:09 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id v7-20020a5d59c7000000b0022e3978fd07sm690169wry.39.2022.10.06.18.10.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Oct 2022 18:10:08 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff Hostetler , Jeff Hostetler via GitGitGadget , =?utf-8?b?w4Z2?= =?utf-8?b?YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [RFC PATCH] trace2 API: don't save a copy of constant "thread_name" Date: Fri, 7 Oct 2022 03:10:06 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.971.ge79ff6d20e7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Since ee4512ed481 (trace2: create new combined trace facility, 2019-02-22) the "thread_name" member of "struct tr2tls_thread_ctx" has been copied from the caller, but those callers have always passed a constant string: $ git -P grep '^\s*trace2_thread_start\(' Documentation/technical/api-trace2.txt: trace2_thread_start("preload_thread"); builtin/fsmonitor--daemon.c: trace2_thread_start("fsm-health"); builtin/fsmonitor--daemon.c: trace2_thread_start("fsm-listen"); compat/simple-ipc/ipc-unix-socket.c: trace2_thread_start("ipc-worker"); compat/simple-ipc/ipc-unix-socket.c: trace2_thread_start("ipc-accept"); compat/simple-ipc/ipc-win32.c: trace2_thread_start("ipc-server"); t/helper/test-fsmonitor-client.c: trace2_thread_start("hammer"); t/helper/test-simple-ipc.c: trace2_thread_start("multiple"); This isn't needed for optimization, but apparently[1] there's been some confusion about the non-const-ness of the previous "struct strbuf". Using the caller's string here makes this more straightforward, as it's now clear that we're not dynamically constructing these. It's also what the progress API does with its "title" string. Since we know we're hardcoding these thread names let's BUG() out when we see that the length of the name plus the length of the prefix would exceed the maximum length for the "perf" format. 1. https://lore.kernel.org/git/82f1672e180afcd876505a4354bd9952f70db49e.1664900407.git.gitgitgadget@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason --- On Thu, Oct 06 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: >> So, we don't need to strdup() and store that "preload_thread" anywhere. >> It's already a constant string we have hardcoded in the program. We just >> need to save a pointer to it. > > That sounds even simpler. A cleaned up version of the test code I had on top of "master", RFC because I may still be missing some context here. E.g. maybe there's a plan to dynamically construct these thread names? json-writer.c | 17 +++++++++++++++++ json-writer.h | 4 ++++ trace2/tr2_tgt_event.c | 2 +- trace2/tr2_tgt_perf.c | 10 +++++++--- trace2/tr2_tls.c | 14 +++++--------- trace2/tr2_tls.h | 9 +++++++-- 6 files changed, 41 insertions(+), 15 deletions(-) diff --git a/json-writer.c b/json-writer.c index f1cfd8fa8c6..569a75bee51 100644 --- a/json-writer.c +++ b/json-writer.c @@ -161,6 +161,23 @@ void jw_object_string(struct json_writer *jw, const char *key, const char *value append_quoted_string(&jw->json, value); } +void jw_strbuf_add_thread_name(struct strbuf *sb, const char *thread_name, + int thread_id) +{ + if (thread_id) + strbuf_addf(sb, "th%02d:", thread_id); + strbuf_addstr(sb, thread_name); +} + +void jw_object_string_thread(struct json_writer *jw, const char *thread_name, + int thread_id) +{ + object_common(jw, "thread"); + strbuf_addch(&jw->json, '"'); + jw_strbuf_add_thread_name(&jw->json, thread_name, thread_id); + strbuf_addch(&jw->json, '"'); +} + void jw_object_intmax(struct json_writer *jw, const char *key, intmax_t value) { object_common(jw, key); diff --git a/json-writer.h b/json-writer.h index 209355e0f12..269c203b119 100644 --- a/json-writer.h +++ b/json-writer.h @@ -75,6 +75,10 @@ void jw_release(struct json_writer *jw); void jw_object_begin(struct json_writer *jw, int pretty); void jw_array_begin(struct json_writer *jw, int pretty); +void jw_strbuf_add_thread_name(struct strbuf *buf, const char *thread_name, + int thread_id); +void jw_object_string_thread(struct json_writer *jw, const char *thread_name, + int thread_id); void jw_object_string(struct json_writer *jw, const char *key, const char *value); void jw_object_intmax(struct json_writer *jw, const char *key, intmax_t value); diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 37a3163be12..1308cf05df4 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -90,7 +90,7 @@ static void event_fmt_prepare(const char *event_name, const char *file, jw_object_string(jw, "event", event_name); jw_object_string(jw, "sid", tr2_sid_get()); - jw_object_string(jw, "thread", ctx->thread_name.buf); + jw_object_string_thread(jw, ctx->thread_name, ctx->thread_id); /* * In brief mode, only emit