Skip to content

Commit

Permalink
Removed internal cache for closures
Browse files Browse the repository at this point in the history
The mechanism of "caching the last closure created for a prototype to
try to reuse it the next time a closure for that prototype is created"
was removed. There are several reasons:

- It is hard to find a natural example where this cache has a measurable
impact on performance.

- Programmers already perceive closure creation as something slow,
so they tend to avoid it inside hot paths. (Any case where the cache
could reuse a closure can be rewritten predefining the closure in some
variable and using that variable.)

- The implementation was somewhat complex, due to a bad interaction
with the generational collector. (Typically, new closures are new,
while prototypes are old. So, the cache breaks the invariant that
old objects should not point to new ones.)
  • Loading branch information
roberto-ieru committed Nov 1, 2018
1 parent 2fc6b55 commit e8c7797
Show file tree
Hide file tree
Showing 9 changed files with 10 additions and 123 deletions.
2 changes: 0 additions & 2 deletions lfunc.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,6 @@ Proto *luaF_newproto (lua_State *L) {
f->p = NULL;
f->sizep = 0;
f->code = NULL;
f->cache = NULL;
f->cachemiss = 0;
f->sizecode = 0;
f->lineinfo = NULL;
f->sizelineinfo = 0;
Expand Down
64 changes: 1 addition & 63 deletions lgc.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,27 +221,6 @@ void luaC_barrierback_ (lua_State *L, GCObject *o) {
}


/*
** Barrier for prototype's cache of closures. It turns the prototype
** back to gray (it was black). For an 'OLD1' prototype, making it
** gray stops it from being visited by 'markold', so it is linked in
** the 'grayagain' list to ensure it will be visited. For other ages,
** it goes to the 'protogray' list, as only its 'cache' field needs to
** be revisited. (A prototype to be in this barrier must be already
** finished, so its other fields cannot change and do not need to be
** revisited.)
*/
LUAI_FUNC void luaC_protobarrier_ (lua_State *L, Proto *p) {
global_State *g = G(L);
lua_assert(g->gckind != KGC_GEN || isold(p));
if (getage(p) == G_OLD1) /* still need to be visited? */
linkgclist(p, g->grayagain); /* link it in 'grayagain' */
else
linkgclist(p, g->protogray); /* link it in 'protogray' */
black2gray(p); /* make prototype gray */
}


void luaC_fix (lua_State *L, GCObject *o) {
global_State *g = G(L);
lua_assert(g->allgc == o); /* object must be 1st in 'allgc' list! */
Expand Down Expand Up @@ -379,7 +358,7 @@ static int remarkupvals (global_State *g) {
*/
static void restartcollection (global_State *g) {
g->gray = g->grayagain = NULL;
g->weak = g->allweak = g->ephemeron = g->protogray = NULL;
g->weak = g->allweak = g->ephemeron = NULL;
markobject(g, g->mainthread);
markvalue(g, &g->l_registry);
markmt(g);
Expand Down Expand Up @@ -534,40 +513,13 @@ static int traverseudata (global_State *g, Udata *u) {
}


/*
** Check the cache of a prototype, to keep invariants. If the
** cache is white, clear it. (A cache should not prevent the
** collection of its reference.) Otherwise, if in generational
** mode, check the generational invariant. If the cache is old,
** everything is ok. If the prototype is 'OLD0', everything
** is ok too. (It will naturally be visited again.) If the
** prototype is older than 'OLD0', then its cache (which is new)
** must be visited again in the next collection, so the prototype
** goes to the 'protogray' list. (If the prototype has a cache,
** it is already immutable and does not need other barriers;
** then, it can become gray without problems for its other fields.)
*/
static void checkprotocache (global_State *g, Proto *p) {
if (p->cache) {
if (iswhite(p->cache))
p->cache = NULL; /* allow cache to be collected */
else if (g->gckind == KGC_GEN && !isold(p->cache) && getage(p) >= G_OLD1) {
linkgclist(p, g->protogray); /* link it in 'protogray' */
black2gray(p); /* make prototype gray */
}
}
p->cachemiss = 0; /* restart counting */
}


/*
** Traverse a prototype. (While a prototype is being build, its
** arrays can be larger than needed; the extra slots are filled with
** NULL, so the use of 'markobjectN')
*/
static int traverseproto (global_State *g, Proto *f) {
int i;
checkprotocache(g, f);
markobjectN(g, f->source);
for (i = 0; i < f->sizek; i++) /* mark literals */
markvalue(g, &f->k[i]);
Expand Down Expand Up @@ -696,19 +648,6 @@ static void convergeephemerons (global_State *g) {
** =======================================================
*/

static void clearprotolist (global_State *g) {
GCObject *p = g->protogray;
g->protogray = NULL;
while (p != NULL) {
Proto *pp = gco2p(p);
GCObject *next = pp->gclist;
lua_assert(isgray(pp) && (pp->cache != NULL || pp->cachemiss >= MAXMISS));
gray2black(pp);
checkprotocache(g, pp);
p = next;
}
}


/*
** clear entries with unmarked keys from all weaktables in list 'l'
Expand Down Expand Up @@ -1439,7 +1378,6 @@ static lu_mem atomic (lua_State *L) {
clearbyvalues(g, g->weak, origweak);
clearbyvalues(g, g->allweak, origall);
luaS_clearcache(g);
clearprotolist(g);
g->currentwhite = cast_byte(otherwhite(g)); /* flip current white */
lua_assert(g->gray == NULL);
return work; /* estimate of slots marked by 'atomic' */
Expand Down
4 changes: 0 additions & 4 deletions lgc.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,6 @@
(isblack(p) && iswhite(o)) ? \
luaC_barrier_(L,obj2gco(p),obj2gco(o)) : cast_void(0))

#define luaC_protobarrier(L,p,o) \
(isblack(p) ? luaC_protobarrier_(L,p) : cast_void(0))

LUAI_FUNC void luaC_fix (lua_State *L, GCObject *o);
LUAI_FUNC void luaC_freeallobjects (lua_State *L);
LUAI_FUNC void luaC_step (lua_State *L);
Expand All @@ -175,7 +172,6 @@ LUAI_FUNC void luaC_fullgc (lua_State *L, int isemergency);
LUAI_FUNC GCObject *luaC_newobj (lua_State *L, int tt, size_t sz);
LUAI_FUNC void luaC_barrier_ (lua_State *L, GCObject *o, GCObject *v);
LUAI_FUNC void luaC_barrierback_ (lua_State *L, GCObject *o);
LUAI_FUNC void luaC_protobarrier_ (lua_State *L, Proto *p);
LUAI_FUNC void luaC_checkfinalizer (lua_State *L, GCObject *o, Table *mt);
LUAI_FUNC void luaC_changemode (lua_State *L, int newmode);

Expand Down
2 changes: 0 additions & 2 deletions lobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,6 @@ typedef struct Proto {
lu_byte numparams; /* number of fixed (named) parameters */
lu_byte is_vararg;
lu_byte maxstacksize; /* number of registers needed by this function */
lu_byte cachemiss; /* count for successive misses for 'cache' field */
int sizeupvalues; /* size of 'upvalues' */
int sizek; /* size of 'k' */
int sizecode;
Expand All @@ -516,7 +515,6 @@ typedef struct Proto {
int linedefined; /* debug information */
int lastlinedefined; /* debug information */
TValue *k; /* constants used by the function */
struct LClosure *cache; /* last-created closure with this prototype */
Instruction *code; /* opcodes */
struct Proto **p; /* functions defined inside the function */
Upvaldesc *upvalues; /* upvalue information */
Expand Down
2 changes: 1 addition & 1 deletion lstate.c
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ LUA_API lua_State *lua_newstate (lua_Alloc f, void *ud) {
g->finobjsur = g->finobjold = g->finobjrold = NULL;
g->sweepgc = NULL;
g->gray = g->grayagain = NULL;
g->weak = g->ephemeron = g->allweak = g->protogray = NULL;
g->weak = g->ephemeron = g->allweak = NULL;
g->twups = NULL;
g->totalbytes = sizeof(LG);
g->GCdebt = 0;
Expand Down
3 changes: 0 additions & 3 deletions lstate.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@
** 'weak': tables with weak values to be cleared;
** 'ephemeron': ephemeron tables with white->white entries;
** 'allweak': tables with weak keys and/or weak values to be cleared.
** There is also a list 'protogray' for prototypes that need to have
** their caches cleared.
*/

Expand Down Expand Up @@ -186,7 +184,6 @@ typedef struct global_State {
GCObject *weak; /* list of tables with weak values */
GCObject *ephemeron; /* list of ephemeron tables (weak keys) */
GCObject *allweak; /* list of all-weak tables */
GCObject *protogray; /* list of prototypes with "new" caches */
GCObject *tobefnz; /* list of userdata to be GC */
GCObject *fixedgc; /* list of objects not to be collected */
/* fields for generational collector */
Expand Down
4 changes: 0 additions & 4 deletions ltests.c
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ static void checkudata (global_State *g, Udata *u) {
static void checkproto (global_State *g, Proto *f) {
int i;
GCObject *fgc = obj2gco(f);
checkobjref(g, fgc, f->cache);
checkobjref(g, fgc, f->source);
for (i=0; i<f->sizek; i++) {
if (ttisstring(f->k + i))
Expand Down Expand Up @@ -417,8 +416,6 @@ static void checkobject (global_State *g, GCObject *o, int maybedead,
getage(o) == G_TOUCHED1 ||
getage(o) == G_OLD0 ||
o->tt == LUA_TTHREAD ||
(o->tt == LUA_TPROTO &&
(gco2p(o)->cache != NULL || gco2p(o)->cachemiss >= MAXMISS)) ||
(o->tt == LUA_TUPVAL && upisopen(gco2upv(o))));
}
}
Expand Down Expand Up @@ -452,7 +449,6 @@ static void checkgrays (global_State *g) {
checkgraylist(g, g->grayagain);
checkgraylist(g, g->weak);
checkgraylist(g, g->ephemeron);
checkgraylist(g, g->protogray);
}


Expand Down
39 changes: 2 additions & 37 deletions lvm.c
Original file line number Diff line number Diff line change
Expand Up @@ -683,31 +683,9 @@ lua_Integer luaV_shiftl (lua_Integer x, lua_Integer y) {
}


/*
** check whether cached closure in prototype 'p' may be reused, that is,
** whether there is a cached closure with the same upvalues needed by
** new closure to be created.
*/
static LClosure *getcached (Proto *p, UpVal **encup, StkId base) {
LClosure *c = p->cache;
if (c != NULL) { /* is there a cached closure? */
int nup = p->sizeupvalues;
Upvaldesc *uv = p->upvalues;
int i;
for (i = 0; i < nup; i++) { /* check whether it has right upvalues */
TValue *v = uv[i].instack ? s2v(base + uv[i].idx) : encup[uv[i].idx]->v;
if (c->upvals[i]->v != v)
return NULL; /* wrong upvalue; cannot reuse closure */
}
p->cachemiss = 0; /* got a hit */
}
return c; /* return cached closure (or NULL if no cached closure) */
}


/*
** create a new Lua closure, push it in the stack, and initialize
** its upvalues. ???
** its upvalues.
*/
static void pushclosure (lua_State *L, Proto *p, UpVal **encup, StkId base,
StkId ra) {
Expand All @@ -724,13 +702,6 @@ static void pushclosure (lua_State *L, Proto *p, UpVal **encup, StkId base,
ncl->upvals[i] = encup[uv[i].idx];
luaC_objbarrier(L, ncl, ncl->upvals[i]);
}
if (p->cachemiss >= MAXMISS) /* too many missings? */
p->cache = NULL; /* give up cache */
else {
p->cache = ncl; /* save it on cache for reuse */
luaC_protobarrier(L, p, ncl);
p->cachemiss++;
}
}


Expand Down Expand Up @@ -1811,13 +1782,7 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
}
vmcase(OP_CLOSURE) {
Proto *p = cl->p->p[GETARG_Bx(i)];
LClosure *ncl = getcached(p, cl->upvals, base); /* cached closure */
if (ncl == NULL) { /* no match? */
savestate(L, ci); /* in case of allocation errors */
pushclosure(L, p, cl->upvals, base, ra); /* create a new one */
}
else
setclLvalue2s(L, ra, ncl); /* push cashed closure */
halfProtect(pushclosure(L, p, cl->upvals, base, ra));
checkGC(L, ra + 1);
vmbreak;
}
Expand Down
13 changes: 6 additions & 7 deletions testes/closure.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,17 @@ assert(B.g == 19)

-- testing equality
a = {}
collectgarbage"stop"
for i = 1, 5 do a[i] = function (x) return x + a + _ENV end end
collectgarbage"restart"
assert(a[3] == a[4] and a[4] == a[5])

for i = 1, 5 do a[i] = function (x) return i + a + _ENV end end
assert(a[3] ~= a[4] and a[4] ~= a[5])

local function f()
return function (x) return math.sin(_ENV[x]) end
do
local a = function (x) return math.sin(_ENV[x]) end
local function f()
return a
end
assert(f() == f())
end
assert(f() == f())


-- testing closures with 'for' control variable
Expand Down

0 comments on commit e8c7797

Please sign in to comment.