Re: [Tarantool-patches] [PATCH 1/2] metrics: add counters for metrics interested in

[Sergey Kaplun <skaplun@tarantool.org>]

Igor,

On 23.07.20, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! Please consider my comments below.
> 
> On 21.07.20, Sergey Kaplun wrote:
> 
<snipped>
> 
> This counter is not incremented if cdata is allocated on trace. Consider
> the following test:
> | $ LUA_PATH="$HOME/.luarocks/share/lua/5.1/?.lua;;" ./luajit -joff <<EOF
> | heredoc> local t = { }
> | heredoc> for i = 1, 1000 do
> | heredoc>   table.insert(t, require('ffi').new('uint64_t', i))
> | heredoc> end
> | heredoc> t = nil
> | heredoc> collectgarbage('collect')
> | heredoc> print(require('inspect')(misc.getmetrics()))
> | heredoc> EOF
> | {
> |   global = {
> |     cdatanum = 0,
> |     gc_total = 74722,
> |     jit_mcode_size = 0,
> |     jit_trace_num = 0,
> |     strnum = 534,
> |     tabnum = 77,
> |     udatanum = 5
> |   },
> |   incremental = {
> |     gc_allocated = 173092,
> |     gc_freed = 98370,
> |     gc_steps_atomic = 1,
> |     gc_steps_finalize = 0,
> |     gc_steps_pause = 3,
> |     gc_steps_propagate = 553,
> |     gc_steps_sweep = 64,
> |     gc_steps_sweepstring = 1024,
> |     jit_snap_restore = 0,
> |     jit_trace_abort = 0,
> |     strhash_hit = 4021,
> |     strhash_miss = 538
> |   }
> | }
> | $ LUA_PATH="$HOME/.luarocks/share/lua/5.1/?.lua;;" ./luajit -jon <<EOF
> | heredoc> local t = { }
> | heredoc> for i = 1, 1000 do
> | heredoc>   table.insert(t, require('ffi').new('uint64_t', i))
> | heredoc> end
> | heredoc> t = nil
> | heredoc> collectgarbage('collect')
> | heredoc> print(require('inspect')(misc.getmetrics()))
> | heredoc> EOF
> | {
> |   global = {
> |     cdatanum = -941,
> |     gc_total = 76561,
> |     jit_mcode_size = 65536,
> |     jit_trace_num = 3,
> |     strnum = 534,
> |     tabnum = 77,
> |     udatanum = 5
> |   },
> |   incremental = {
> |     gc_allocated = 145299,
> |     gc_freed = 68738,
> |     gc_steps_atomic = 3,
> |     gc_steps_finalize = 0,
> |     gc_steps_pause = 2,
> |     gc_steps_propagate = 505,
> |     gc_steps_sweep = 64,
> |     gc_steps_sweepstring = 1024,
> |     jit_snap_restore = 6,
> |     jit_trace_abort = 0,
> |     strhash_hit = 3085,
> |     strhash_miss = 538
> |   }
> | }
> 
> As you can see global.cdatanum is negative when JIT is on. You can take
> a look on the compiled trace by yourself. The root cause is CNEW IR
> semantics: for VLA/VLS cdata types <lj_cdata_newv> call is emitted and
> the counters are fine, but for other cdata types JIT compiler emits just
> a <lj_mem_newgco> call that so this object is not counted. Thereby JIT
> has to emit the corresponding instructions while assembling CNEW IR.
> I will elaborate on this later.

Can we use an approach like this?

diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h
index 3e189b1..a32a8c4 100644
--- a/src/lj_asm_x86.h
+++ b/src/lj_asm_x86.h
@@ -1835,6 +1835,12 @@ static void asm_cnew(ASMState *as, IRIns *ir)
     return;
   }
 
+  /* Global state live longer than given trace.
+  ** Increment cdatanum counter by address directly.
+  */
+  emit_gmroi(as, XG_ARITHi(XOg_ADD), RID_NONE,
+        ptr2addr(&J2G(as->J)->gc.cdatanum), (int32_t)1);
+
   /* Combine initialization of marked, gct and ctypeid. */
   emit_movtomro(as, RID_ECX, RID_RET, offsetof(GCcdata, marked));
   emit_gri(as, XG_ARITHi(XOg_OR), RID_ECX,

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

I've fixed your other comments in [1].

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018866.html

-- 
Best regards,
Sergey Kaplun