Table alloc tracking #267
Table alloc tracking #267
Conversation
allows tracking the max number of tables and table keys used by an LState, so that its memory usage can be roughly tracked and limited.
|
@yuin what do you think? Happy to hear your thoughts. |
|
Awesome work and great contribution @tul! I'm very sorry about this. I'm very busy with my work until end of March. I will look this PR once things settle down at work. |
| // is because the capacity of a slice is automatically managed by the go runtime and is free to change between versions | ||
| // of go. This would mean Gopher Lua could behave differently between different version of go, which is not desirable. | ||
| // We could switch to using capacity if we moved away from using the `append` built in for the table slice. | ||
| type LTableAllocInfo struct { |
There was a problem hiding this comment.
LTableAllocInfo fields are private. It seems useless for users.
| // CheckQuota checks if this table's alloc info's quotas have been exceeded and if so invoke's the LState's quota | ||
| // exceeded callback. CheckQuota is called automatically by vm operation which modify tables, so only needs to be | ||
| // invoked directly if you are writing code which has directly added keys to a table using the Raw set methods. | ||
| func (tb *LTable) CheckQuota(L *LState) { |
There was a problem hiding this comment.
I feel uncomfortable that the function takes *LState . This function should be implemented as a method of LState .
| @@ -1129,6 +1138,8 @@ func (ls *LState) setField(obj LValue, key LValue, value LValue) { | |||
| ls.RaiseError("attempt to index a non-table object(%v) with key '%s'", curobj.Type().String(), key.String()) | |||
| } | |||
| ls.RawSet(tb, key, value) | |||
| // potentially a new key was created, so check the quota | |||
| tb.CheckQuota(ls) | |||
There was a problem hiding this comment.
This statement should be wrapped by if to avoid needless function call that relatively high cost.
if ls.tblAllocInfo != nil {
tb.CheckQuota(ls)
}
|
@yuin Apologies, I have only just seen your comments. I will review them and respond next time I'm in the code area. I also have a further update to this PR to optionally force a GC when a quota is exceeded, as I found that Go's lazy GC would sometimes lead to Lua scripts with a high churn of |
|
@tul Thanks for the patch. Any update on getting this over the line? |
|
To be honest after having used this in production for a while I’m considering alternative approaches. The problems I’ve had with this approach:
So in short, I’m thinking of alternative approaches, possibly a combination of light weight finaliser based counting like this, combined with registry analysis to determine where tables are retained (essentially allowing a per LState “retention graph” to be produced). |
Fixes #250
Changes proposed in this pull request: