From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id EB4CB6EFDA; Wed, 11 May 2022 11:26:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EB4CB6EFDA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1652257561; bh=2soJXPZKXH5px60xH5Zc5bCCcXn51OHATjgsgNa4BaY=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=fnzcZ6zwxhKzP3QW1uGc9Z1S+F29WGIaPS2a+MhMtqvtxHn5zL5B+5JjpGytf9Z4T NCNW6/g0IyrJRhaA6qx5UqigZ27n9kmrGLl//dBJgA6laLwAgpkSfCSvG5oOpvzvGh iGnZTG4hn7oHWrgK+jJJJD5ydJpBFp3tw4wmdlGo= Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 70DEF6EFDB for ; Wed, 11 May 2022 11:25:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 70DEF6EFDB Received: by mail-lf1-f54.google.com with SMTP id p10so2172867lfa.12 for ; Wed, 11 May 2022 01:25:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=IswQBtM64WTb02nNF6JBIVX73mQceEWP2fYuvW1tKmE=; b=LZuRC9NJDzrOC8Oe9G5n9O0stkEU8GwxPxvQJc49V8OUFDKeKNNXSLLs+ShQHpYW8h cPvaN9bXETbGlI7fWTg5QP9TUAWPMwZdvlxcQl1ZVd7LW/JjpDyWdHvrw2DmG82Y32+P Zik04ILKaMHU8QJBJdxHxOGCNSevBios2pj7afo9JY8/q/rLhzZkTnPnchQGS1DAQFTA 37EpedDJqNp4gPQ+ERC/2zibvgSxRVOqxa3k82QEVin4kvIGJ9a2c3OpizHqDgoAyxL3 MoBy2lnkNkYSsEJ37pK8iFA8p9rxl/lp8rtLvXacKZr2I7M00Y9A/Qz/+5ydJiW7/i55 Q3cA== X-Gm-Message-State: AOAM533rCPPLXN3cQT0sfrRqFugAhBVpqoUGeIA0B4C++rwnA8M9Ts5A EccNGeTejz4YDrBxX989kVNQD6vmoipugQ== X-Google-Smtp-Source: ABdhPJxo2bdLHofhJCAQwvyu4rB0z92DyX44UU5ncbN5DQBR+m8Vxoqrf28S5xa0GKMmUqBfmJCbbw== X-Received: by 2002:ac2:482f:0:b0:472:47d5:ef32 with SMTP id 15-20020ac2482f000000b0047247d5ef32mr19167973lft.344.1652257526513; Wed, 11 May 2022 01:25:26 -0700 (PDT) Received: from localhost.localdomain ([93.175.11.199]) by smtp.gmail.com with ESMTPSA id 20-20020ac25f54000000b0047255d21167sm174724lfz.150.2022.05.11.01.25.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 May 2022 01:25:26 -0700 (PDT) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, imun@tarantool.org, skaplun@tarantool.org Date: Wed, 11 May 2022 11:25:18 +0300 Message-Id: <20220511082521.389687-2-m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220511082521.389687-1-m.kokryashkin@tarantool.org> References: <20220511082521.389687-1-m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] sysprof: change C configuration API X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This patch replaces the `lj_sysprof_configure` with individual configuration handles for each config entry. Also, it chnages the internal sysprof checks to let users run sysprof in default mode without configuratons. Part of tarantool/tarantool#781 --- Branch: https://github.com/tarantool/luajit/tree/fckxorg/sysprof-rc-full-ci PR with Tarantool integration: https://github.com/tarantool/tarantool/pull/7123 src/lib_misc.c | 20 +--- src/lj_mapi.c | 13 ++- src/lj_sysprof.c | 91 +++++++++++++------ src/lj_sysprof.h | 6 +- src/lmisclib.h | 54 +++++------ .../misclib-sysprof-capi/testsysprof.c | 17 +--- .../misclib-sysprof-lapi.test.lua | 2 +- 7 files changed, 116 insertions(+), 87 deletions(-) diff --git a/src/lib_misc.c b/src/lib_misc.c index e07f1b32..370307a7 100644 --- a/src/lib_misc.c +++ b/src/lib_misc.c @@ -288,23 +288,10 @@ LJLIB_CF(misc_sysprof_start) int status = PROFILE_SUCCESS; struct luam_Sysprof_Options opt = {}; - struct luam_Sysprof_Config conf = {}; - status = parse_options(L, &opt); - if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) { - return sysprof_error(L, status); - } - - conf.writer = buffer_writer_default; - conf.on_stop = on_stop_cb_default; - conf.backtracer = NULL; - - status = luaM_sysprof_configure(&conf); - if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) { - on_stop_cb_default(opt.ctx, opt.buf); + if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) return sysprof_error(L, status); - } status = luaM_sysprof_start(L, &opt); if (LJ_UNLIKELY(status != PROFILE_SUCCESS)) @@ -454,9 +441,12 @@ LJLIB_CF(misc_memprof_stop) LUALIB_API int luaopen_misc(struct lua_State *L) { + luaM_sysprof_set_writer(buffer_writer_default); + luaM_sysprof_set_on_stop(on_stop_cb_default); + luaM_sysprof_set_backtracer(NULL); + LJ_LIB_REG(L, LUAM_MISCLIBNAME, misc); LJ_LIB_REG(L, LUAM_MISCLIBNAME ".memprof", misc_memprof); LJ_LIB_REG(L, LUAM_MISCLIBNAME ".sysprof", misc_sysprof); - return 1; } diff --git a/src/lj_mapi.c b/src/lj_mapi.c index 73b5dc74..b327d2ca 100644 --- a/src/lj_mapi.c +++ b/src/lj_mapi.c @@ -67,10 +67,19 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics) } /* --- Platform and Lua profiler ------------------------------------------ */ +LUAMISC_API int luaM_sysprof_set_writer(sp_writer writer) +{ + return lj_sysprof_set_writer(writer); +} + +LUAMISC_API int luaM_sysprof_set_on_stop(sp_on_stop on_stop) +{ + return lj_sysprof_set_on_stop(on_stop); +} -LUAMISC_API int luaM_sysprof_configure(const struct luam_Sysprof_Config *config) +LUAMISC_API int luaM_sysprof_set_backtracer(sp_backtracer backtracer) { - return lj_sysprof_configure(config); + return lj_sysprof_set_backtracer(backtracer); } LUAMISC_API int luaM_sysprof_start(lua_State *L, diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c index 23947315..3088036f 100644 --- a/src/lj_sysprof.c +++ b/src/lj_sysprof.c @@ -54,8 +54,6 @@ #define vmstfit4(st) ((st & ~(uint32_t)((1 << 4) - 1)) == 0) enum sysprof_state { - /* Profiler needs to be configured. */ - SPS_UNCONFIGURED, /* Profiler is not running. */ SPS_IDLE, /* Profiler is running. */ @@ -74,7 +72,9 @@ struct sysprof { struct lj_wbuf out; /* Output accumulator. */ struct luam_Sysprof_Counters counters; /* Profiling counters. */ struct luam_Sysprof_Options opt; /* Profiling options. */ - struct luam_Sysprof_Config config; /* Profiler configurations. */ + sp_writer writer; /* Writer function for profile events. */ + sp_on_stop on_stop; /* Callback on profiling stopping. */ + sp_backtracer backtracer; /* Backtracing function for the host stack. */ lj_profile_timer timer; /* Profiling timer. */ int saved_errno; /* Saved errno when profiler failed. */ uint32_t lib_adds; /* Number of libs loaded. Monotonic. */ @@ -95,6 +95,11 @@ static int stream_is_needed(struct sysprof *sp) return sp->opt.mode != LUAM_SYSPROF_DEFAULT; } +static int is_unconfigured(struct sysprof *sp) +{ + return sp->backtracer == NULL || sp->on_stop == NULL || sp->writer == NULL; +} + static void stream_prologue(struct sysprof *sp) { lj_symtab_dump(&sp->out, sp->g, &sp->lib_adds); @@ -206,8 +211,8 @@ static void default_backtrace_host(void *(writer)(int frame_no, void *addr)) static void stream_backtrace_host(struct sysprof *sp) { - lua_assert(sp->config.backtracer != NULL); - sp->config.backtracer(stream_frame_host); + lua_assert(sp->backtracer != NULL); + sp->backtracer(stream_frame_host); lj_wbuf_addu64(&sp->out, (uintptr_t)LJP_FRAME_HOST_LAST); } @@ -325,15 +330,11 @@ static int sysprof_validate(struct sysprof *sp, const struct luam_Sysprof_Options *opt) { switch (sp->state) { - case SPS_UNCONFIGURED: - return PROFILE_ERRUSE; - case SPS_IDLE: if (opt->mode > LUAM_SYSPROF_CALLGRAPH) { return PROFILE_ERRUSE; } else if (opt->mode != LUAM_SYSPROF_DEFAULT && - (opt->buf == NULL || opt->len == 0 || - sp->config.writer == NULL || sp->config.on_stop == NULL)) { + (opt->buf == NULL || opt->len == 0 || is_unconfigured(sp))) { return PROFILE_ERRUSE; } else if (opt->interval == 0) { return PROFILE_ERRUSE; @@ -373,24 +374,46 @@ static int sysprof_init(struct sysprof *sp, lua_State *L, sp->saved_errno = 0; if (stream_is_needed(sp)) - lj_wbuf_init(&sp->out, sp->config.writer, opt->ctx, opt->buf, opt->len); + lj_wbuf_init(&sp->out, sp->writer, opt->ctx, opt->buf, opt->len); return PROFILE_SUCCESS; } /* -- Public profiling API ------------------------------------------------ */ -int lj_sysprof_configure(const struct luam_Sysprof_Config *config) -{ +int lj_sysprof_set_writer(sp_writer writer) { + struct sysprof *sp = &sysprof; + + if (sp->state != SPS_IDLE || writer == NULL) + return PROFILE_ERRUSE; + + sp->writer = writer; + if (!is_unconfigured(sp)) { + sp->state = SPS_IDLE; + } + return PROFILE_SUCCESS; +} + +int lj_sysprof_set_on_stop(sp_on_stop on_stop) { struct sysprof *sp = &sysprof; - lua_assert(config != NULL); - if (sp->state != SPS_UNCONFIGURED && sp->state != SPS_IDLE) + + if (sp->state != SPS_IDLE || on_stop == NULL) return PROFILE_ERRUSE; - memcpy(&sp->config, config, sizeof(*config)); + sp->on_stop = on_stop; + if (!is_unconfigured(sp)) { + sp->state = SPS_IDLE; + } + return PROFILE_SUCCESS; +} + +int lj_sysprof_set_backtracer(sp_backtracer backtracer) { + struct sysprof *sp = &sysprof; - if (sp->config.backtracer == NULL) { - sp->config.backtracer = default_backtrace_host; + if (sp->state != SPS_IDLE) + return PROFILE_ERRUSE; + if (backtracer == NULL) { + sp->backtracer = default_backtrace_host; /* ** XXX: `backtrace` is not signal-safe, according to man, ** because it is lazy loaded on the first call, which triggers @@ -400,9 +423,12 @@ int lj_sysprof_configure(const struct luam_Sysprof_Config *config) void *dummy = NULL; backtrace(&dummy, 1); } - - sp->state = SPS_IDLE; - + else { + sp->backtracer = backtracer; + } + if (!is_unconfigured(sp)) { + sp->state = SPS_IDLE; + } return PROFILE_SUCCESS; } @@ -412,12 +438,12 @@ int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt) int status = sysprof_init(sp, L, opt); if (PROFILE_SUCCESS != status) { - if (NULL != sp->config.on_stop) { + if (NULL != sp->on_stop) { /* ** Initialization may fail in case of unconfigured sysprof, ** so we cannot guarantee cleaning up resources in this case. */ - sp->config.on_stop(opt->ctx, opt->buf); + sp->on_stop(opt->ctx, opt->buf); } return status; } @@ -430,7 +456,7 @@ int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt) /* on_stop call may change errno value. */ const int saved_errno = lj_wbuf_errno(&sp->out); /* Ignore possible errors. mp->out.buf may be NULL here. */ - sp->config.on_stop(opt->ctx, sp->out.buf); + sp->on_stop(opt->ctx, sp->out.buf); lj_wbuf_terminate(&sp->out); sp->state = SPS_IDLE; errno = saved_errno; @@ -473,7 +499,7 @@ int lj_sysprof_stop(lua_State *L) stream_epilogue(sp); lj_wbuf_flush(out); - cb_status = sp->config.on_stop(sp->opt.ctx, out->buf); + cb_status = sp->on_stop(sp->opt.ctx, out->buf); if (LJ_UNLIKELY(lj_wbuf_test_flag(out, STREAM_ERRIO | STREAM_STOP)) || cb_status != 0) { errno = lj_wbuf_errno(out); @@ -530,9 +556,18 @@ void lj_sysprof_add_trace(const struct GCtrace *tr) #else /* LJ_HASSYSPROF */ -int lj_sysprof_configure(const struct luam_Sysprof_Config *config) -{ - UNUSED(config); +int lj_sysprof_set_writer(sp_writer writer) { + UNUSED(writer); + return PROFILE_ERRUSE; +} + +int lj_sysprof_set_on_stop(sp_on_stop on_stop) { + UNUSED(on_stop); + return PROFILE_ERRUSE; +} + +int lj_sysprof_set_backtracer(sp_backtracer backtracer) { + UNUSED(backtracer); return PROFILE_ERRUSE; } diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h index c31d61d8..d869013e 100644 --- a/src/lj_sysprof.h +++ b/src/lj_sysprof.h @@ -86,7 +86,11 @@ #define LJP_SYMTAB_TRACE_EVENT ((uint8_t)12) #define LJP_EPILOGUE_BYTE 0x80 -int lj_sysprof_configure(const struct luam_Sysprof_Config *config); +int lj_sysprof_set_writer(sp_writer writer); + +int lj_sysprof_set_on_stop(sp_on_stop on_stop); + +int lj_sysprof_set_backtracer(sp_backtracer backtracer); int lj_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt); diff --git a/src/lmisclib.h b/src/lmisclib.h index b41f7f59..3ebf18ff 100644 --- a/src/lmisclib.h +++ b/src/lmisclib.h @@ -63,31 +63,29 @@ LUAMISC_API void luaM_metrics(lua_State *L, struct luam_Metrics *metrics); /* --- Sysprof - platform and lua profiler -------------------------------- */ /* Profiler configurations. */ -struct luam_Sysprof_Config { - /* - ** Writer function for profile events. Must be async-safe, see also - ** `man 7 signal-safety`. - ** Should return amount of written bytes on success or zero in case of error. - ** Setting *data to NULL means end of profiling. - ** For details see . - */ - size_t (*writer)(const void **data, size_t len, void *ctx); - /* - ** Callback on profiler stopping. Required for correctly cleaning - ** at VM finalization when profiler is still running. - ** Returns zero on success. - */ - int (*on_stop)(void *ctx, uint8_t *buf); - /* - ** Backtracing function for the host stack. Should call `frame_writer` on - ** each frame in the stack in the order from the stack top to the stack - ** bottom. The `frame_writer` function is implemented inside the sysprof - ** and will be passed to the `backtracer` function. If `frame_writer` returns - ** NULL, backtracing should be stopped. If `frame_writer` returns not NULL, - ** the backtracing should be continued if there are frames left. - */ - void (*backtracer)(void *(*frame_writer)(int frame_no, void *addr)); -}; +/* +** Writer function for profile events. Must be async-safe, see also +** `man 7 signal-safety`. +** Should return amount of written bytes on success or zero in case of error. +** Setting *data to NULL means end of profiling. +** For details see . +*/ +typedef size_t (*sp_writer)(const void **data, size_t len, void *ctx); +/* +** Callback on profiler stopping. Required for correctly cleaning +** at VM finalization when profiler is still running. +** Returns zero on success. +*/ +typedef int (*sp_on_stop)(void *ctx, uint8_t *buf); +/* +** Backtracing function for the host stack. Should call `frame_writer` on +** each frame in the stack in the order from the stack top to the stack +** bottom. The `frame_writer` function is implemented inside the sysprof +** and will be passed to the `backtracer` function. If `frame_writer` returns +** NULL, backtracing should be stopped. If `frame_writer` returns not NULL, +** the backtracing should be continued if there are frames left. +*/ +typedef void (*sp_backtracer)(void *(*frame_writer)(int frame_no, void *addr)); /* ** DEFAULT mode collects only data for luam_sysprof_counters, which is stored @@ -144,7 +142,11 @@ struct luam_Sysprof_Options { #define PROFILE_ERRMEM 3 #define PROFILE_ERRIO 4 -LUAMISC_API int luaM_sysprof_configure(const struct luam_Sysprof_Config *config); +LUAMISC_API int luaM_sysprof_set_writer(sp_writer writer); + +LUAMISC_API int luaM_sysprof_set_on_stop(sp_on_stop on_stop); + +LUAMISC_API int luaM_sysprof_set_backtracer(sp_backtracer backtracer); LUAMISC_API int luaM_sysprof_start(lua_State *L, const struct luam_Sysprof_Options *opt); diff --git a/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c b/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c index 4b258766..984fc121 100644 --- a/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c +++ b/test/tarantool-tests/misclib-sysprof-capi/testsysprof.c @@ -122,14 +122,9 @@ static int c_payload(lua_State *L) static int base(lua_State *L) { - struct luam_Sysprof_Config config = {}; struct luam_Sysprof_Options opt = {}; struct luam_Sysprof_Counters cnt = {}; - (void)config.writer; - (void)config.on_stop; - (void)config.backtracer; - (void)opt.interval; (void)opt.mode; (void)opt.ctx; @@ -156,13 +151,9 @@ static int base(lua_State *L) static int validation(lua_State *L) { - struct luam_Sysprof_Config config = {}; struct luam_Sysprof_Options opt = {}; int status = PROFILE_SUCCESS; - status = luaM_sysprof_configure(&config); - assert(PROFILE_SUCCESS == status); - /* Unknown mode. */ opt.mode = 0x40; status = luaM_sysprof_start(L, &opt); @@ -204,7 +195,6 @@ static int validation(lua_State *L) static int profile_func(lua_State *L) { - struct luam_Sysprof_Config config = {}; struct luam_Sysprof_Options opt = {}; struct luam_Sysprof_Counters cnt = {}; int status = PROFILE_ERRUSE; @@ -222,10 +212,9 @@ static int profile_func(lua_State *L) opt.interval = SYSPROF_INTERVAL_DEFAULT; stream_init(&opt); - config.on_stop = on_stop_cb_default; - config.writer = buffer_writer_default; - status = luaM_sysprof_configure(&config); - assert(PROFILE_SUCCESS == status); + assert(luaM_sysprof_set_writer(buffer_writer_default) == PROFILE_SUCCESS); + assert(luaM_sysprof_set_on_stop(on_stop_cb_default) == PROFILE_SUCCESS); + assert(luaM_sysprof_set_backtracer(NULL) == PROFILE_SUCCESS); status = luaM_sysprof_start(L, &opt); assert(PROFILE_SUCCESS == status); diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua index bb0e065d..2f53d771 100644 --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua @@ -70,7 +70,7 @@ res, err = misc.sysprof.start{ mode = "D" } assert(res, err) res, err, errno = misc.sysprof.start{ mode = "D" } -test:ok(res == nil and err:match("profiler misuse")) +test:ok(res == nil and err:match("profiler is running already")) test:ok(type(errno) == "number") res, err = misc.sysprof.stop() -- 2.35.1