From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <imun@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (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 1057A469719
 for <tarantool-patches@dev.tarantool.org>;
 Thu,  8 Oct 2020 23:36:49 +0300 (MSK)
Date: Thu, 8 Oct 2020 23:26:13 +0300
From: Igor Munkin <imun@tarantool.org>
Message-ID: <20201008202613.GW18920@tarantool.org>
References: <cover.1601878708.git.skaplun@tarantool.org>
 <20201005063029.31737-1-skaplun@tarantool.org>
 <20201008172512.GU18920@tarantool.org> <20201008192954.GA8864@root>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20201008192954.GA8864@root>
Subject: Re: [Tarantool-patches] [RFC v4] rfc: luajit metrics
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Sergey,

Thanks, the RFC LGTM in general now, but please consider the last minor
comments below.

On 08.10.20, Sergey Kaplun wrote:
> Hi, Igor! Thanks for the review!
> 
> On 08.10.20, Igor Munkin wrote:
> > Sergey,
> > 

<snipped>

> > 
> > Minor: ChangeLog is misordered (the latest changes are the first entry).
> 
> Thanks!
> 
> Forgot to add ChangeLog to patch:
> @ChangeLog:
>  * Add Lua and C API for LuaJIT platform metrics about:
>    - overall amount of allocated tables, cdata and udata objects
>    - number of incremental GC steps grouped by GC state
>    - number of string hashes hits and misses
>    - amount of allocated and freed memory
>    - number of trace aborts, number of traces and restored snapshots

The related issue should be also mentioned here.

> 
> > 
> > > 
> > >  doc/rfc/5187-luajit-metrics.md | 299 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 299 insertions(+)
> > >  create mode 100644 doc/rfc/5187-luajit-metrics.md
> > > 
> > > diff --git a/doc/rfc/5187-luajit-metrics.md b/doc/rfc/5187-luajit-metrics.md
> > > new file mode 100644
> > > index 000000000..02f5b559f
> > > --- /dev/null
> > > +++ b/doc/rfc/5187-luajit-metrics.md
> > > @@ -0,0 +1,299 @@
> > 

<snipped>

> 
> > 
> > > +- `jit_mcode_size` -- whenever new MCode area is allocated `jit_mcode_size` is
> > > +  increased at corresponding size in bytes. Sets to 0 when all mcode area is
> > > +  freed.
> > 
> > How does it change, when a trace is collected as a result of its flush?
> 
> It doesn't. IINM, this area will be reused later for other traces.
> MCode area is linked with jit_State not with trace by itself. Trace just
> reserve MCode area that needed.

I guess this should be explicitly mentioned here then.

> 

<snipped>

> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM