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 625E96F153; Wed, 31 Aug 2022 18:03:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 625E96F153 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1661958222; bh=qaa+19JpI6Mbv257rwjEOkbX9Fn9Izj4xEgHugoVaMw=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=LoY/Runk5ZeIxk9Cx1hQ4NbZObdbJdUk+Ej+xNG4uvAnAFKKDRmz7C9lvDgenKBHC VjUbXgc2YNStpVEVZFZbrUP0Mj6Z5LPnMkQ5Cce9CZ6Jv2uTqI5aZtkVMtr3afIQeA rYimbzDvXcjKlGxakTea4UO7XjnngdMbhQZMYvZY= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 E72606F153 for ; Wed, 31 Aug 2022 18:03:41 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E72606F153 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1oTPFl-0002rG-0d; Wed, 31 Aug 2022 18:03:41 +0300 Date: Wed, 31 Aug 2022 17:53:24 +0300 To: Sergey Bronnikov Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9BF9AC82A1D2D7237ECC9B9481A4CD7BD50746863994A0877182A05F53808504027F650CF39AD9D97396BB81E10F70BBAA5A120824FC725EB1ABA96DDCDF298CA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78E8764B5BC580342EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E1D2769089B3DFB28638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8DB9AE757E3567A3F90F204C064C8CC5C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B3733B5EC72352B9FA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CA1D607EB49F9BFEF43847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 9604B64F49C60606AD91A466A1DEF99B296C473AB1E142185AC9E3593CE4B31AB1881A6453793CE9274300E5CE05BD4401A9E91200F654B0F56C66D0505D9834523897933F9F83875BE1D8BF3FBD0D73672DEF95EB9DA4449C2B6934AE262D3EE7EAB7254005DCED8DA55E71E02F9FC08E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34E54F8089C01448AADF71A2E4DA96F0A97D9CE3FF38AD02EAFFBE9D09AF7D9CB154B7DE5E8474F6ED1D7E09C32AA3244CEC5B856C1B4883881A2EDEA786AB06FAD9ADFF0C0BDB8D1F927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojxzT94hBWyprAUK3Vh3YkTA== X-DA7885C5: 9B0437FD804235447E945E04F1611E3F6ABB5BF464C9F200F89B8B9C8D97F10D262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F7393CC2E0F076E87284E90158034383235031F181DC110F3E1C2A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for your review! On 15.08.22, Sergey Bronnikov wrote: > 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. I believe your commit subject is too long, so I propose another solution. Since the only thing affected by LUAJIT_TEST_VARDIR is , so I can write something like "test: introduce utils.profilename helper" and describe its rationale and functions within the commit message. Are you OK with it? > > > > > > 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(-) > > > > @@ -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? These are artefacts from not such elegant memprof tests... > > > > -- Collect all previous allocated objects. > > subtest:ok(free.INTERNAL.num == 22) -- Best regards, IM