Sergos

On 27 Dec 2020, at 14:54, = Sergey Kaplun <skaplun@tarantool.org> wrote:

Hi, = Sergos!

Thanks for = the review!

On 27.12.20, = Sergey Ostanevich wrote:
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 =3D &memprof;
+ struct = alloc *oalloc =3D &mp->orig_alloc;
+ struct = lj_wbuf *out =3D &mp->out;
+ int = return_status =3D PROFILE_SUCCESS;
+ int saved_errno = =3D 0;
+ struct lua_State *main_L;
+ = int cb_status;
+
+ = memprof_lock();
+
+ if = (mp->state =3D=3D MPS_HALT) {
+    errno = =3D mp->saved_errno;
+    mp->state =3D= MPS_IDLE
+    memprof_unlock();
+    return PROFILE_ERRIO;
+ = }
+
+ if (mp->state !=3D = MPS_PROFILE) {
+    memprof_unlock();
+    return PROFILE_ERRRUN;
+ = }
+

+ if (L !=3D NULL = && mp->g !=3D G(L)) {
+ =    memprof_unlock();
+ =    return PROFILE_ERR;
+ }
+
+ mp->state =3D MPS_IDLE;
+
+ lua_assert(mp->g !=3D NULL);
+ main_L =3D mainthread(mp->g);
+
+ lua_assert(memprof_allocf =3D=3D = lua_getallocf(main_L, NULL));
+ = lua_assert(oalloc->allocf !=3D NULL);
+ = lua_assert(oalloc->state !=3D 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 =3D MPS_HALT;
+ =    /* on_stop call may change errno value. */
+    mp->saved_errno =3D = lj_wbuf_errno(out);
+    /* Ignore possible = errors. mp->opt.buf =3D=3D 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 =3D mp->opt.on_stop(mp->opt.ctx, = mp->opt.buf);
+ if = (LJ_UNLIKELY(lj_wbuf_test_flag(out, STREAM_ERR_IO) || cb_status !=3D 0)) = {
+    saved_errno =3D = lj_wbuf_errno(out);

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

Yes, you're right! Previous MPS_HALT is redundant -- profiler = anyway
should stop = (IDLE) immediately, there is no reason to set HALT there.
Thanks! = Fixed!

See the = iterative diff below. Branch is force-pushed.

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

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
diff --git = a/src/lj_memprof.c b/src/lj_memprof.c
index e0df057..998cbea 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -354,13 +354,13 @@ static int memprof_stop(const struct = lua_State *L)

  if = (LJ_UNLIKELY(lj_wbuf_test_flag(out, STREAM_STOP))) {
    lua_assert(lj_wbuf_test_flag(out, = STREAM_ERR_IO));
-    mp->state =3D MPS_HALT;
    /* on_stop call may change errno = value. */
- =    mp->saved_errno =3D lj_wbuf_errno(out);
+ =    saved_errno =3D lj_wbuf_errno(out);
    /* Ignore possible errors. = mp->opt.buf =3D=3D NULL here. */
    mp->opt.on_stop(mp->opt.ctx, = mp->opt.buf);
    lj_wbuf_terminate(out);
    memprof_unlock();
+ =    errno =3D saved_errno;
    return PROFILE_ERRIO;
  }
  lj_wbuf_addbyte(out, = LJM_EPILOGUE_HEADER);
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

--
Best = regards,
Sergey = Kaplun