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 352666F154; Mon, 15 Aug 2022 15:08:25 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 352666F154 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1660565305; bh=UInGYGMMEWHFZYUm8JuAxitZj+F6ghwfQ1x4jD4h8Cg=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=zMNgSVaJd8ukPSuJIYPP7ltw32eBQbSH1Xx/IedS3j6cNGf+tGBK8K5FNrc7yrlwf UqgfQdRpOrM+EXG1zkvT5+AoIus51/lr78jycvYucvInJ1yviAShgXzcSLY0EsgmFE 5SLSvA9rOvRRpiYQpYjCQZ7F99v7W11rsddAqER8= Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 870F76F154 for ; Mon, 15 Aug 2022 15:08:23 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 870F76F154 Received: by smtp44.i.mail.ru with esmtpa (envelope-from ) id 1oNYtK-0003oN-EC; Mon, 15 Aug 2022 15:08:22 +0300 Message-ID: Date: Mon, 15 Aug 2022 15:08:22 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Content-Language: en-US To: Igor Munkin , Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9E6910A3ADAF35E0291F67E1A9226D6336AADCE203761B7A3182A05F5380850404C228DA9ACA6FE2703BBF3171671AF1821E6DC5A43FEA70E575B9D9291EBB33DF2E0865872F0670D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7956F10FFCC7409BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637CDA089757FB31C668638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D826D38B78EA81AADD87D58B68CBAF7EB5117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCED943DBD20860CC2A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EEB28585415E75ADA9A9D86F9317F2E7ACD8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE35FF72824B19451C62D242C3BD2E3F4C6C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637C31AA6A98257FC32EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 9604B64F49C60606AD91A466A1DEF99B296C473AB1E142185AC9E3593CE4B31AB1881A6453793CE9274300E5CE05BD4401A9E91200F654B0C7A0BC55FA0FE5FCAA6D6587D0187CD16D89599A7D3229FF2EB3B2DA86D50864B1881A6453793CE9C32612AADDFBE06133F7A9E5587C79A693EDB24507CE13387DFF0A840B692CF8 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34411AA3F52B5778B6B7B42394BFA5673958D139C3B4F3F6A26CE314FC2A701230FD4F8EFFFDCBCDFF1D7E09C32AA3244CDE664218F983E5807F6AE8200793053C95A9E0DC41E9A4CFFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojKOC4OzB6pCOSNYJhvSq8tQ== X-Mailru-Sender: 11C2EC085EDE56FAC71737E9F694C0DE89915C0F9BAE9F7F2599B5214B456435E436CDC11A29B020645D15D82EE4B272BD6E4642A116CA93524AA66B5ACBE6721EF430B9A63E2A504198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/8] test: introduce LUAJIT_TEST_VARDIR variable 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: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Igor,  thanks for the patch. See my comments inline. On 11.08.2022 14:17, Igor Munkin wrote: > Before the patch both memprof and sysprof artefacts are generated within > the binary artefacts tree (i.e. in the same directory LuaJIT binary is > generated). However, more convenient way is producing these temporary > profiles in a separate directory (e.g. located on the partition with > more strict space limits). As a result of the patch all memprof and > sysprof test chunks consider LUAJIT_TEST_VARDIR environment variable to > set the directory where test profiles are generated. For the case when > LUAJIT_TEST_VARDIR is not set, everything works as before. Commit message is inconsistent a bit with a patch itself. As far as I understand many hunks are not related to introducing LUAJIT_TEST_VARDIR. Probably it is better to change commit one-line message from "test: introduce LUAJIT_TEST_VARDIR variable" to something like "test: refactoring and introduce LUAJIT_TEST_VARDIR variable". It allows to keep patch as is and change expectations for those who will look at your patch. > > Part of tarantool/tarantool#7472 > > Signed-off-by: Igor Munkin > --- > .../gh-5813-resolving-of-c-symbols.test.lua | 6 +++-- > .../misclib-memprof-lapi.test.lua | 22 ++++++++++--------- > .../misclib-sysprof-lapi.test.lua | 8 ++++--- > test/tarantool-tests/utils.lua | 12 ++++++++++ > 4 files changed, 33 insertions(+), 15 deletions(-) > > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua > index d589dddf..e0b6d86d 100644 > --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua > +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua > @@ -1,5 +1,7 @@ > -- Memprof is implemented for x86 and x64 architectures only. > -require("utils").skipcond( > +local utils = require("utils") > + > +utils.skipcond( > jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux", > jit.arch.." architecture or "..jit.os.. > " OS is NIY for memprof c symbols resolving" > @@ -18,7 +20,7 @@ local testboth = require "resboth" > local testhash = require "reshash" > local testgnuhash = require "resgnuhash" > > -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin") > +local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin") > > local function tree_contains(node, name) > if node == nil then > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua > index a11f0be1..bae0c27c 100644 > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua > @@ -1,5 +1,7 @@ > -- Memprof is implemented for x86 and x64 architectures only. > -require("utils").skipcond( > +local utils = require("utils") > + > +utils.skipcond( > jit.arch ~= "x86" and jit.arch ~= "x64", > jit.arch.." architecture is NIY for memprof" > ) > @@ -26,8 +28,8 @@ local memprof = require "memprof.parse" > local process = require "memprof.process" > local symtab = require "utils.symtab" > > -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin") > -local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/memprofdata.tmp.bin") > +local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin") > +local BAD_PATH = utils.profilename("memprofdata/tmp.bin") > local SRC_PATH = "@"..arg[0] > > local function default_payload() > @@ -189,9 +191,9 @@ test:test("output", function(subtest) > -- one is the number of allocations. 1 event - alocation of > -- table by itself + 1 allocation of array part as far it is > -- bigger than LJ_MAX_COLOSIZE (16). > - subtest:ok(check_alloc_report(alloc, { line = 35, linedefined = 33 }, 2)) > + subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2)) > -- 20 strings allocations. > - subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 20)) > + subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20)) What are these magic numbers mean and why we should change them for introducing LUAJIT_TEST_VARDIR? > > -- Collect all previous allocated objects. > subtest:ok(free.INTERNAL.num == 22) > @@ -199,8 +201,8 @@ test:test("output", function(subtest) > -- Tests for leak-only option. > -- See also https://github.com/tarantool/tarantool/issues/5812. > local heap_delta = process.form_heap_delta(events, symbols) > - local tab_alloc_stats = heap_delta[form_source_line(35)] > - local str_alloc_stats = heap_delta[form_source_line(40)] > + local tab_alloc_stats = heap_delta[form_source_line(37)] > + local str_alloc_stats = heap_delta[form_source_line(42)] > subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree) > subtest:ok(tab_alloc_stats.dbytes == 0) > subtest:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree) > @@ -291,10 +293,10 @@ test:test("jit-output", function(subtest) > -- 10 allocations in interpreter mode, 1 allocation for a trace > -- recording and assembling and next 9 allocations will happen > -- while running the trace. > - subtest:ok(check_alloc_report(alloc, { line = 40, linedefined = 33 }, 11)) > - subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 38 }, 9)) > + subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 11)) > + subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 40 }, 9)) > -- See same checks with jit.off(). > - subtest:ok(check_alloc_report(alloc, { line = 35, linedefined = 33 }, 2)) > + subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2)) > > -- Restore default JIT settings. > jit.opt.start(unpack(jit_opt_default)) > diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua > index 9e0a8a77..dbff41b0 100644 > --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua > +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua > @@ -1,6 +1,8 @@ > -- Sysprof is implemented for x86 and x64 architectures only. > local ffi = require("ffi") > -require("utils").skipcond( > +local utils = require("utils") > + > +utils.skipcond( > jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux" > or ffi.abi("gc64"), > jit.arch.." architecture or "..jit.os.. > @@ -19,8 +21,8 @@ local bufread = require("utils.bufread") > local symtab = require("utils.symtab") > local sysprof = require("sysprof.parse") > > -local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.sysprofdata.tmp.bin") > -local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/sysprofdata.tmp.bin") > +local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin") > +local BAD_PATH = utils.profilename("sysprofdata/tmp.bin") > > local function payload() > local function fib(n) > diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua > index a1906498..87f7ff15 100644 > --- a/test/tarantool-tests/utils.lua > +++ b/test/tarantool-tests/utils.lua > @@ -123,4 +123,16 @@ function M.hasbc(f, bytecode) > return hasbc > end > > +function M.profilename(name) > + local vardir = os.getenv('LUAJIT_TEST_VARDIR') > + -- Replace pattern will change directory name of the generated > + -- profile to LUAJIT_TEST_VARDIR if it is set in the process > + -- environment. Otherwise, the original dirname is left intact. > + -- As a basename for this profile the test name is concatenated > + -- with the name given as an argument. > + local replacepattern = ('%s/%s-%s'):format(vardir or '%1', '%2', name) > + -- XXX: return only the resulting string. > + return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern)) > +end > + > return M