Re: [Tarantool-patches] [RFC v4] rfc: luajit metrics

[Sergey Kaplun <skaplun@tarantool.org>]

Igor,

On 08.10.20, Igor Munkin wrote:
> 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.

Removed bullets as we had discussed offline.

@ChangeLog:
 * Introduced LuaJIT platform metrics (gh-5187).

> 
> > 
> > > 
> > > > 
> > > >  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.

OK, added. See iterative patch in the bottom. Branch force-pushed.

> 
> > 
> 
> <snipped>
> 
> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 
> -- 
> Best regards,
> IM

===================================================================
diff --git a/doc/rfc/5187-luajit-metrics.md b/doc/rfc/5187-luajit-metrics.md
index 0c1df6901..db9ec7ee7 100644
--- a/doc/rfc/5187-luajit-metrics.md
+++ b/doc/rfc/5187-luajit-metrics.md
@@ -108,7 +108,9 @@ Couple of words about how metrics are collected:
   BC or builtins this counter is incremented.
 - `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.
+  freed. When a trace is collected by GC this value doesn't change. This area
+  will be reused later for other traces. MCode area is linked with `jit_State`
+  not with trace by itself. Traces just reserve MCode area that needed.
 
 All metrics are collected throughout the platform uptime. These metrics
 increase monotonically and can overflow:
===================================================================

-- 
Best regards,
Sergey Kaplun