Re: [Tarantool-patches] [PATCH luajit v2 5/7] core: introduce memory profiler

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks for the patch! Just one question below, looks good otherwise.

Sergos

> diff --git a/src/lj_memprof.c b/src/lj_memprof.c
> new file mode 100644
> index 0000000..e0df057
> --- /dev/null
> +++ b/src/lj_memprof.c
<snipped>
> +static int memprof_stop(const struct lua_State *L)
> +{
> +  struct memprof *mp = &memprof;
> +  struct alloc *oalloc = &mp->orig_alloc;
> +  struct lj_wbuf *out = &mp->out;
> +  int return_status = PROFILE_SUCCESS;
> +  int saved_errno = 0;
> +  struct lua_State *main_L;
> +  int cb_status;
> +
> +  memprof_lock();
> +
> +  if (mp->state == MPS_HALT) {
> +    errno = mp->saved_errno;
> +    mp->state = MPS_IDLE
> +    memprof_unlock();
> +    return PROFILE_ERRIO;
> +  }
> +
> +  if (mp->state != MPS_PROFILE) {
> +    memprof_unlock();
> +    return PROFILE_ERRRUN;
> +  }
> +

> +  if (L != NULL && mp->g != G(L)) {
> +    memprof_unlock();
> +    return PROFILE_ERR;
> +  }
> +
> +  mp->state = MPS_IDLE;
> +
> +  lua_assert(mp->g != NULL);
> +  main_L = mainthread(mp->g);
> +
> +  lua_assert(memprof_allocf == lua_getallocf(main_L, NULL));
> +  lua_assert(oalloc->allocf != NULL);
> +  lua_assert(oalloc->state != NULL);
> +  lua_setallocf(main_L, oalloc->allocf, oalloc->state);
> +
> +  if (LJ_UNLIKELY(lj_wbuf_test_flag(out, STREAM_STOP))) {
> +    lua_assert(lj_wbuf_test_flag(out, STREAM_ERR_IO));
> +    mp->state = MPS_HALT;
> +    /* on_stop call may change errno value. */
> +    mp->saved_errno = lj_wbuf_errno(out);
> +    /* Ignore possible errors. mp->opt.buf == NULL here. */
> +    mp->opt.on_stop(mp->opt.ctx, mp->opt.buf);
> +    lj_wbuf_terminate(out);
> +    memprof_unlock();
> +    return PROFILE_ERRIO;
> +  }
> +  lj_wbuf_addbyte(out, LJM_EPILOGUE_HEADER);
> +
> +  lj_wbuf_flush(out);
> +
> +  cb_status = mp->opt.on_stop(mp->opt.ctx, mp->opt.buf);
> +  if (LJ_UNLIKELY(lj_wbuf_test_flag(out, STREAM_ERR_IO) || cb_status != 0)) {
> +    saved_errno = lj_wbuf_errno(out);

Previous return of PROFILE_ERRIO causes MPS_HALT. Should it be the same?

> +    return_status = PROFILE_ERRIO;
> +  }
> +
> +  lj_wbuf_terminate(out);
> +
> +  memprof_unlock();
> +  errno = saved_errno;
> +  return return_status;
> +}