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 E5B3B13E37A6; Wed, 4 Jun 2025 16:13:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E5B3B13E37A6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1749042811; bh=X5TTic/0OwIvXruqR8m6LkdTwSuHdudsSmjdH2QX/LM=; 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=alq4lKQiDj9YQaJKmKGlzlYIR0pSS+ROyQSc5ZaSyRongiLOCI9Xhm+lHU4Z0cOdz UdkSAg4zexbo/Mkckzr5Kxk871MB2lNuGBOo5cqTqBjdBYdYIL0Ioh4NovBPKiiM2m jsuk31+Wz4/zfsBuHfbS/f0xUyZv6PvVR0HqINJo= Received: from send60.i.mail.ru (send60.i.mail.ru [89.221.237.155]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 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 A639613E37A6 for ; Wed, 4 Jun 2025 16:13:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A639613E37A6 Received: by exim-smtp-75656d46d5-n5bp2 with esmtpa (envelope-from ) id 1uMnvs-000000001dA-1rYT; Wed, 04 Jun 2025 16:13:28 +0300 Date: Wed, 4 Jun 2025 16:13:34 +0300 To: Sergey Bronnikov Cc: Sergey Bronnikov , tarantool-patches@dev.tarantool.org Message-ID: References: <6aefeed4eee55e77e7c79ac9b31c284b014d2c7a.1747399055.git.sergeyb@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9186843A488DB0002F5E220F3556F92296BF3943151A02FCB1313CFAB8367EF908E2BE116634AD74DE140B4DB5C91AB469487ABAC94A94B54A712680CE9E533CEA065F9720E92348F1E39E150AE5B0BE1 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE705C173BDDADFC82AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375662BC6C8E92A5ED823B67028194DADC9B6643EEF6511E4C0017506912144CFAE03389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B652D31B9D28593E51CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE76D0F27F7E6A6C418731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A58637FA02B209CEAE5002B1117B3ED696F202876E60D3F09A484B8D70797403F6823CB91A9FED034534781492E4B8EEADA3A806F356AF31D6 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34E3E1DC0F9BD5530993ABC8818BC7C803201783D2624728D8973800DBF531F48AE291560F10099D2A1D7E09C32AA3244C553D7AF2E69A15EB77DD89D51EBB7742EAB42ABC1873A19CEA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVSykAyseJQ6/2gGXBL/YPi8= X-DA7885C5: 6A5F3F07771BC4CDF255D290C0D534F97B330E1BBC2944661D6EDC71F645F3161F5152F887D0C25C5B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393FE9E42A757851DB64456C44C2A25A85D43B2D9A8A6D044F5BDE549E069FF289AE49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit][v1] sysprof: allow calling sysprof.report before stopping 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for the fixes! LGTM, with the ignorable last nit below. On 04.06.25, Sergey Bronnikov wrote: > Hi, Sergey, > > thanks for the comments! See my replies below. > > > On 6/4/25 11:35, Sergey Kaplun via Tarantool-patches wrote: > > Hi, Sergey! > > Thanks for the patch! > > LGTM, with minor comments below. > > > > On 16.05.25, Sergey Bronnikov wrote: > >> It is not allowed to call a function `sysprof.report()` without > >> stopping profiler. However, sometimes it may be useful to analyze > > Typo: s/profiler/the profiler/ > Fixed. > >> numbers provided by the report without stopping the profiler. The > >> patch removes the appropriate condition and allows reporting > >> without stopping. > >> > >> Resolves tarantool/tarantool#11229 > >> --- > >> Branch:https://github.com/tarantool/luajit/tree/ligurio/gh-11229-misc.sysprof.report > >> Issue:https://github.com/tarantool/tarantool/issues/11229 > >> > >> src/lj_sysprof.c | 2 -- > >> .../profilers/misclib-sysprof-lapi.test.lua | 18 ++++++++++++++++-- > >> 2 files changed, 16 insertions(+), 4 deletions(-) > >> > --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua > +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua > @@ -168,7 +168,7 @@ misc.sysprof.stop() > >  -- Intermediate sysprof.report(). >  res, err, errno = misc.sysprof.start{ > -    mode = "C", > +    mode = "D", >      interval = 1, >      path = "/dev/null", >  } > > > > >> + interval = 1, > >> + path = "/dev/null", > >> +} > >> +test:is(res, true, "res is correct") > >> +test:is(err, nil, "no error") > >> +test:is(errno, nil, "no errno") > > I suppose that 2 last checks are excess. The first one is enough to be > > sure that the profiler is started. Also, we may use `assert()` here > > instead of `test:is()` check, since we don't want to _test_ the starting of > > the profiler only to _assert_ that the sysprof has been started. > > > last two checks were removed and test:is() replaced with assert() I would rather use | assert(misc.sysprof.start({...}) and | assert(misc.sysprof.stop()) instead, for simplicity and to avoid the irrelevant local variables. Feel free to ignore. > >> + > >> +local report = misc.sysprof.report() > >> +test:ok(report.samples == 0, "total number of samples is non-zero") > > I'm not sure that this will always be true (for example, in coverage > > workflow). I suggest increasing the interval dramatically to avoid false > > positives here. > Updated. Side note: Checking the non-0 samples for default payload instead. > >> +misc.sysprof.stop() > >> + > > > > > >> 2.43.0 > >> -- Best regards, Sergey Kaplun