20,741
edits
m (→Contexts: moving from Nasal Maintenance) |
|||
Line 1,016: | Line 1,016: | ||
= Contexts = | = Contexts = | ||
{{WIP}} | |||
{{cquote|<nowiki>New nasal objects are added to a temporary | |||
bin when they are created, because further allocation might cause a | |||
garbage collection to happen before the code that created the object | |||
can store a reference to it where the garbage collector can find it. | |||
For performance and simplicity, this list is stored per-context. When | |||
the context next executes code, it clears this list. | |||
Here's the problem: we do a lot of our naNewXX() calls in FlightGear | |||
using the old "global context" object that is created at startup. But | |||
this context is no longer used to execute scripts* at runtime, so as | |||
Csaba discovered, it's temporaries are never flushed. That | |||
essentially causes a resource leak: those allocations (mostly listener | |||
nodes) will never be freed. And all the extra "reachable" Nasal data | |||
floating around causes the garbage collector to take longer and longer | |||
to do a full collection as time goes on, causing "stutter". And | |||
scripts that use listeners extensively (the cmdarg() they use was one | |||
of the affected allocations) will see the problem more seriously. | |||
(That's a feature, not a bug. Once listeners were added, scripts | |||
could be recursive: (script A sets property X which causes listener | |||
L to fire and cause script B to run) We need two or more contexts on | |||
the stack to handle that; a single global one won't work.) | |||
I didn't like the fix though. Exposing the temporary bin as part of | |||
the Nasal public API is ugly; it's an internal design feature, not | |||
something users should tune. Instead, I just hacked at the FlightGear | |||
code to reinitialize this context every frame, thus cleaning it up. A | |||
"proper" fix would be to remove the global context entirely, but that | |||
would touch a bunch of code. | |||
</nowiki><ref>{{cite web |url=http://www.mail-archive.com/flightgear-devel@lists.sourceforge.net/msg13028.html|title=<nowiki>[Flightgear-devel] Stutter/Nasal issue resolved (was: FlightGear/Plib periodic stutter notes)</nowiki>|author=<nowiki>Andy Ross</nowiki>|date=<nowiki>Wed, 24 Oct 2007 11:33:36 -0700</nowiki>}}</ref>|<nowiki>Andy Ross</nowiki>}} | |||
<references/> | |||
Also see: http://gitorious.org/fg/flightgear/blobs/next/src/Scripting/NasalSys.cxx (in FGNasalSys::update) | |||
<syntaxhighlight lang="c"> | |||
// The global context is a legacy thing. We use dynamically | |||
// created contexts for naCall() now, so that we can call them | |||
// recursively. But there are still spots that want to use it for | |||
// naNew*() calls, which end up leaking memory because the context | |||
// only clears out its temporary vector when it's *used*. So just | |||
// junk it and fetch a new/reinitialized one every frame. This is | |||
// clumsy: the right solution would use the dynamic context in all | |||
// cases and eliminate _context entirely. But that's more work, | |||
// and this works fine (yes, they say "New" and "Free", but | |||
// they're very fast, just trust me). -Andy | |||
</syntaxhighlight> | |||
'''Consider using coccinelle here to rewrite naContext handling in all places at once''' | |||
[[Category:Developer Plans]] | |||
[[File:NasalContextStruct.png|thumb|500px|Nasal Context structure]] | [[File:NasalContextStruct.png|thumb|500px|Nasal Context structure]] |