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 2A7286ECE3; Wed, 24 Nov 2021 19:45:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2A7286ECE3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1637772331; bh=z1U3AGN1HsSSLX1HyXGOIskHsOA5wTLeevBWfy7Nqk0=; 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=BMPa6izGXzFVMbqFPvU8U/wyDn4Mo08GM5GQ4brZhFYUmDZCHKvYWalgBM0nt8mWC qykOdQOj6Rzfr32m7+b7jo7cXJ/kcXlhMWVdSCYLNQruDwmt7b1a4enymyXaEoUXUd qMuRPAeKCsPHwFl67JyKxsa029K6tJKOcZSoe40M= 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 D61046ECE3 for ; Wed, 24 Nov 2021 19:45:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D61046ECE3 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mpvOi-0002aB-PH; Wed, 24 Nov 2021 19:45:29 +0300 Date: Wed, 24 Nov 2021 19:44:58 +0300 To: Mikhail Shishatskiy Message-ID: References: <20210929200758.149446-1-m.shishatskiy@tarantool.org> <20210929200758.149446-5-m.shishatskiy@tarantool.org> <20211101163113.GE8831@tarantool.org> <20211104130228.x6qcne5xeh544hm7@surf.localdomain> <20211122151119.ilbfcxcux4kpzrqy@surf.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20211122151119.ilbfcxcux4kpzrqy@surf.localdomain> X-Clacks-Overhead: GNU Terry Pratchett X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9FE0487E50246814620C6330E291889C60F1CC786531A6181182A05F538085040D9687E582B282C10DA8EDD34946038CCD641E520487E2B2192790703345D0265 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE77633BACAB33B9508C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7EB05B7739F1E6D56EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F200B6A155FBA50022360E7AD3F3727526CC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8062BEEFFB5F8EA3EF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6753C3A5E0A5AB5B7089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5DEED0CD07041FEBCB61C9F0A9CA6241A459AA018B8A3709ED59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7506FE1F977233B9BB410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34AF61ACC197BF517A82AB79D83A831D67832EA56A57519DBFDA1F5AC11C310BB370CABEB1883708881D7E09C32AA3244C6D0AFB52614610EB5B0FDCC6C00D45A23A92A9747B6CC886927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFw1tprMwPO0Gvc7iVdH2/g== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D62599DFB7285F83B32897E1E453501AFA7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v4 4/4] memprof: add info about trace start to symtab 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" Misha, Thanks for the fixes! Considering your next reply with reverting the remaining excess changes, LGTM. On 22.11.21, Mikhail Shishatskiy wrote: > Igor, > Thank you for the review! Hope, I got your comments right :) Yes, everything is fine now :) > > >> >> + -- recording started. > >> >> + if trace and trace.addr == loc.addr then > >> >> + return symtab.demangle(symbols, loc).." started at ".. > >> >> + symtab.demangle(symbols, trace.start) > >> > > >> >Finally, I got the thing that bothers me the most. Why do you make > >> > so complex? It looks that you can move all these > >> >if-else branching to and concatenation to > >> > function, doesn't it? AFAICS, you can remove > >> > as a result and trace demangling will be > >> >encapsulated in scope of function. Feel free to correct > >> >me if I'm wrong. > >> > >> Initially it was implemented, as you suggest now. But Sergey in his > >> review led me to believe, that "started at" part should ideologically > >> relate to the humanizer module. And I agree with that point, but maybe > >> I decomposed things not in a very good way. > > > >Em... In that way all other types (such as "INTERNAL" and "CFUNC %#x") > >should also be in the humanizer module, since this representation is > >specific for a particular output format. All in all nobody stops you > >from moving to the humanize module, since it's used > >only there (and need to be used only there). > > > >BTW, Sergey is also in Cc, so he can also drop a few words regarding it. > > > >> > >> Another way to implement this is to demangle without "started at" and > >> then insert it to the demangled name. What do you think? > > > >My point is to have the whole "stringification" mess encapsulated in a > >single function (like it's almost done within ). And > >the only thing remaining outside of this function is "started at" tail. > >I hope this fits your vision regarding decomposition :) > > So, I decided to decompose things, as you suggested initially. Now > traces are demangled separately inside the function. > The "started at" string also moved to that function in order not to > spout some convoluted logic of injecting this string inside the > demangled string outside the demangler module. The only nuance is > a possibility of recursion calls: > > | symtab.demangle -> demangle_trace -> symtab.demangle -> demangle_trace -> ... > > But I inserted asserts to prevent such cases. Neato, thanks a lot! > > > -- > Best regards, > Mikhail Shishatskiy -- Best regards, IM