[Tarantool-patches] [PATCH luajit] Flush and close output file after profiling run.

[Tarantool-patches] [PATCH luajit] Flush and close output file after profiling run.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Sergey Ostanevich.

Using `jit.p` with a file argument does not flush result to the
file on profile stop.

This patch adds missing close for the file descriptor on profile stop.

Sergey Kaplun:
* added the description and the test for the problem
---

LuaJIT PR: https://github.com/LuaJIT/LuaJIT/pull/726
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-726-profile-flush-close-full-ci
Tarantool PR: https://github.com/tarantool/tarantool/pull/7128

 src/jit/p.lua                                 |  1 +
 .../lj-726-profile-flush-close.test.lua       | 29 +++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 test/tarantool-tests/lj-726-profile-flush-close.test.lua

diff --git a/src/jit/p.lua b/src/jit/p.lua
index 7be10586..4569d69e 100644
--- a/src/jit/p.lua
+++ b/src/jit/p.lua
@@ -238,6 +238,7 @@ local function prof_finish()
     prof_count1 = nil
     prof_count2 = nil
     prof_ud = nil
+    if out ~= stdout then out:close() end
   end
 end
 
diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
new file mode 100644
index 00000000..290bd722
--- /dev/null
+++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
@@ -0,0 +1,29 @@
+local tap = require('tap')
+
+local test = tap.test('lj-726-profile-flush-close')
+test:plan(1)
+
+local TEST_FILE = 'lj-726-profile-flush-close.profile'
+
+local function payload()
+  local r = 0
+  for i = 1, 1e8 do
+    r = r + i
+  end
+  return r
+end
+
+local p = require('jit.p')
+p.start("f", TEST_FILE)
+payload()
+p.stop()
+
+local f, err = io.open(TEST_FILE)
+assert(f, err)
+
+-- Check that file is not empty.
+test:ok(f:read(0), 'profile output was flushed and closed')
+
+assert(os.remove(TEST_FILE))
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1

Re: [Tarantool-patches] [PATCH luajit] Flush and close output file after profiling run.

[sergos via Tarantool-patches]

Hi!

Thanks for the patch!
LGTM.

Sergos


> On 12 May 2022, at 13:09, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> Thanks to Sergey Ostanevich.
> 
> Using `jit.p` with a file argument does not flush result to the
> file on profile stop.
> 
> This patch adds missing close for the file descriptor on profile stop.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> ---
> 
> LuaJIT PR: https://github.com/LuaJIT/LuaJIT/pull/726
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-726-profile-flush-close-full-ci
> Tarantool PR: https://github.com/tarantool/tarantool/pull/7128
> 
> src/jit/p.lua                                 |  1 +
> .../lj-726-profile-flush-close.test.lua       | 29 +++++++++++++++++++
> 2 files changed, 30 insertions(+)
> create mode 100644 test/tarantool-tests/lj-726-profile-flush-close.test.lua
> 
> diff --git a/src/jit/p.lua b/src/jit/p.lua
> index 7be10586..4569d69e 100644
> --- a/src/jit/p.lua
> +++ b/src/jit/p.lua
> @@ -238,6 +238,7 @@ local function prof_finish()
>     prof_count1 = nil
>     prof_count2 = nil
>     prof_ud = nil
> +    if out ~= stdout then out:close() end
>   end
> end
> 
> diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> new file mode 100644
> index 00000000..290bd722
> --- /dev/null
> +++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> @@ -0,0 +1,29 @@
> +local tap = require('tap')
> +
> +local test = tap.test('lj-726-profile-flush-close')
> +test:plan(1)
> +
> +local TEST_FILE = 'lj-726-profile-flush-close.profile'
> +
> +local function payload()
> +  local r = 0
> +  for i = 1, 1e8 do
> +    r = r + i
> +  end
> +  return r
> +end
> +
> +local p = require('jit.p')
> +p.start("f", TEST_FILE)
> +payload()
> +p.stop()
> +
> +local f, err = io.open(TEST_FILE)
> +assert(f, err)
> +
> +-- Check that file is not empty.
> +test:ok(f:read(0), 'profile output was flushed and closed')

I wonder if this will ever cause flakiness? 

> +
> +assert(os.remove(TEST_FILE))
> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.34.1
> 

Re: [Tarantool-patches] [PATCH luajit] Flush and close output file after profiling run.

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch! LGTM.

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] Flush and close output file after profiling run.

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patch into all long-term branches in tarantool/luajit
and bumped a new version in master, 2.10 and 1.10.

On 12.05.22, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> Thanks to Sergey Ostanevich.
> 
> Using `jit.p` with a file argument does not flush result to the
> file on profile stop.
> 
> This patch adds missing close for the file descriptor on profile stop.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> ---
> 
> LuaJIT PR: https://github.com/LuaJIT/LuaJIT/pull/726
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-726-profile-flush-close-full-ci
> Tarantool PR: https://github.com/tarantool/tarantool/pull/7128
> 
>  src/jit/p.lua                                 |  1 +
>  .../lj-726-profile-flush-close.test.lua       | 29 +++++++++++++++++++
>  2 files changed, 30 insertions(+)
>  create mode 100644 test/tarantool-tests/lj-726-profile-flush-close.test.lua
> 

<snipped>

> -- 
> 2.34.1
> 

-- 
Best regards,
IM