Removed internal cache for closures

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.)
This commit is contained in:
Roberto Ierusalimschy 2018-11-01 13:21:00 -03:00
parent 2fc6b55dae
commit e8c779736f
9 changed files with 10 additions and 123 deletions

View File

@ -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;

64
lgc.c
View File

@ -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! */
@ -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);
@ -534,32 +513,6 @@ 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
@ -567,7 +520,6 @@ static void checkprotocache (global_State *g, Proto *p) {
*/
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]);
@ -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'
@ -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' */

4
lgc.h
View File

@ -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);
@ -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);

View File

@ -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;
@ -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 */

View File

@ -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;

View File

@ -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.
*/
@ -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 */

View File

@ -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))
@ -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))));
}
}
@ -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);
}

39
lvm.c
View File

@ -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) {
@ -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++;
}
}
@ -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;
}

View File

@ -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