Re: [Tarantool-discussions] [RFC luajit v3] rfc: describe a LuaJIT memory profiler

[Sergey Kaplun via Tarantool-discussions <tarantool-discussions@dev.tarantool.org>]

Hi, Sergos!

Thanks, for the review!

On 20.01.21, Sergey Ostanevich wrote:
> Hi! 
> 
> Thanks for the patch, I've looked into 
> https://github.com/tarantool/tarantool/blob/skaplun/gh-5442-luajit-memory-profiler-rfc/doc/rfc/5442-luajit-memory-profiler.md <https://github.com/tarantool/tarantool/blob/skaplun/gh-5442-luajit-memory-profiler-rfc/doc/rfc/5442-luajit-memory-profiler.md>
> 
> in ‘Prerequisites’: 
> > Also all, deallocations are reported as internal too.
> 
> the comma is not needed

Indeed! Fixed!

> 
> > Lua developers can do nothing with allocations made inside the built-ins except reducing its usage.
> 
> ‘its’ doesn’t explain exact matter. I would rephrase: "As for allocations made inside the built-ins user can do nothing but reduce use of these built-ins."

Thanks, applied!

> 
> > Currently VM state identifies C function execution only, so Fast and Lua functions states are added.
> 
> ‘Currently’ -> ‘Originally’

Fixed, thanks!

See the iterative patch below. Branch is force pushed.

> 
> Otherwise LGTM.
> Sergos
> 

<snipped>

> 

===================================================================
diff --git a/doc/rfc/5442-luajit-memory-profiler.md b/doc/rfc/5442-luajit-memory-profiler.md
index f9c43f91f..cb8adab79 100644
--- a/doc/rfc/5442-luajit-memory-profiler.md
+++ b/doc/rfc/5442-luajit-memory-profiler.md
@@ -32,7 +32,7 @@ This section describes additional changes in LuaJIT required for the feature
 implementation. This version of LuaJIT memory profiler does not support verbose
 reporting for allocations made on traces. All allocation from traces are
 reported as internal. But trace code semantics should be totally the same as
-for the Lua interpreter (excluding sink optimizations). Also all, deallocations
+for the Lua interpreter (excluding sink optimizations). Also all deallocations
 are reported as internal too.
 
 There are two different representations of functions in LuaJIT: the function's
@@ -44,8 +44,8 @@ that is used for LuaJIT built-ins
 Tail call optimization does not create a new call frame, so all allocations
 inside the function called via `CALLT`/`CALLMT` are attributed to its caller.
 
-Lua developers can do nothing with allocations made inside the built-ins except
-reducing its usage. So if fast function is called from a Lua function all
+As for allocations made inside the built-ins user can do nothing but reduce use
+of these built-ins. So if fast function is called from a Lua function all
 allocations made in its scope are attributed to this Lua function (i.e. the
 built-in caller). Otherwise, this event is attributed to a C function.
 
@@ -98,7 +98,7 @@ INTERNAL: 20    0       1481
 ```
 
 So we need to know a type of function being executed by the virtual machine
-(VM). Currently VM state identifies C function execution only, so Fast and Lua
+(VM). Originally VM state identifies C function execution only, so Fast and Lua
 functions states are added.
 
 To determine currently allocating coroutine (that may not be equal to currently
===================================================================

-- 
Best regards,
Sergey Kaplun