From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id C1CF36EC40; Thu, 23 Sep 2021 14:37:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C1CF36EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1632397063; bh=3LltMrCsafVsApV7oNRgyIcOKRTdG+kbTvO0E1tLvs0=; h=Date:To:In-Reply-To:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=GtFmdErm3KHrXFbxKD03NpGoai0qzCeA6S+8LtT+TRFvaPWYHWGmaTrm6Kkxbxwya EaZwPmL4tZFZv1GrIgEpPcEcmLdjYEuM3xk6FIhQpBZ/vXdPxt+2a3aTx+OeumhoF3 v1JYF+e0x5Cs10Hfq9bDyqvV4OC4foGCoxpRejMQ= Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3395A6EC40 for ; Thu, 23 Sep 2021 14:37:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3395A6EC40 Received: by smtp49.i.mail.ru with esmtpa (envelope-from ) id 1mTN2q-0007zC-Qd; Thu, 23 Sep 2021 14:37:41 +0300 Date: Thu, 23 Sep 2021 14:37:38 +0300 To: Sergey Kaplun Message-ID: <20210923113738.ov5lhqlkxx6cmnqg@surf.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline In-Reply-To: X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD91AE02D33A9C88A2F5A5BD9D2085A59586CBBC9313A5A610100894C459B0CD1B9BB80F971C57A5262A4047F05CC9F657A7AE362E5A487ADA2BD3E183E79334B09 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE71EEA4C46C73542F4EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F4DB495E34953D8A8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D0302692F9ADEFA7D53128855C39E11E117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE41BF15D38FB6CB3A3071BDA2D7A0BB39D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE3632EDEA9CD5989A3AD7EC71F1DB88427C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637967A675A37A195C7EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505CCE2B402E27AEF8DAE281781B59BD766 X-C1DE0DAB: 0D63561A33F958A531429DC5939CA40A44268DB805F357A3FF44CF29E352590ED59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA759D2A03B9C34326B3410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34FA5A4D623E7343BBE0E91E45BE47DD35202923CB4AFAB29B3E7AB9337D5125BF668044B11E0266B21D7E09C32AA3244CA5F53D2808E2B5114CE8F030B6677FBAD9ADFF0C0BDB8D1F927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXh7P4vbO8NjVMsosOQrrB++ X-Mailru-Sender: EFA0F3A8419EF21635BFE795C6CB22C9E094D93B517C6CCC7D2267CFB0F8EC2FC9F871340C5A6C402376072A51849BFFE66B5C1DBFD5D09D5E022D45988A037B448E0EA96F20AB367402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/7] core: separate the profiling timer from lj_profile X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mikhail Shishatskiy via Tarantool-patches Reply-To: Mikhail Shishatskiy Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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 >> >> 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 > > > >> 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 > > > >> -#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) >> -{ > > > >> -} >> - >> -#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) >> -{ > > > >> -} >> >> -/* Stop profiling timer thread. */ >> -static void profile_timer_stop(ProfileState *ps) >> +static void profile_handler() >> { > > > >> -} >> - >> -#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) >> -{ > > > >> } >> >> #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) > > > >> 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) { > > > >> +} >> + >> +#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) { > > > >> +} >> + >> +#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) { > > > >> +} >> + >> +#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 > >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 > > > >> +#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 - +#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