[Tarantool-patches] [PATCH v2 1/2] core: introduce various platform metrics
Sergey Kaplun
skaplun at tarantool.org
Thu Sep 3 15:51:45 MSK 2020
Igor,
On 26.08.20, Igor Munkin wrote:
> Sergey,
>
> Thanks for the patch! It looks OK in general, except several nits, I
> left below.
>
> On 26.07.20, Sergey Kaplun wrote:
> > This patch introduces the following counters:
> > - 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 and restored snapshots
>
> Typo: we usually use whitespace prior to the list bullets (as you did in
> the previous version).
Sure, thanks!
>
> >
> > Interfaces to obtain these metrics via both Lua and C API are
> > introduced in the next patch.
> >
> > Part of tarantool/tarantool#5187
> > ---
> > src/lj_cdata.c | 2 ++
> > src/lj_cdata.h | 2 ++
> > src/lj_gc.c | 4 ++++
> > src/lj_gc.h | 6 +-----
> > src/lj_jit.h | 3 +++
> > src/lj_obj.h | 22 ++++++++++++++++++++++
> > src/lj_snap.c | 1 +
> > src/lj_state.c | 2 +-
> > src/lj_str.c | 5 +++++
> > src/lj_tab.c | 2 ++
> > src/lj_trace.c | 5 ++++-
> > src/lj_udata.c | 2 ++
> > 12 files changed, 49 insertions(+), 7 deletions(-)
> >
>
> <snipped>
>
> > diff --git a/src/lj_jit.h b/src/lj_jit.h
> > index 7eb3d2a..90c1914 100644
> > --- a/src/lj_jit.h
> > +++ b/src/lj_jit.h
> > @@ -475,6 +475,9 @@ typedef struct jit_State {
> > size_t szmcarea; /* Size of current mcode area. */
> > size_t szallmcarea; /* Total size of all allocated mcode areas. */
> >
> > + size_t nsnaprestore; /* Overall number of snap restores for this jit_State. */
> > + size_t ntraceabort; /* Overall number of abort traces for this jit_State. */
>
> Why did you emphasize that the counters relate to *this jit_State*?
> There are no such mentions elsewhere.
Well, I suppose that they can be omitted.
>
> > +
> > TValue errinfo; /* Additional info element for trace errors. */
> >
> > #if LJ_HASPROFILE
> > diff --git a/src/lj_obj.h b/src/lj_obj.h
> > index f368578..18df173 100644
> > --- a/src/lj_obj.h
> > +++ b/src/lj_obj.h
>
> <snipped>
>
> > @@ -578,6 +589,9 @@ typedef struct GCState {
> > uint8_t state; /* GC state. */
> > uint8_t nocdatafin; /* No cdata finalizer called. */
> > uint8_t unused2;
> > + size_t freed; /* Total amount of freed memory. */
> > + size_t allocated; /* Total amount of allocated memory. */
> > + size_t state_count[GCSmax]; /* Count of incremental GC steps per state. */
>
> One more time: consider the structure alignment and reorder the
> introduced fields to avoid excess padding.
Sure! I will add all fields to the end of structure.
>
> > MSize sweepstr; /* Sweep position in string table. */
> > GCRef root; /* List of all collectable objects. */
> > MRef sweep; /* Sweep position in root list. */
>
> <snipped>
>
> > @@ -602,6 +622,8 @@ typedef struct global_State {
> > BloomFilter next[2];
> > } strbloom;
> > #endif
> > + size_t strhash_hit; /* Strings amount founded in string hash. */
>
> Typo: s/founded/found/.
Thanks!
>
> > + size_t strhash_miss; /* Strings amount allocated and put into string hash. */
> > lua_Alloc allocf; /* Memory allocator. */
> > void *allocd; /* Memory allocator data. */
> > GCState gc; /* Garbage collector. */
>
> <snipped>
>
> >
> > --
> > 2.24.1
> >
>
> Furthermore, please address the comments I left regarding the patch
> you've made for CNEW IR[1] and squash it with this one.
>
> Sergos, do we need other JIT architectures to be patched in scope of
> this series or Sergey can just add the corresponding preprocessor
> condition to stub the issue for now?
>
>
> [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-August/019012.html
>
I'll add several architectures support with the next series.
> --
> Best regards,
> IM
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list