Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: Maxim Kokryashkin <max.kokryashkin@gmail.com>,
	tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution
Date: Mon, 4 Sep 2023 08:44:58 +0300	[thread overview]
Message-ID: <tgqs2spzzai6pvvmp6d3ucpcbvq5zf3cvkrbsbd432sbk3loii@jg6vmv5khenu> (raw)
In-Reply-To: <ZPRcjVoYCbbkciWf@root>

Hi, Sergey!
Thanks for the review!
Fixed your comment, the branch is force pushed.

On Sun, Sep 03, 2023 at 01:14:37PM +0300, Sergey Kaplun wrote:
> Hi, Maxim!
> Thanks for the patch!
> LGTM, with a minor nit below.
> 
> On 28.08.23, Maxim Kokryashkin wrote:
> > This patch refactors symbol resolution in memprof, so
> > now they are resolved during the process of parsing.
> > This makes the generation mechanism excessive since
> > symtab updates are no longer affecting the previous events.
> > 
> > Follows up tarantool/tarantool#8700
> > ---
> >  .../misclib-memprof-lapi.test.lua             | 73 +++++--------------
> >  tools/memprof/humanize.lua                    | 16 ++--
> >  tools/memprof/parse.lua                       |  2 +-
> >  tools/memprof/process.lua                     |  8 +-
> >  4 files changed, 31 insertions(+), 68 deletions(-)
> > 
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index 3cb5c8be..0eee4e21 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> 
> <snipped>
> 
> > @@ -86,44 +86,13 @@ local function form_source_line(line, source)
> >  end
> >  
> >  local function form_trace_line(traceno, line, source)
> > -  return ("TRACE [%d] %s:%d"):format(traceno, source or SRC_PATH, line)
> > +  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)
> 
> Nit: Line width is more than 80 symbols.

Here is the diff:
==========================================================================
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index 0eee4e21..b97462a5 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -86,7 +86,11 @@ local function form_source_line(line, source)
 end
 
 local function form_trace_line(traceno, line, source)
-  return ("TRACE [%d] started at %s:%d"):format(traceno, source or SRC_PATH, line)
+  return ("TRACE [%d] started at %s:%d"):format(
+    traceno,
+    source or SRC_PATH,
+    line
+  )
 end
 
 local function fill_ev_type(events, event_type)
==========================================================================
> 
> >  end
> >  
> > -local function fill_ev_type(events, symbols, event_type)
> 
> <snipped>
> 
> > diff --git a/tools/memprof/humanize.lua b/tools/memprof/humanize.lua
> > index 267baa91..5b289ce3 100644
> > --- a/tools/memprof/humanize.lua
> > +++ b/tools/memprof/humanize.lua
> 
> <snipped>
> 
> > diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
> > index d865267b..1b6eceb4 100644
> > --- a/tools/memprof/parse.lua
> > +++ b/tools/memprof/parse.lua
> 
> <snipped>
> 
> > diff --git a/tools/memprof/process.lua b/tools/memprof/process.lua
> > index 0bcb965b..faf4be4e 100644
> > --- a/tools/memprof/process.lua
> > +++ b/tools/memprof/process.lua
> 
> <snipped>
> 
> > -- 
> > 2.41.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

  reply	other threads:[~2023-09-04  5:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-28 15:23 [Tarantool-patches] [PATCH luajit 0/2] profilers: purge generations Maxim Kokryashkin via Tarantool-patches
2023-08-28 15:23 ` [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution Maxim Kokryashkin via Tarantool-patches
2023-09-03 10:14   ` Sergey Kaplun via Tarantool-patches
2023-09-04  5:44     ` Maxim Kokryashkin via Tarantool-patches [this message]
2023-09-05 11:47   ` Sergey Bronnikov via Tarantool-patches
2023-09-08 11:04     ` Maxim Kokryashkin via Tarantool-patches
2023-10-06 14:52       ` Sergey Bronnikov via Tarantool-patches
2023-08-28 15:23 ` [Tarantool-patches] [PATCH luajit 2/2] profilers: purge generation mechanism Maxim Kokryashkin via Tarantool-patches
2023-09-03 10:21   ` Sergey Kaplun via Tarantool-patches
2023-09-04  5:47     ` Maxim Kokryashkin via Tarantool-patches
2023-09-05 11:53   ` Sergey Bronnikov via Tarantool-patches
2023-09-12 10:26     ` Maxim Kokryashkin via Tarantool-patches
2023-09-14  9:47       ` Sergey Bronnikov via Tarantool-patches
2023-09-18  9:18         ` Maxim Kokryashkin via Tarantool-patches
2023-10-06 14:43           ` Sergey Bronnikov via Tarantool-patches
2023-10-06 14:45             ` Sergey Bronnikov via Tarantool-patches
2023-10-06 14:40       ` Sergey Bronnikov via Tarantool-patches
2023-10-06 14:52       ` Sergey Bronnikov via Tarantool-patches
2023-11-13 18:48       ` Igor Munkin via Tarantool-patches
2023-11-23  6:32 ` [Tarantool-patches] [PATCH luajit 0/2] profilers: purge generations Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tgqs2spzzai6pvvmp6d3ucpcbvq5zf3cvkrbsbd432sbk3loii@jg6vmv5khenu \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 1/2] memprof: refactor symbol resolution' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox