[osg-users] Refactoring DatabasePager NeedToRemove stringflagging technique
lewandowski at ai.com.pl
Mon Nov 23 03:53:45 PST 2009
I wish to join the discussion because we are currently dealing with real problem related to this topic.
First some background. Its a long comedy of circumstances and sideffects. But please bear with me...:
1. We use PagedLOD database. We used our tools to build it, but it does not really differ from osgDEM produced DB. We use it for rendering and intersections.
2. Its a simulation. We need to position and move many entities over terrain so we need exact intersections. To get exact elevations we use setLODSelectionMode( USE_HGHEST_LEVEL_OF_DETAIL ) to make sure IntersectionVisitor computes correct intersection.
3. I don't know if this was deliberate or not, but IntersectionVisitor in USE_HGHEST_LEVEL_OF_DETAIL mode does load highest level tiles temporary. It does intersections and free them. It does not hook them up to their parents.
4. Above has this horrible effect that if some node was not already loaded by DatabasePager it will be constantly loaded and removed by IntersectionVisitor. SARCASM#1: But don't loose faith there is a solution. See next point.
5. We found out that we can use osgSim::DatabaseCacheReadCallback to mitigate former problem. This is an IntersectVisitor read callback that keeps internal cache to avoid repetitive loading and freeing. Well...this works but only for IntesectVisitor. DatabasePager does not know anything about this cache so when DatabasePager finally decides to load a tile it does it again, although IntersectionVisitor have already loaded it. SARCASM#2: But don't loose hope yet, because we found a solution to this as well....
6. How to make sure DatabasePager sees the tile IntersectVisitor already loaded ? Its simple: We could use osgDB cache. So we started to load tiles with CACHE_NODES option and everything seemed to be fine....
7 But after some time we started to observe crashes and memory leaks. Long story about them is in my friend Pawel Ksiezopolski post "Re: [osg-users] PagedLOD experts?" from November 5th. Short story is that caching tiles does not free renamed "NeedToRemove" nodes, but keep them in memory so with time some if them land int the scene again. When this happens, logic that was invented to remove not used nodes removes wrong ones leaving those that should be removed. Hence we get PageLOD thrashing and memory leaks. Cool HUH ? (Yes its SARCASM#3)
Now to conclusion:
In my opinion PageLOD renaming is the reason our elaborated scheme failed. I am both unhappy and glad that it happened because it clearly shows that Object::_name should not be modified by internal OSG methods. It should be never ever changed by OSG. Management of node name belongs to creators or users of the node.
Thats why I am going to prepare a fix that will not rename nodes to remove. But will instead drop their addresses into a set (or sorted vector) and will later us this set to test if node is marked for removal. It may be not O(n) but O(n log n) but it will work at least. Is anyone preparing similar fix ?
----- Original Message -----
From: "Robert Osfield" <robert.osfield at gmail.com>
To: "OpenSceneGraph Users" <osg-users at lists.openscenegraph.org>
Sent: Friday, November 20, 2009 10:58 AM
Subject: Re: [osg-users] Refactoring DatabasePager NeedToRemove stringflagging technique
On Mon, Nov 2, 2009 at 4:05 PM, Chris 'Xenon' Hanson
<xenon at alphapixel.com> wrote:
> I'm kind of an old-school C-flavor programmer, so my instinctive response is rather
> C-like. I don't like bloating people's in-memory objects, so I'd lean towards defining a
> 32-bit flag member on class Node called something like _ephemeralInternalStateFlags. This
> would not be saved or restored, and the definitions of each flag bit would be enumerated
> as 1<<n enums in the Node class header. Code outside of the OSG core distribution would be
> prohibited (by policy, can't really enforce it in the compiler) from defining and
> utilizing these flag bits, but code within OSG core could. The first bit allocated would
> be used to replace the NeedToRemove indicator.
I'm not sure whether this is really needed. The NeedToRemove trick in
DatabasePager is very specific to a piece of code embedded in
DatabasePager it has no value outside this specific bit of code, and I
believe it's not appropriate to build algorithm specific such support
into the core OSG.
> Also, while the read-modify-write cycle used to toggle just one bit of a flag is not as
> fast as the blind-write cycle used when you dedicate a whole boolean variable (and consume
> more memory), both the setting operation AND the testing operation should be faster than
> the current string-marking technique.
> If Robert has no objections to this solution, I'll code it up quick and share it.
I think the right thing to do is nothing in the core OSG, and look at
the specific DatabasePager algorithm to retain the O(1) cost of the
check that I was using, with the bugs with handling multiple
> I'm also concerned about the amount of very similar-but-not-quite-the-same code between
> DatabasePager::capped_removeExpiredSubgraphs and
> DatabasePager::expiry_removeExpiredSubgraphs. I'd like to refactor that to DRY (Don't
> Repeat Yourself) it out a bit as I'm implementing some code of my own that needs to
> cleanly dispose of some PagedLOD children. But, I don't understand why there are
> differences in the disposal technique between them (one calls
> _fileRequestQueue->updateBlock() directly, one sets a local flag and calls updateBlock()
> at the end of the operation, etc).
Both methods exists while we test the new
capped_removeExpiredSubgraphd , once it's fully tested and debugged
the old expiry_removeExpiredSubgraphs method will be removed
completely this neatly avoiding the issue of similar code.
osg-users mailing list
osg-users at lists.openscenegraph.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the osg-users