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 0DE706F873; Fri, 28 Jan 2022 17:16:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0DE706F873 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1643379411; bh=cgjYAZwCj4BWO1OBOU+GEMR/t7H2YL8dGbpG1taFpMo=; h=Date:In-Reply-To:To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=KuzpXVs/ZWFsSVjTCHUsPT+rhvw4fJznn7YAR7k0nWMneFA6uSSrArXTJrV2IOWxd wSSo8FKXEFYUUcuhFTPdM/nbL7jiDbRuKcWq7ehAB/cBk7D2YDT5H/kCmRM7ZLG+eh 6Fao6NyWaW+PXej7WcInAeFbdBYxGe0EzAUzPUss= Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 EF6086F873 for ; Fri, 28 Jan 2022 17:16:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EF6086F873 Received: by smtp33.i.mail.ru with esmtpa (envelope-from ) id 1nDS3V-00014o-4X; Fri, 28 Jan 2022 17:16:49 +0300 Message-Id: <671DCAEC-D19C-409F-89D4-0065C8292D34@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_E60BC263-7B6C-4427-9FF1-AB77BC4930B4" Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) Date: Fri, 28 Jan 2022 17:16:47 +0300 In-Reply-To: To: Sergey Kaplun References: <20211229122731.31617-1-skaplun@tarantool.org> X-Mailer: Apple Mail (2.3654.120.0.1.13) X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD99F281FB7F96F126D2655BC70899A3332D9DD7D630E6E493F00894C459B0CD1B9B9FA02E701B52030B6DFFA88553827F32B7F9B41A2710632C752CF1D619E8C08 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A140E7B1D51EB231EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006372338AE33E473C9B88638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D862E07D08175A5F96260BD4F68DA9D403117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC974A882099E279BDA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6300D3B61E77C8D3B089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5CE233779833DDA471793883C525AC01294C93FC093CB5E3DD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75F04B387B5D7535DE410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D340EEB18E491831571D9F46FFE4F8A4DCCF69E76218DE7FB32E1566A1D856251D7EB48B0667A03B7451D7E09C32AA3244C035BC5249E9B80C9F1B7CE1D46BE072830452B15D76AEC14FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojqLK0kLh8sGeFqZ1Aogt84g== X-Mailru-Sender: 3B9A0136629DC912F4AABCEFC589C81E76F7DCBE1C08BC9EDE92A2FD28D975E6EA05D070C7EB72F9AD07DD1419AC565FA614486B47F28B67C5E079CCF3B0523AED31B7EB2E253A9EB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Actually implement maxirconst trace limit. 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: sergos via Tarantool-patches Reply-To: sergos Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" --Apple-Mail=_E60BC263-7B6C-4427-9FF1-AB77BC4930B4 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi! I reviewed the one at = https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f37489dd842d9a6= b068cf12c = Overall is LGTM, I would like to update the 2nd test message to = something like =E2=80=99trace should not appear due to maxirconst limit=E2=80=99 Sergos > On 28 Jan 2022, at 15:35, Sergey Kaplun wrote: >=20 > Igor, >=20 > On 28.01.22, Igor Munkin wrote: >> Sergey, >>=20 >> On 28.01.22, Sergey Kaplun wrote: >>> Igor, >>>=20 >>> Thanks for the review! >>>=20 >>=20 >> >>=20 >>>>>=20 >>>>> Issue: https://github.com/LuaJIT/LuaJIT/issues/430 >>>>> Branch: = https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirconst-actuall= y-implement-full-ci >>>>> Tarantool branch: = https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxirconst-actu= ally-implement-full-ci >>>>>=20 >>>>> src/lj_record.c | 5 ++- >>>>> .../lj-430-maxirconst.test.lua | 43 = +++++++++++++++++++ >>>>> 2 files changed, 46 insertions(+), 2 deletions(-) >>>>> create mode 100644 test/tarantool-tests/lj-430-maxirconst.test.lua >>>>>=20 >>>>=20 >>>> >>>>=20 >>>>> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua = b/test/tarantool-tests/lj-430-maxirconst.test.lua >>>>> new file mode 100644 >>>>> index 00000000..1829b37d >>>>> --- /dev/null >>>>> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua >>>>> @@ -0,0 +1,43 @@ >>>>> +-- XXX: avoid any other traces compilation due to hotcount >>>>> +-- collisions for predictible results. >>=20 >> >>=20 >>>>> +jit.off() >>>>> +jit.flush() >>>>=20 >>>> Minor: I'd rather move this part closer to 'jit.opt.start' to save = the >>>> test structure closer to the other test chunks. Feel free to = ignore. >>>=20 >>> I really want to exclude __any__ JIT work here to avoid = false-positive >>> hotcount (was precendents during writing this test :)). Ignoring. >>=20 >> OK, got it. >>=20 >>>=20 >>=20 >> >>=20 >>>>> +test:ok(ntrace_old + 1 =3D=3D misc.getmetrics().jit_trace_num, >>>>> + 'trace number increases') >>>>=20 >>>> Typo: I doubt we use tabs in Lua sources, but I might be wrong... >>>=20 >>> I suggest to use quarter tabs indent style as we use for C code and = Mike >>> use in src/jit/*.lua (see bcsave.lua for example). >>>=20 >>> There was no precedent before, IINM :). >>=20 >> See test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1]. >=20 > Fixed, repushed. >=20 >>=20 >>>=20 >>>>=20 >>>>> + >>>>> +ntrace_old =3D misc.getmetrics().jit_trace_num >>>>> +jit.on() >>>>> +irconst4() >>>>> +irconst4() >>>>> +jit.off() >>>>> +test:ok(ntrace_old =3D=3D misc.getmetrics().jit_trace_num, >>>>> + 'trace number is the same') >>>>=20 >>>> Ditto. >>>>=20 >>>>> + >>>>> +os.exit(test:check() and 0 or 1) >>>>> --=20 >>>>> 2.34.1 >>>>>=20 >>>>=20 >>>> --=20 >>>> Best regards, >>>> IM >>>=20 >>> --=20 >>> Best regards, >>> Sergey Kaplun >>=20 >> [1]: = https://github.com/tarantool/luajit/blob/tarantool/test/tarantool-tests/lj= -695-ffi-vararg-call.test.lua >>=20 >> --=20 >> Best regards, >> IM >=20 > --=20 > Best regards, > Sergey Kaplun --Apple-Mail=_E60BC263-7B6C-4427-9FF1-AB77BC4930B4 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hi!

I = reviewed the one at https://github.com/tarantool/luajit/commit/2cdd5ac0367629d2f374= 89dd842d9a6b068cf12c

Overall is LGTM, I would like to update the 2nd test message = to something like
=E2=80=99trace should not appear = due to maxirconst limit=E2=80=99

Sergos

On 28 = Jan 2022, at 15:35, Sergey Kaplun <skaplun@tarantool.org> wrote:

Igor,

On 28.01.22, Igor Munkin wrote:
Sergey,

On= 28.01.22, Sergey Kaplun wrote:
Igor,

Thanks for the review!


<snipped>


Issue: https://github.com/LuaJIT/LuaJIT/issues/430
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-430-maxirco= nst-actually-implement-full-ci
Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/lj-430-maxi= rconst-actually-implement-full-ci

src/lj_record.c =             &n= bsp;           &nbs= p;     |  5 ++-
.../lj-430-maxirconst.test.lua =             &n= bsp;  | 43 +++++++++++++++++++
2 files changed, = 46 insertions(+), 2 deletions(-)
create mode 100644 = test/tarantool-tests/lj-430-maxirconst.test.lua


<snipped>

diff --git = a/test/tarantool-tests/lj-430-maxirconst.test.lua = b/test/tarantool-tests/lj-430-maxirconst.test.lua
new file = mode 100644
index 00000000..1829b37d
--- = /dev/null
+++ = b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -0,0 = +1,43 @@
+-- XXX: avoid any other traces compilation due = to hotcount
+-- collisions for predictible results.

<snipped>

+jit.off()
+jit.flush()

Minor: I'd rather move this part = closer to 'jit.opt.start' to save the
test structure = closer to the other test chunks. Feel free to ignore.

I really want to exclude __any__ = JIT work here to avoid false-positive
hotcount (was = precendents during writing this test :)). Ignoring.

OK, got it.



<snipped>

+test:ok(ntrace_old + 1 = =3D=3D misc.getmetrics().jit_trace_num,
+ 'trace = number increases')

Typo: I = doubt we use tabs in Lua sources, but I might be wrong...

I suggest to use quarter tabs = indent style as we use for C code and Mike
use in = src/jit/*.lua (see bcsave.lua for example).

There was no precedent before, IINM :).

See = test/tarantool-tests/lj-695-ffi-vararg-call.test.lua[1].

Fixed, repushed.




+
+ntrace_old =3D misc.getmetrics().jit_trace_num
+jit.on()
+irconst4()
+irconst4()
+jit.off()
+test:ok(ntrace_old =3D=3D = misc.getmetrics().jit_trace_num,
+ 'trace = number is the same')

Ditto.

+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1


-- 
Best = regards,
IM

-- 
Best regards,
Sergey Kaplun

[1]: https://github.com/tarantool/luajit/blob/tarantool/test/taranto= ol-tests/lj-695-ffi-vararg-call.test.lua

-- 
Best regards,
IM

-- 
Best = regards,
Sergey = Kaplun

= --Apple-Mail=_E60BC263-7B6C-4427-9FF1-AB77BC4930B4--