From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 D53624765E0 for ; Wed, 30 Dec 2020 14:06:55 +0300 (MSK) Date: Wed, 30 Dec 2020 14:06:03 +0300 From: Sergey Kaplun Message-ID: <20201230110603.GS14702@root> References: <20201230093916.GX5396@tarantool.org> <20201230095031.GR14702@root> <2B46045C-1624-431A-AC2A-0BA583A45990@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2B46045C-1624-431A-AC2A-0BA583A45990@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review! On 30.12.20, Sergey Ostanevich wrote: > Hi! > > Thanks for the patch! > Just one nit in comments. > > LGTM. > Sergos > > > > > Reworded. See the iterative patch below. Branch is force-pushed. > > > > | core: remove excess assertion inside memprof > > | > > | There are the cases when the memory profiler attempts to attribute > ^^^ remove, just 'there are cases' Thanks! Fixed in the commit message and the comment. Branch is force-pushed. See the iterative diff below. > > > | allocations triggered by JIT engine recording phase with a Lua function > > | to be recorded. At this case lj_debug_frameline() may return BC_NOPOS > > | (i.e. a negative value) so the assertion in the Lua writer > > | memprof_write_lfunc() is violated. > > | > > | This patch removes this assertion. For negative returned line value > > | profiler is reported zero frameline. > > | > > | Follows up tarantool/tarantool#5442 > > > >> > >>> ** -DLUAJIT_DISABLE_DEBUGINFO flag. > >>> */ > >>> - lua_assert(line >= 0); > >>> - lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC); > >>> - lj_wbuf_addu64(out, (uintptr_t)funcproto(fn)); > >>> - lj_wbuf_addu64(out, (uint64_t)line); > >>> + lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0); > >>> } > >>> > >>> static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent, > >>> -- > >>> 2.28.0 > >>> > >> > >> -- > >> Best regards, > >> IM > > > > =================================================================== > > diff --git a/src/lj_memprof.c b/src/lj_memprof.c > > index 0568049..37ec4c9 100644 > > --- a/src/lj_memprof.c > > +++ b/src/lj_memprof.c > > @@ -94,7 +94,10 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent, > > lj_wbuf_addu64(out, (uintptr_t)funcproto(fn)); > > /* > > ** Line is >= 0 if we are inside a Lua function. > > - ** An exception may be when the Lua function is on top. > > + ** There are the cases when the memory profiler attempts > > + ** to attribute allocations triggered by JIT engine recording > > + ** phase with a Lua function to be recorded. At this case > > + ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value). > > ** Equals to zero when LuaJIT is built with the > > ** -DLUAJIT_DISABLE_DEBUGINFO flag. > > */ > > =================================================================== > > > > -- > > Best regards, > > Sergey Kaplun > =================================================================== diff --git a/src/lj_memprof.c b/src/lj_memprof.c index 37ec4c9..2c1ef3b 100644 --- a/src/lj_memprof.c +++ b/src/lj_memprof.c @@ -94,7 +94,7 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent, lj_wbuf_addu64(out, (uintptr_t)funcproto(fn)); /* ** Line is >= 0 if we are inside a Lua function. - ** There are the cases when the memory profiler attempts + ** There are cases when the memory profiler attempts ** to attribute allocations triggered by JIT engine recording ** phase with a Lua function to be recorded. At this case ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value). =================================================================== -- Best regards, Sergey Kaplun