From: Mikhail Shishatskiy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 2/7] core: separate the profiling timer from lj_profile
Date: Thu, 23 Sep 2021 14:37:38 +0300 [thread overview]
Message-ID: <20210923113738.ov5lhqlkxx6cmnqg@surf.localdomain> (raw)
In-Reply-To: <YUm+QJQE4hzCdMlB@root>
Hi, Sergey!
Thanks for the review!
On 21.09.2021 14:13, Sergey Kaplun wrote:
>Hi, Maxim, Mikhail!
>
>Thanks for the patch!
>
>Please consider my comments below.
>
>On 08.09.21, Maxim Kokryashkin wrote:
>> From: Mikhail Shishatskiy <m.shishatskiy@tarantool.org>
>>
>> This patch makes timer machinery in lj_profile self-reliant by
>> introducing lj_profile_timer structure and start/stop interface
>> for it. This makes the timer useful for other sampling profiling
>
>Minor: s/This makes the timer useful/This timer is useful/
Fixed.
>
>> features.
>>
>> Part of tarantool/tarantool#781
>> ---
>> src/CMakeLists.txt | 1 +
>> src/Makefile.dep.original | 5 +-
>> src/lj_profile.c | 178 +++-----------------------------------
>> src/lj_profile_timer.c | 126 +++++++++++++++++++++++++++
>> src/lj_profile_timer.h | 83 ++++++++++++++++++
>
>Amalgamated build should be updated as well.
Fixed.
>
>> 5 files changed, 227 insertions(+), 166 deletions(-)
>> create mode 100644 src/lj_profile_timer.c
>> create mode 100644 src/lj_profile_timer.h
>>
>> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>> index 809aac68..50c23cae 100644
>> --- a/src/CMakeLists.txt
>> +++ b/src/CMakeLists.txt
>> @@ -68,6 +68,7 @@ make_source_list(SOURCES_UTILS
>> make_source_list(SOURCES_PROFILER
>> SOURCES
>> lj_memprof.c
>> + lj_profile_timer.c
>> lj_profile.c
>
>Nit: lj_profile.c should go first (alphabetically).
Fixed.
>
>> )
>>
>> diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original
>
><snipped>
>
>> diff --git a/src/lj_profile.c b/src/lj_profile.c
>> index 7b09a63a..e7e1a303 100644
>> --- a/src/lj_profile.c
>> +++ b/src/lj_profile.c
>> @@ -19,66 +19,19 @@
>> #include "lj_trace.h"
>> #endif
>> #include "lj_profile.h"
>> +#include "lj_profile_timer.h"
>>
>> #include "luajit.h"
>>
>> -#if LJ_PROFILE_SIGPROF
>
><snipped>
>
>> -#endif
>> -
>> /* Profiler state. */
>> typedef struct ProfileState {
>> global_State *g; /* VM state that started the profiler. */
>> luaJIT_profile_callback cb; /* Profiler callback. */
>> void *data; /* Profiler callback data. */
>> SBuf sb; /* String buffer for stack dumps. */
>> - int interval; /* Sample interval in milliseconds. */
>> int samples; /* Number of samples for next callback. */
>> int vmstate; /* VM state when profile timer triggered. */
>> -#if LJ_PROFILE_SIGPROF
>> - struct sigaction oldsa; /* Previous SIGPROF state. */
>> -#elif LJ_PROFILE_PTHREAD
>> - pthread_mutex_t lock; /* g->hookmask update lock. */
>> - pthread_t thread; /* Timer thread. */
>> - int abort; /* Abort timer thread. */
>> -#elif LJ_PROFILE_WTHREAD
>> -#if LJ_TARGET_WINDOWS
>> - HINSTANCE wmm; /* WinMM library handle. */
>> - WMM_TPFUNC wmm_tbp; /* WinMM timeBeginPeriod function. */
>> - WMM_TPFUNC wmm_tep; /* WinMM timeEndPeriod function. */
>> -#endif
>> - CRITICAL_SECTION lock; /* g->hookmask update lock. */
>> - HANDLE thread; /* Timer thread. */
>> - int abort; /* Abort timer thread. */
>> -#endif
>> + lj_profile_timer timer; /* Profiling timer */
>
>Minor: Use tabs for allignment here.
Fixed.
>
>> } ProfileState;
>>
>> /* Sadly, we have to use a static profiler state.
>> @@ -145,6 +98,7 @@ void LJ_FASTCALL lj_profile_interpreter(lua_State *L)
>> profile_unlock(ps);
>> }
>>
>> +
>
>This change is excess.
Fixed.
>
>> /* Trigger profile hook. Asynchronous call from OS-specific profile timer. */
>> static void profile_trigger(ProfileState *ps)
>> {
>> @@ -168,129 +122,21 @@ static void profile_trigger(ProfileState *ps)
>> profile_unlock(ps);
>> }
>>
>> -/* -- OS-specific profile timer handling ---------------------------------- */
>> -
>> #if LJ_PROFILE_SIGPROF
>>
>> -/* SIGPROF handler. */
>> -static void profile_signal(int sig)
>> +static void profile_handler(int sig, siginfo_t *info, void* ctx)
>
>Why do we need to change signature of this function?
We need this to make use of extra arguments inside the sysprof.
Yes, we can make this optional, but this will lead to the complicated
API.
>
>Nit: s/void* ctx/void *ctx/
Fixed.
>
>> {
>> UNUSED(sig);
>> + UNUSED(info);
>> + UNUSED(ctx);
>> profile_trigger(&profile_state);
>> }
>>
>> -/* Start profiling timer. */
>> -static void profile_timer_start(ProfileState *ps)
>> -{
>> - int interval = ps->interval;
>> - struct itimerval tm;
>> - struct sigaction sa;
>> - tm.it_value.tv_sec = tm.it_interval.tv_sec = interval / 1000;
>> - tm.it_value.tv_usec = tm.it_interval.tv_usec = (interval % 1000) * 1000;
>> - setitimer(ITIMER_PROF, &tm, NULL);
>> - sa.sa_flags = SA_RESTART;
>> - sa.sa_handler = profile_signal;
>> - sigemptyset(&sa.sa_mask);
>> - sigaction(SIGPROF, &sa, &ps->oldsa);
>> -}
>> -
>> -/* Stop profiling timer. */
>> -static void profile_timer_stop(ProfileState *ps)
>> -{
>
><snipped>
>
>> -}
>> -
>> -#elif LJ_PROFILE_PTHREAD
>> -
>> -/* POSIX timer thread. */
>> -static void *profile_thread(ProfileState *ps)
>> -{
>> - int interval = ps->interval;
>> -#if !LJ_TARGET_PS3
>> - struct timespec ts;
>> - ts.tv_sec = interval / 1000;
>> - ts.tv_nsec = (interval % 1000) * 1000000;
>> -#endif
>> - while (1) {
>> -#if LJ_TARGET_PS3
>> - sys_timer_usleep(interval * 1000);
>> #else
>> - nanosleep(&ts, NULL);
>> -#endif
>> - if (ps->abort) break;
>> - profile_trigger(ps);
>> - }
>> - return NULL;
>> -}
>> -
>> -/* Start profiling timer thread. */
>> -static void profile_timer_start(ProfileState *ps)
>> -{
>
><snipped>
>
>> -}
>>
>> -/* Stop profiling timer thread. */
>> -static void profile_timer_stop(ProfileState *ps)
>> +static void profile_handler()
>> {
>
><snipped>
>
>> -}
>> -
>> -#elif LJ_PROFILE_WTHREAD
>> -
>> -/* Windows timer thread. */
>> -static DWORD WINAPI profile_thread(void *psx)
>> -{
>> - ProfileState *ps = (ProfileState *)psx;
>> - int interval = ps->interval;
>> -#if LJ_TARGET_WINDOWS
>> - ps->wmm_tbp(interval);
>> -#endif
>> - while (1) {
>> - Sleep(interval);
>> - if (ps->abort) break;
>> - profile_trigger(ps);
>> - }
>> -#if LJ_TARGET_WINDOWS
>> - ps->wmm_tep(interval);
>> -#endif
>> - return 0;
>> -}
>> -
>> -/* Start profiling timer thread. */
>> -static void profile_timer_start(ProfileState *ps)
>> -{
>> -#if LJ_TARGET_WINDOWS
>> - if (!ps->wmm) { /* Load WinMM library on-demand. */
>> - ps->wmm = LoadLibraryExA("winmm.dll", NULL, 0);
>> - if (ps->wmm) {
>> - ps->wmm_tbp = (WMM_TPFUNC)GetProcAddress(ps->wmm, "timeBeginPeriod");
>> - ps->wmm_tep = (WMM_TPFUNC)GetProcAddress(ps->wmm, "timeEndPeriod");
>> - if (!ps->wmm_tbp || !ps->wmm_tep) {
>> - ps->wmm = NULL;
>> - return;
>> - }
>> - }
>> - }
>> -#endif
>> - InitializeCriticalSection(&ps->lock);
>> - ps->abort = 0;
>> - ps->thread = CreateThread(NULL, 0, profile_thread, ps, 0, NULL);
>> -}
>> -
>> -/* Stop profiling timer thread. */
>> -static void profile_timer_stop(ProfileState *ps)
>> -{
>
><snipped>
>
>> }
>>
>> #endif
>> @@ -327,12 +173,14 @@ LUA_API void luaJIT_profile_start(lua_State *L, const char *mode,
>> if (ps->g) return; /* Profiler in use by another VM. */
>> }
>> ps->g = G(L);
>> - ps->interval = interval;
>> ps->cb = cb;
>> ps->data = data;
>> ps->samples = 0;
>> lj_buf_init(L, &ps->sb);
>> - profile_timer_start(ps);
>> +
>
>Nit: I think this empty line is excess.
>Feel free to ignore.
Fixed.
>
>> + ps->timer.opt.interval_msec = interval;
>> + ps->timer.opt.handler = profile_handler;
>> + lj_profile_timer_start(&ps->timer);
>> }
>>
>> /* Stop profiling. */
>> @@ -341,7 +189,7 @@ LUA_API void luaJIT_profile_stop(lua_State *L)
>
><snipped>
>
>> diff --git a/src/lj_profile_timer.c b/src/lj_profile_timer.c
>> new file mode 100644
>> index 00000000..ce420a85
>> --- /dev/null
>> +++ b/src/lj_profile_timer.c
>> @@ -0,0 +1,126 @@
>> +/*
>> +** Simple profiling timer extracted from inbuilt luajit profiler
>
>Nit: I suppose we can omit the part about extracting.
Fixed. Also, copyright note copied from the lj_profile
>
>> +*/
>> +
>> +#define lj_profile_timer_c
>> +#define LUA_CORE
>> +
>> +#include "lj_profile_timer.h"
>> +
>> +#if LJ_HASPROFILE
>> +
>> +#if LJ_PROFILE_SIGPROF
>> +
>> +/* Start profiling timer. */
>> +void lj_profile_timer_start(lj_profile_timer *timer) {
>
>Minor: use the newline for curly bracket after function declaration.
>Here and below.
Fixed.
>
>> + int interval = timer->opt.interval_msec;
>
>Minor: looks like interval may be declared as `const int`.
>Feel free to ignore.
Fixed.
>
>> + struct itimerval tm;
>> + struct sigaction sa;
>> + tm.it_value.tv_sec = tm.it_interval.tv_sec = interval / 1000;
>> + tm.it_value.tv_usec = tm.it_interval.tv_usec = (interval % 1000) * 1000;
>> + setitimer(ITIMER_PROF, &tm, NULL);
>> + sa.sa_flags = SA_RESTART | SA_SIGINFO;
>> + sa.sa_sigaction = timer->opt.handler;
>
>Why do we need to forcify using of 3 argument handler?
>
>| - sa.sa_flags = SA_RESTART;
>| - sa.sa_handler = profile_signal;
>
>Looks like the old format is enough.
Answered above.
>
>> + sigemptyset(&sa.sa_mask);
>> + sigaction(SIGPROF, &sa, &timer->oldsa);
>> +}
>> +
>> +/* Stop profiling timer. */
>> +void lj_profile_timer_stop(lj_profile_timer *timer) {
>
><snipped>
>
>> +}
>> +
>> +#elif LJ_PROFILE_PTHREAD
>> +
>> +/* POSIX timer thread. */
>> +static void *profile_thread(lj_timer *timer) {
>
>Its lj_profile_timer, not lj_timer :).
Fixed.
>
>Also, `timer_thread()` name is looks better here, as far as it only
>handler timer in another thread.
>Thoughts?
Seems reasonable, fixed.
>
>> + int interval = timer->opt.interval_msec;
>> +#if !LJ_TARGET_PS3
>> + struct timespec ts;
>> + ts.tv_sec = interval / 1000;
>> + ts.tv_nsec = (interval % 1000) * 1000000;
>> +#endif
>> + while (1) {
>> +#if LJ_TARGET_PS3
>> + sys_timer_usleep(interval * 1000);
>> +#else
>> + nanosleep(&ts, NULL);
>> +#endif
>> + if (timer->abort)
>> + break;
>
>
>Minor: I suggest to save old code style here, i.e. without newline.
Fixed.
>
>> + timer->opt.handler();
>> + }
>> + return NULL;
>> +}
>> +
>> +/* Start profiling timer thread. */
>> +void lj_profile_timer_start(lj_profile_timer *timer) {
>> + pthread_mutex_init(&timer->lock, 0);
>> + timer->abort = 0;
>> + pthread_create(&timer->thread, NULL, (void *(*)(void *))profile_thread,
>> + timer);
>
>Minor: Use tabs for allignment here.
Fixed.
>
>Side note: may be `t` stands for timer is good enough here.
>Feel free to ignore.
For me, timer is more clear :)
>
>> +}
>> +
>> +/* Stop profiling timer thread. */
>> +void lj_profile_timer_stop(lj_profile_timer *timer) {
>
><snipped>
>
>> +}
>> +
>> +#elif LJ_PROFILE_WTHREAD
>> +
>> +/* Windows timer thread. */
>> +static DWORD WINAPI profile_thread(void *timerx) {
>
>Also, `timer_thread()` name is looks better here, as far as it only
>handler timer in another thread.
>Thoughts?
>
>> + lj_profile_timer *timer = (lj_profile_timer *)timerx;
>> + int interval = timer->opt.interval_msec;
>> +#if LJ_TARGET_WINDOWS
>> + timer->wmm_tbp(interval);
>> +#endif
>> + while (1) {
>> + Sleep(interval);
>> + if (timer->abort)
>> + break;
>
>Minor: I suggest to save old code style here, i.e. without newline.
Fixed.
>
>> + timer->opt.handler();
>> + }
>> +#if LJ_TARGET_WINDOWS
>> + timer->wmm_tep(interval);
>> +#endif
>> + return 0;
>> +}
>> +
>> +/* Start profiling timer thread. */
>> +void lj_profile_timer_start(lj_profile_timer *timer) {
>> +#if LJ_TARGET_WINDOWS
>> + if (!timer->wmm) { /* Load WinMM library on-demand. */
>> + timer->wmm = LoadLibraryExA("winmm.dll", NULL, 0);
>> + if (timer->wmm) {
>> + timer->wmm_tbp =
>> + (WMM_TPFUNC)GetProcAddress(timer->wmm, "timeBeginPeriod");
>> + timer->wmm_tep = (WMM_TPFUNC)GetProcAddress(timer->wmm, "timeEndPeriod");
>> + if (!timer->wmm_tbp || !timer->wmm_tep) {
>> + timer->wmm = NULL;
>> + return;
>
>LuaJIT use quoter-half tabs codestyle. Please, return 1 tab instead 8
>spaces.
Fixed.
>
>> + }
>> + }
>> + }
>> +#endif
>> + InitializeCriticalSection(&timer->lock);
>> + timer->abort = 0;
>> + timer->thread = CreateThread(NULL, 0, profile_thread, timer, 0, NULL);
>> +}
>> +
>> +/* Stop profiling timer thread. */
>> +void lj_profile_timer_stop(lj_profile_timer *timer) {
>
><snipped>
>
>> +}
>> +
>> +#endif
>> +
>> +#endif /* LJ_HASPROFILE */
>> diff --git a/src/lj_profile_timer.h b/src/lj_profile_timer.h
>> new file mode 100644
>> index 00000000..af9eabce
>> --- /dev/null
>> +++ b/src/lj_profile_timer.h
>> @@ -0,0 +1,83 @@
>> +/*
>> +** Simple profiling timer extracted from inbuilt luajit profiler
>
>Nit: I suppose we can omit the part about extracting.
Fixed.
>
>> +*/
>> +
>> +#ifndef _LJ_PROFILE_TIMER_H
>> +#define _LJ_PROFILE_TIMER_H
>> +
>> +#include <stdint.h>
>
>It's better to use "lj_def.h" here (see it to explanation).
Fixed.
>
>> +
>> +#include "lj_arch.h"
>> +
>> +#if LJ_HASPROFILE
>> +
>> +#if LJ_PROFILE_SIGPROF
>
><snipped>
>
>> +#endif
>> +
>> +typedef struct {
>> +#if LJ_PROFILE_SIGPROF
>> + void (*handler)(int, siginfo_t*, void*);
>> +#else
>> + void (*handler)(void);
>> +#endif
>
>Don't get why do we need different handlers signature here.
>If we use the old one i.e. void (*)(void).
>
>> + uint32_t interval_msec;
>> +} lj_profile_timer_opt;
>
>Why do we need a separate structure for these options?
I think, we need it to highlight the configurable parameters.
>
>> +
>
>Minor: Please fix mess with tabs and spaces below.
Fixed.
>
>> +typedef struct {
>> + lj_profile_timer_opt opt;
>> +#if LJ_PROFILE_SIGPROF
>> + struct sigaction oldsa; /* Previous SIGPROF state. */
>> +#elif LJ_PROFILE_PTHREAD
>> + pthread_mutex_t lock; /* g->hookmask update lock. */
>> + pthread_t thread; /* Timer thread. */
>> + int abort; /* Abort timer thread. */
>> +#elif LJ_PROFILE_WTHREAD
>> +#if LJ_TARGET_WINDOWS
>> + HINSTANCE wmm; /* WinMM library handle. */
>> + WMM_TPFUNC wmm_tbp; /* WinMM timeBeginPeriod function. */
>> + WMM_TPFUNC wmm_tep; /* WinMM timeEndPeriod function. */
>> +#endif
>> + CRITICAL_SECTION lock; /* g->hookmask update lock. */
>> + HANDLE thread; /* Timer thread. */
>> + int abort; /* Abort timer thread. */
>> +#endif
>> +} lj_profile_timer;
>> +
>> +/* Start profiling timer */
>
>Typo: missed dot at the end of the sentence.
Fixed.
>
>> +void lj_profile_timer_start(lj_profile_timer *timer);
>> +
>> +/* Stop profiling timer */
>
>Typo: missed dot at the end of the sentence.
Fixed.
>
>> +void lj_profile_timer_stop(lj_profile_timer *timer);
>> +
>> +#endif /* LJ_HASPROFILE */
>> +
>> +#endif
>> --
>> 2.33.0
>>
>
>--
>Best regards,
>Sergey Kaplun
Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-781-platform-and-lua-profiler
New hash: https://github.com/tarantool/luajit/commit/ecbf4af8ec1e8a597199e6dcdeca2a95b95cf667
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 50c23cae..c92d78cc 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -68,8 +68,8 @@ make_source_list(SOURCES_UTILS
make_source_list(SOURCES_PROFILER
SOURCES
lj_memprof.c
- lj_profile_timer.c
lj_profile.c
+ lj_profile_timer.c
)
# Lua standard library + extensions by LuaJIT.
diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original
index da5abf0d..68f51706 100644
--- a/src/Makefile.dep.original
+++ b/src/Makefile.dep.original
@@ -179,8 +179,8 @@ lj_profile.o: lj_profile.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
lj_buf.h lj_gc.h lj_str.h lj_frame.h lj_bc.h lj_debug.h lj_dispatch.h \
lj_jit.h lj_ir.h lj_trace.h lj_traceerr.h lj_profile.h \
lj_profile_timer.h luajit.h
-lj_profile_timer.o: lj_profile_timer.c lj_profile_timer.h lj_arch.h lua.h \
- luaconf.h
+lj_profile_timer.o: lj_profile_timer.c lj_profile_timer.h lj_def.h lua.h \
+ luaconf.h lj_arch.h
lj_record.o: lj_record.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
lj_err.h lj_errmsg.h lj_str.h lj_tab.h lj_meta.h lj_frame.h lj_bc.h \
lj_ctype.h lj_gc.h lj_ff.h lj_ffdef.h lj_debug.h lj_ir.h lj_jit.h \
@@ -223,21 +223,23 @@ ljamalg.o: ljamalg.c lua.h luaconf.h lauxlib.h lj_gc.c lj_obj.h lj_def.h \
lj_func.h lj_udata.h lj_meta.h lj_state.h lj_frame.h lj_bc.h lj_ctype.h \
lj_cdata.h lj_trace.h lj_jit.h lj_ir.h lj_dispatch.h lj_traceerr.h \
lj_vm.h lj_err.c lj_debug.h lj_ff.h lj_ffdef.h lj_strfmt.h lj_char.c \
- lj_char.h lj_bc.c lj_bcdef.h lj_obj.c lj_buf.c lj_wbuf.c lj_wbuf.h lj_utils.h \
- lj_str.c lj_tab.c lj_func.c lj_udata.c lj_meta.c lj_strscan.h lj_lib.h \
- lj_debug.c lj_state.c lj_lex.h lj_alloc.h luajit.h lj_dispatch.c \
- lj_ccallback.h lj_profile.h lj_memprof.h lj_vmevent.c lj_vmevent.h \
+ lj_char.h lj_bc.c lj_bcdef.h lj_obj.c lj_buf.c lj_wbuf.c lj_wbuf.h \
+ lj_utils.h lj_str.c lj_tab.c lj_func.c lj_udata.c lj_meta.c lj_strscan.h \
+ lj_lib.h lj_debug.c lj_state.c lj_lex.h lj_alloc.h luajit.h lj_memprof.h \
+ lj_dispatch.c lj_ccallback.h lj_profile.h lj_vmevent.c lj_vmevent.h \
lj_vmmath.c lj_strscan.c lj_strfmt.c lj_strfmt_num.c lj_api.c lj_mapi.c \
- lmisclib.h lj_profile.c lj_memprof.c lj_lex.c lualib.h lj_parse.h lj_parse.c \
- lj_bcread.c lj_bcdump.h lj_bcwrite.c lj_load.c lj_ctype.c lj_cdata.c \
- lj_cconv.h lj_cconv.c lj_ccall.c lj_ccall.h lj_ccallback.c lj_target.h \
- lj_target_*.h lj_mcode.h lj_carith.c lj_carith.h lj_clib.c lj_clib.h \
- lj_cparse.c lj_cparse.h lj_lib.c lj_ir.c lj_ircall.h lj_iropt.h lj_opt_mem.c \
- lj_opt_fold.c lj_folddef.h lj_opt_narrow.c lj_opt_dce.c lj_opt_loop.c \
- lj_snap.h lj_opt_split.c lj_opt_sink.c lj_mcode.c lj_snap.c lj_record.c \
- lj_record.h lj_ffrecord.h lj_crecord.c lj_crecord.h lj_ffrecord.c lj_recdef.h \
- lj_asm.c lj_asm.h lj_emit_*.h lj_asm_*.h lj_trace.c lj_gdbjit.h lj_gdbjit.c \
- lj_alloc.c lj_utils_leb128.c lib_aux.c lib_base.c lj_libdef.h lib_math.c \
+ lmisclib.h lj_profile.c lj_profile_timer.h lj_profile_timer.c \
+ lj_memprof.c lj_lex.c lualib.h lj_parse.h lj_parse.c lj_bcread.c \
+ lj_bcdump.h lj_bcwrite.c lj_load.c lj_ctype.c lj_cdata.c lj_cconv.h \
+ lj_cconv.c lj_ccall.c lj_ccall.h lj_ccallback.c lj_target.h \
+ lj_target_x86.h lj_mcode.h lj_carith.c lj_carith.h lj_clib.c lj_clib.h \
+ lj_cparse.c lj_cparse.h lj_lib.c lj_ir.c lj_ircall.h lj_iropt.h \
+ lj_opt_mem.c lj_opt_fold.c lj_folddef.h lj_opt_narrow.c lj_opt_dce.c \
+ lj_opt_loop.c lj_snap.h lj_opt_split.c lj_opt_sink.c lj_mcode.c \
+ lj_snap.c lj_record.c lj_record.h lj_ffrecord.h lj_crecord.c \
+ lj_crecord.h lj_ffrecord.c lj_recdef.h lj_asm.c lj_asm.h lj_emit_x86.h \
+ lj_asm_x86.h lj_trace.c lj_gdbjit.h lj_gdbjit.c lj_alloc.c \
+ lj_utils_leb128.c lib_aux.c lib_base.c lj_libdef.h lib_math.c \
lib_string.c lib_table.c lib_io.c lib_os.c lib_package.c lib_debug.c \
lib_bit.c lib_jit.c lib_ffi.c lib_misc.c lib_init.c
luajit.o: luajit.c lua.h luaconf.h lauxlib.h lualib.h luajit.h lj_arch.h
diff --git a/src/lj_profile.c b/src/lj_profile.c
index e7e1a303..4412d68b 100644
--- a/src/lj_profile.c
+++ b/src/lj_profile.c
@@ -31,7 +31,7 @@ typedef struct ProfileState {
SBuf sb; /* String buffer for stack dumps. */
int samples; /* Number of samples for next callback. */
int vmstate; /* VM state when profile timer triggered. */
- lj_profile_timer timer; /* Profiling timer */
+ lj_profile_timer timer; /* Profiling timer */
} ProfileState;
/* Sadly, we have to use a static profiler state.
@@ -98,7 +98,6 @@ void LJ_FASTCALL lj_profile_interpreter(lua_State *L)
profile_unlock(ps);
}
-
/* Trigger profile hook. Asynchronous call from OS-specific profile timer. */
static void profile_trigger(ProfileState *ps)
{
@@ -124,7 +123,7 @@ static void profile_trigger(ProfileState *ps)
#if LJ_PROFILE_SIGPROF
-static void profile_handler(int sig, siginfo_t *info, void* ctx)
+static void profile_handler(int sig, siginfo_t *info, void *ctx)
{
UNUSED(sig);
UNUSED(info);
@@ -177,7 +176,6 @@ LUA_API void luaJIT_profile_start(lua_State *L, const char *mode,
ps->data = data;
ps->samples = 0;
lj_buf_init(L, &ps->sb);
-
ps->timer.opt.interval_msec = interval;
ps->timer.opt.handler = profile_handler;
lj_profile_timer_start(&ps->timer);
diff --git a/src/lj_profile_timer.c b/src/lj_profile_timer.c
index ce420a85..056fd1f7 100644
--- a/src/lj_profile_timer.c
+++ b/src/lj_profile_timer.c
@@ -1,5 +1,6 @@
/*
-** Simple profiling timer extracted from inbuilt luajit profiler
+** Simple profiling timer.
+** Copyright (C) 2005-2017 Mike Pall. See Copyright Notice in luajit.h
*/
#define lj_profile_timer_c
@@ -12,8 +13,9 @@
#if LJ_PROFILE_SIGPROF
/* Start profiling timer. */
-void lj_profile_timer_start(lj_profile_timer *timer) {
- int interval = timer->opt.interval_msec;
+void lj_profile_timer_start(lj_profile_timer *timer)
+{
+ const int interval = timer->opt.interval_msec;
struct itimerval tm;
struct sigaction sa;
tm.it_value.tv_sec = tm.it_interval.tv_sec = interval / 1000;
@@ -26,7 +28,8 @@ void lj_profile_timer_start(lj_profile_timer *timer) {
}
/* Stop profiling timer. */
-void lj_profile_timer_stop(lj_profile_timer *timer) {
+void lj_profile_timer_stop(lj_profile_timer *timer)
+{
struct itimerval tm;
tm.it_value.tv_sec = tm.it_interval.tv_sec = 0;
tm.it_value.tv_usec = tm.it_interval.tv_usec = 0;
@@ -37,7 +40,8 @@ void lj_profile_timer_stop(lj_profile_timer *timer) {
#elif LJ_PROFILE_PTHREAD
/* POSIX timer thread. */
-static void *profile_thread(lj_timer *timer) {
+static void *timer_thread(lj_profile_timer *timer)
+{
int interval = timer->opt.interval_msec;
#if !LJ_TARGET_PS3
struct timespec ts;
@@ -50,23 +54,24 @@ static void *profile_thread(lj_timer *timer) {
#else
nanosleep(&ts, NULL);
#endif
- if (timer->abort)
- break;
+ if (timer->abort) break;
timer->opt.handler();
}
return NULL;
}
/* Start profiling timer thread. */
-void lj_profile_timer_start(lj_profile_timer *timer) {
+void lj_profile_timer_start(lj_profile_timer *timer)
+{
pthread_mutex_init(&timer->lock, 0);
timer->abort = 0;
- pthread_create(&timer->thread, NULL, (void *(*)(void *))profile_thread,
- timer);
+ pthread_create(&timer->thread, NULL, (void *(*)(void *))timer_thread,
+ timer);
}
/* Stop profiling timer thread. */
-void lj_profile_timer_stop(lj_profile_timer *timer) {
+void lj_profile_timer_stop(lj_profile_timer *timer)
+{
timer->abort = 1;
pthread_join(timer->thread, NULL);
pthread_mutex_destroy(&timer->lock);
@@ -75,7 +80,8 @@ void lj_profile_timer_stop(lj_profile_timer *timer) {
#elif LJ_PROFILE_WTHREAD
/* Windows timer thread. */
-static DWORD WINAPI profile_thread(void *timerx) {
+static DWORD WINAPI timer_thread(void *timerx)
+{
lj_profile_timer *timer = (lj_profile_timer *)timerx;
int interval = timer->opt.interval_msec;
#if LJ_TARGET_WINDOWS
@@ -83,8 +89,7 @@ static DWORD WINAPI profile_thread(void *timerx) {
#endif
while (1) {
Sleep(interval);
- if (timer->abort)
- break;
+ if (timer->abort) break;
timer->opt.handler();
}
#if LJ_TARGET_WINDOWS
@@ -94,28 +99,30 @@ static DWORD WINAPI profile_thread(void *timerx) {
}
/* Start profiling timer thread. */
-void lj_profile_timer_start(lj_profile_timer *timer) {
+void lj_profile_timer_start(lj_profile_timer *timer)
+{
#if LJ_TARGET_WINDOWS
if (!timer->wmm) { /* Load WinMM library on-demand. */
timer->wmm = LoadLibraryExA("winmm.dll", NULL, 0);
if (timer->wmm) {
timer->wmm_tbp =
- (WMM_TPFUNC)GetProcAddress(timer->wmm, "timeBeginPeriod");
+ (WMM_TPFUNC)GetProcAddress(timer->wmm, "timeBeginPeriod");
timer->wmm_tep = (WMM_TPFUNC)GetProcAddress(timer->wmm, "timeEndPeriod");
if (!timer->wmm_tbp || !timer->wmm_tep) {
- timer->wmm = NULL;
- return;
+ timer->wmm = NULL;
+ return;
}
}
}
#endif
InitializeCriticalSection(&timer->lock);
timer->abort = 0;
- timer->thread = CreateThread(NULL, 0, profile_thread, timer, 0, NULL);
+ timer->thread = CreateThread(NULL, 0, timer_thread, timer, 0, NULL);
}
/* Stop profiling timer thread. */
-void lj_profile_timer_stop(lj_profile_timer *timer) {
+void lj_profile_timer_stop(lj_profile_timer *timer)
+{
timer->abort = 1;
WaitForSingleObject(timer->thread, INFINITE);
DeleteCriticalSection(&timer->lock);
diff --git a/src/lj_profile_timer.h b/src/lj_profile_timer.h
index af9eabce..1deeea53 100644
--- a/src/lj_profile_timer.h
+++ b/src/lj_profile_timer.h
@@ -1,12 +1,12 @@
/*
-** Simple profiling timer extracted from inbuilt luajit profiler
+** Simple profiling timer.
+** Copyright (C) 2005-2017 Mike Pall. See Copyright Notice in luajit.h
*/
#ifndef _LJ_PROFILE_TIMER_H
#define _LJ_PROFILE_TIMER_H
-#include <stdint.h>
-
+#include "lj_def.h"
#include "lj_arch.h"
#if LJ_HASPROFILE
@@ -55,27 +55,27 @@ typedef struct {
typedef struct {
lj_profile_timer_opt opt;
#if LJ_PROFILE_SIGPROF
- struct sigaction oldsa; /* Previous SIGPROF state. */
+ struct sigaction oldsa; /* Previous SIGPROF state. */
#elif LJ_PROFILE_PTHREAD
- pthread_mutex_t lock; /* g->hookmask update lock. */
- pthread_t thread; /* Timer thread. */
- int abort; /* Abort timer thread. */
+ pthread_mutex_t lock; /* g->hookmask update lock. */
+ pthread_t thread; /* Timer thread. */
+ int abort; /* Abort timer thread. */
#elif LJ_PROFILE_WTHREAD
#if LJ_TARGET_WINDOWS
- HINSTANCE wmm; /* WinMM library handle. */
- WMM_TPFUNC wmm_tbp; /* WinMM timeBeginPeriod function. */
- WMM_TPFUNC wmm_tep; /* WinMM timeEndPeriod function. */
+ HINSTANCE wmm; /* WinMM library handle. */
+ WMM_TPFUNC wmm_tbp; /* WinMM timeBeginPeriod function. */
+ WMM_TPFUNC wmm_tep; /* WinMM timeEndPeriod function. */
#endif
- CRITICAL_SECTION lock; /* g->hookmask update lock. */
- HANDLE thread; /* Timer thread. */
- int abort; /* Abort timer thread. */
+ CRITICAL_SECTION lock; /* g->hookmask update lock. */
+ HANDLE thread; /* Timer thread. */
+ int abort; /* Abort timer thread. */
#endif
} lj_profile_timer;
-/* Start profiling timer */
+/* Start profiling timer. */
void lj_profile_timer_start(lj_profile_timer *timer);
-/* Stop profiling timer */
+/* Stop profiling timer. */
void lj_profile_timer_stop(lj_profile_timer *timer);
#endif /* LJ_HASPROFILE */
diff --git a/src/ljamalg.c b/src/ljamalg.c
index 3f7e6860..ce7a0d6c 100644
--- a/src/ljamalg.c
+++ b/src/ljamalg.c
@@ -51,6 +51,7 @@
#include "lj_api.c"
#include "lj_mapi.c"
#include "lj_profile.c"
+#include "lj_profile_timer.c"
#include "lj_memprof.c"
#include "lj_lex.c"
#include "lj_parse.c"
Best regards,
Mikhail Shishatskiy
next prev parent reply other threads:[~2021-09-23 11:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-08 17:50 [Tarantool-patches] [PATCH luajit 0/7] misc: introduce sysprof Maxim Kokryashkin via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 1/7] core: save current frame top to lj_obj Maxim Kokryashkin via Tarantool-patches
2021-09-20 17:21 ` Sergey Kaplun via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 2/7] core: separate the profiling timer from lj_profile Maxim Kokryashkin via Tarantool-patches
2021-09-21 11:13 ` Sergey Kaplun via Tarantool-patches
2021-09-23 11:37 ` Mikhail Shishatskiy via Tarantool-patches [this message]
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 3/7] memprof: move symtab to a separate module Maxim Kokryashkin via Tarantool-patches
2021-09-22 7:51 ` Sergey Kaplun via Tarantool-patches
2021-09-22 8:14 ` Sergey Kaplun via Tarantool-patches
2021-09-23 14:51 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 4/7] core: introduce lua and platform profiler Maxim Kokryashkin via Tarantool-patches
2021-09-29 6:53 ` Sergey Kaplun via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 5/7] memprof: add profile common section Maxim Kokryashkin via Tarantool-patches
2021-10-05 10:48 ` Sergey Kaplun via Tarantool-patches
2021-10-06 19:15 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 6/7] sysprof: introduce Lua API Maxim Kokryashkin via Tarantool-patches
2021-10-05 15:36 ` Sergey Kaplun via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 7/7] tools: introduce parsers for sysprof Maxim Kokryashkin via Tarantool-patches
2021-10-07 11:28 ` Sergey Kaplun via Tarantool-patches
2022-04-07 12:15 ` [Tarantool-patches] [PATCH luajit 0/7] misc: introduce sysprof Sergey Kaplun 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=20210923113738.ov5lhqlkxx6cmnqg@surf.localdomain \
--to=tarantool-patches@dev.tarantool.org \
--cc=m.shishatskiy@tarantool.org \
--cc=max.kokryashkin@gmail.com \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit 2/7] core: separate the profiling timer from lj_profile' \
/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