Re: [Tarantool-patches] [PATCH luajit v1 09/11] misc: add Lua API for memory profiler

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks for the patch!
Although, the patch might be outdate, still I have some comments below
that are still relevant in branch.

> diff --git a/src/lib_misc.c b/src/lib_misc.c
> index 6f7b9a9..d3b5ab4 100644
> --- a/src/lib_misc.c
> +++ b/src/lib_misc.c
<snipped>
> +
> +/* local started, err, errno = misc.memprof.start(fname) */
> +LJLIB_CF(misc_memprof_start)
> +{
> +  struct luam_Prof_options opt = {0};
> +  struct memprof_ctx *ctx;
> +  const char *fname;
> +  int memprof_status;
> +  int started;
> +
> +  fname = strdata(lj_lib_checkstr(L, 1));
> +
> +  ctx = lj_mem_new(L, sizeof(*ctx));
> +  if (ctx == NULL)
> +    goto errmem;
> +
> +  opt.ctx = ctx;
> +  opt.writer = buffer_writer_default;
> +  opt.on_stop = on_stop_cb_default;
> +  opt.len = STREAM_BUFFER_SIZE;
> +  opt.buf = (uint8_t *)lj_mem_new(L, STREAM_BUFFER_SIZE);
> +  if (NULL == opt.buf) {
> +    lj_mem_free(G(L), ctx, sizeof(*ctx));
> +    goto errmem;
> +  }
> +
> +  ctx->g = G(L);
> +  ctx->stream = fopen(fname, "wb");
> +
> +  if (ctx->stream == NULL) {
> +    memprof_ctx_free(ctx, opt.buf);
> +    return luaL_fileresult(L, 0, fname);
> +  }
> +
> +  memprof_status = ljp_memprof_start(L, &opt);
> +  started = memprof_status == LUAM_PROFILE_SUCCESS;
> +
> +  if (LJ_UNLIKELY(!started)) {
> +    fclose(ctx->stream);
> +    remove(fname);
> +    memprof_ctx_free(ctx, opt.buf);
> +    switch (memprof_status) {
> +    case LUAM_PROFILE_ERRRUN:
> +      lua_pushnil(L);
> +      setstrV(L, L->top++, lj_err_str(L, LJ_ERR_PROF_ISRUNNING));
> +      return 2;
> +    case LUAM_PROFILE_ERRMEM:
> +      /* Unreachable for now. */
> +      goto errmem;
> +    case LUAM_PROFILE_ERRIO:
> +      return luaL_fileresult(L, 0, fname);
> +    default:
> +      break;

This one means, that you can return ‘false’ without any err/errno set.

> +    }
> +  }
> +  lua_pushboolean(L, started);
> +
> +  return 1;
> +errmem:
> +  lua_pushnil(L);
> +  setstrV(L, L->top++, lj_err_str(L, LJ_ERR_ERRMEM));
> +  return 2;
> +}
> +
> +/* local stopped, err, errno = misc.memprof.stop() */
> +LJLIB_CF(misc_memprof_stop)
> +{
> +  int status = ljp_memprof_stop();
> +  int stopped_successfully = status == LUAM_PROFILE_SUCCESS;
> +  if (!stopped_successfully) {
> +    switch (status) {
> +    case LUAM_PROFILE_ERRRUN:
> +      lua_pushnil(L);
> +      setstrV(L, L->top++, lj_err_str(L, LJ_ERR_PROF_NOTRUNNING));
> +      return 2;
> +    case LUAM_PROFILE_ERRIO:
> +      return luaL_fileresult(L, 0, NULL);
> +    default:
> +      break;

Ditto

> +    }
> +  }
> +  lua_pushboolean(L, stopped_successfully);
> +  return 1;
> +}
> +