From 6b347d8a1bc3248219204bd2f420d82bc3872004 Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Sat, 13 Jun 2020 03:50:03 +0100 Subject: [PATCH 1/3] cron: use Lua table rather than native array Pin a table to the registry and use that instead of manually allocating and reallocating and shuffling integers around. Thanks to @TerryE for some style suggestions. --- app/modules/cron.c | 111 +++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 49 deletions(-) diff --git a/app/modules/cron.c b/app/modules/cron.c index aa85e8b039..6d315c85ac 100644 --- a/app/modules/cron.c +++ b/app/modules/cron.c @@ -27,8 +27,7 @@ typedef struct cronent_ud { static ETSTimer cron_timer; -static int *cronent_list = 0; -static size_t cronent_count = 0; +static int cronent_table_ref; static uint64_t lcron_parsepart(lua_State *L, char *str, char **end, uint8_t min, uint8_t max) { uint64_t res = 0; @@ -86,12 +85,20 @@ static int lcron_parsedesc(lua_State *L, char *str, struct cronent_desc *desc) { static int lcron_create(lua_State *L) { // Check arguments char *strdesc = (char*)luaL_checkstring(L, 1); - void *newlist; luaL_checktype(L, 2, LUA_TFUNCTION); - // Parse description - struct cronent_desc desc; - lcron_parsedesc(L, strdesc, &desc); - // Allocate userdata + + // Grab the table of all entries onto the stack + lua_rawgeti(L, LUA_REGISTRYINDEX, cronent_table_ref); + + // Find a free index + int ix = 1; + while(lua_rawgeti(L, -1, ix) != LUA_TNIL) { + lua_pop(L, 1); + ix++; + } + lua_pop(L, 1); // pop the nil off the stack + + // Allocate userdata onto the stack cronent_ud_t *ud = lua_newuserdata(L, sizeof(cronent_ud_t)); // Set metatable luaL_getmetatable(L, "cron.entry"); @@ -100,29 +107,30 @@ static int lcron_create(lua_State *L) { lua_pushvalue(L, 2); ud->cb_ref = luaL_ref(L, LUA_REGISTRYINDEX); // Set entry - ud->desc = desc; - // Store entry - newlist = os_realloc(cronent_list, sizeof(int) * (cronent_count + 1)); - if (newlist == NULL) { - return luaL_error(L, "out of memory"); - } - lua_pushvalue(L, -1); - cronent_list = newlist; - cronent_list[cronent_count++] = luaL_ref(L, LUA_REGISTRYINDEX); - return 1; + lcron_parsedesc(L, strdesc, &ud->desc); + + // Store entry to table while preserving userdata + lua_pushvalue(L, -1); // clone userdata + lua_rawseti(L, -3, ix); // -userdata; userdata, cronent table, cb, desc + + return 1; // just the userdata } +// Finds index, leaves table at top of stack for convenience static size_t lcron_findindex(lua_State *L, cronent_ud_t *ud) { - cronent_ud_t *eud; - size_t i; - for (i = 0; i < cronent_count; i++) { - lua_rawgeti(L, LUA_REGISTRYINDEX, cronent_list[i]); - eud = lua_touserdata(L, -1); + lua_rawgeti(L, LUA_REGISTRYINDEX, cronent_table_ref); + size_t count = lua_objlen(L, -1); + + for (size_t i = 1; i <= count; i++) { + lua_rawgeti(L, -1, i); + cronent_ud_t *eud = lua_touserdata(L, -1); lua_pop(L, 1); - if (eud == ud) break; + if (eud == ud) { + return i; + } } - if (i == cronent_count) return -1; - return i; + + return count + 1; } static int lcron_schedule(lua_State *L) { @@ -131,17 +139,12 @@ static int lcron_schedule(lua_State *L) { struct cronent_desc desc; lcron_parsedesc(L, strdesc, &desc); ud->desc = desc; + size_t i = lcron_findindex(L, ud); - if (i == -1) { - void *newlist; - newlist = os_realloc(cronent_list, sizeof(int) * (cronent_count + 1)); - if (newlist == NULL) { - return luaL_error(L, "out of memory"); - } - cronent_list = newlist; - lua_pushvalue(L, 1); - cronent_list[cronent_count++] = luaL_ref(L, LUA_REGISTRYINDEX); - } + + lua_pushvalue(L, 1); // copy ud to top of stack + lua_rawseti(L, -2, i); // install into table + return 0; } @@ -157,27 +160,25 @@ static int lcron_handler(lua_State *L) { static int lcron_unschedule(lua_State *L) { cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); size_t i = lcron_findindex(L, ud); - if (i == -1) return 0; - luaL_unref(L, LUA_REGISTRYINDEX, cronent_list[i]); - memmove(cronent_list + i, cronent_list + i + 1, sizeof(int) * (cronent_count - i - 1)); - cronent_count--; + + lua_pushnil(L); + lua_rawseti(L, -2, i); + return 0; } +// scheduled entries are pinned, so we cannot arrive at the __gc metamethod static int lcron_delete(lua_State *L) { cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); - lcron_unschedule(L); luaL_unref(L, LUA_REGISTRYINDEX, ud->cb_ref); return 0; } static int lcron_reset(lua_State *L) { - for (size_t i = 0; i < cronent_count; i++) { - luaL_unref(L, LUA_REGISTRYINDEX, cronent_list[i]); - } - cronent_count = 0; - free(cronent_list); - cronent_list = 0; + lua_newtable(L); + luaL_unref(L, LUA_REGISTRYINDEX, cronent_table_ref); + cronent_table_ref = luaL_ref(L, LUA_REGISTRYINDEX); + return 0; } @@ -189,19 +190,27 @@ static void cron_handle_time(uint8_t mon, uint8_t dom, uint8_t dow, uint8_t hour desc.dow = ( uint8_t)1 << dow; desc.hour = (uint32_t)1 << hour; desc.min = (uint64_t)1 << min; - for (size_t i = 0; i < cronent_count; i++) { - lua_rawgeti(L, LUA_REGISTRYINDEX, cronent_list[i]); + + lua_rawgeti(L, LUA_REGISTRYINDEX, cronent_table_ref); + size_t count = lua_objlen(L, -1); + + for (size_t i = 1; i <= count; i++) { + lua_rawgeti(L, -1, i); cronent_ud_t *ent = lua_touserdata(L, -1); lua_pop(L, 1); + if ((ent->desc.mon & desc.mon ) == 0) continue; if ((ent->desc.dom & desc.dom ) == 0) continue; if ((ent->desc.dow & desc.dow ) == 0) continue; if ((ent->desc.hour & desc.hour) == 0) continue; if ((ent->desc.min & desc.min ) == 0) continue; + lua_rawgeti(L, LUA_REGISTRYINDEX, ent->cb_ref); - lua_rawgeti(L, LUA_REGISTRYINDEX, cronent_list[i]); + lua_rawgeti(L, -2, i); // get ud again luaL_pcallx(L, 1, 0); } + + lua_pop(L, 1); // pop table } static void cron_handle_tmr() { @@ -250,6 +259,10 @@ int luaopen_cron( lua_State *L ) { //My guess: To be sure to give the other modules required by cron enough time to get to a ready state, restart cron_timer. os_timer_arm(&cron_timer, 1000, 0); luaL_rometatable(L, "cron.entry", LROT_TABLEREF(cronent)); + + cronent_table_ref = LUA_NOREF; + lcron_reset(L); + return 0; } From 4bcb4bdfb8b182872133d32f4821ca4df9ed476f Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Sat, 13 Jun 2020 04:00:15 +0100 Subject: [PATCH 2/3] cron: permit entry.schedule() to reuse existing --- app/modules/cron.c | 11 +++++++---- docs/modules/cron.md | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/modules/cron.c b/app/modules/cron.c index 6d315c85ac..4071f47c4e 100644 --- a/app/modules/cron.c +++ b/app/modules/cron.c @@ -135,10 +135,13 @@ static size_t lcron_findindex(lua_State *L, cronent_ud_t *ud) { static int lcron_schedule(lua_State *L) { cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); - char *strdesc = (char*)luaL_checkstring(L, 2); - struct cronent_desc desc; - lcron_parsedesc(L, strdesc, &desc); - ud->desc = desc; + char *strdesc = (char*)luaL_optstring(L, 2, NULL); + + if (strdesc != NULL) { + struct cronent_desc desc; + lcron_parsedesc(L, strdesc, &desc); + ud->desc = desc; + } size_t i = lcron_findindex(L, ud); diff --git a/docs/modules/cron.md b/docs/modules/cron.md index 09c2fe91e8..9568cdee04 100644 --- a/docs/modules/cron.md +++ b/docs/modules/cron.md @@ -83,10 +83,10 @@ end) ## cron.entry:schedule() -Sets a new schedule mask. +Sets a new schedule mask and/or restores an unscheduled entry. #### Syntax -`schedule(mask)` +`schedule([mask])` #### Parameters - `mask` - [crontab](https://en.wikipedia.org/wiki/Cron#Overview)-like string mask for schedule From 474e6a7c343e9f0602d80840dc7b34ad529f22ab Mon Sep 17 00:00:00 2001 From: Nathaniel Wesley Filardo Date: Tue, 28 Dec 2021 02:17:09 +0000 Subject: [PATCH 3/3] cron: tidying and more chatty debugging --- app/modules/cron.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/app/modules/cron.c b/app/modules/cron.c index 4071f47c4e..ea1fd8f9ce 100644 --- a/app/modules/cron.c +++ b/app/modules/cron.c @@ -12,6 +12,14 @@ #include "rtc/rtctime.h" #include "mem.h" +static const char *CRON_ENTRY_METATABLE = "cron.entry"; + +#if 0 +# define cron_dbg(...) printf(__VA_ARGS__) +#else +# define cron_dbg(...) +#endif + struct cronent_desc { uint64_t min; // Minutes repeat - bits 0-59 uint32_t hour; // Hours repeat - bits 0-23 @@ -51,15 +59,14 @@ static uint64_t lcron_parsepart(lua_State *L, char *str, char **end, uint8_t min } } else { uint32_t val; - while (1) { + do { val = strtol(str, end, 10); if (val < min || val > max) { return luaL_error(L, "invalid spec (val %d out of range %d..%d)", val, min, max); } res |= (uint64_t)1 << (val - min); - if (**end != ',') break; str = *end + 1; - } + } while (**end == ','); } return res; } @@ -101,7 +108,7 @@ static int lcron_create(lua_State *L) { // Allocate userdata onto the stack cronent_ud_t *ud = lua_newuserdata(L, sizeof(cronent_ud_t)); // Set metatable - luaL_getmetatable(L, "cron.entry"); + luaL_getmetatable(L, CRON_ENTRY_METATABLE); lua_setmetatable(L, -2); // Set callback lua_pushvalue(L, 2); @@ -134,7 +141,7 @@ static size_t lcron_findindex(lua_State *L, cronent_ud_t *ud) { } static int lcron_schedule(lua_State *L) { - cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); + cronent_ud_t *ud = luaL_checkudata(L, 1, CRON_ENTRY_METATABLE); char *strdesc = (char*)luaL_optstring(L, 2, NULL); if (strdesc != NULL) { @@ -145,6 +152,8 @@ static int lcron_schedule(lua_State *L) { size_t i = lcron_findindex(L, ud); + cron_dbg("cron: schedule %p at index %d\n", ud, i); + lua_pushvalue(L, 1); // copy ud to top of stack lua_rawseti(L, -2, i); // install into table @@ -152,7 +161,7 @@ static int lcron_schedule(lua_State *L) { } static int lcron_handler(lua_State *L) { - cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); + cronent_ud_t *ud = luaL_checkudata(L, 1, CRON_ENTRY_METATABLE); luaL_checktype(L, 2, LUA_TFUNCTION); lua_pushvalue(L, 2); luaL_unref(L, LUA_REGISTRYINDEX, ud->cb_ref); @@ -161,9 +170,11 @@ static int lcron_handler(lua_State *L) { } static int lcron_unschedule(lua_State *L) { - cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); + cronent_ud_t *ud = luaL_checkudata(L, 1, CRON_ENTRY_METATABLE); size_t i = lcron_findindex(L, ud); + cron_dbg("cron: unschedule %p at index %d\n", ud, i); + lua_pushnil(L); lua_rawseti(L, -2, i); @@ -172,7 +183,7 @@ static int lcron_unschedule(lua_State *L) { // scheduled entries are pinned, so we cannot arrive at the __gc metamethod static int lcron_delete(lua_State *L) { - cronent_ud_t *ud = luaL_checkudata(L, 1, "cron.entry"); + cronent_ud_t *ud = luaL_checkudata(L, 1, CRON_ENTRY_METATABLE); luaL_unref(L, LUA_REGISTRYINDEX, ud->cb_ref); return 0; } @@ -182,6 +193,8 @@ static int lcron_reset(lua_State *L) { luaL_unref(L, LUA_REGISTRYINDEX, cronent_table_ref); cronent_table_ref = luaL_ref(L, LUA_REGISTRYINDEX); + cron_dbg("cron: cronent_table_ref is %d\n", cronent_table_ref); + return 0; } @@ -198,10 +211,14 @@ static void cron_handle_time(uint8_t mon, uint8_t dom, uint8_t dow, uint8_t hour size_t count = lua_objlen(L, -1); for (size_t i = 1; i <= count; i++) { + cron_dbg("cron: handle_time index %d (of %d; top %d)\n", i, count, lua_gettop(L)); + lua_rawgeti(L, -1, i); cronent_ud_t *ent = lua_touserdata(L, -1); lua_pop(L, 1); + cron_dbg(" ... is %p\n", ent); + if ((ent->desc.mon & desc.mon ) == 0) continue; if ((ent->desc.dom & desc.dom ) == 0) continue; if ((ent->desc.dow & desc.dow ) == 0) continue; @@ -261,7 +278,7 @@ int luaopen_cron( lua_State *L ) { //cron_handle_tmr determines when to execute a scheduled cron job //My guess: To be sure to give the other modules required by cron enough time to get to a ready state, restart cron_timer. os_timer_arm(&cron_timer, 1000, 0); - luaL_rometatable(L, "cron.entry", LROT_TABLEREF(cronent)); + luaL_rometatable(L, CRON_ENTRY_METATABLE, LROT_TABLEREF(cronent)); cronent_table_ref = LUA_NOREF; lcron_reset(L);