Hi Sebastian<br><br>I sat down and thought of possible solutions, and in the end I came up with 3. The first was the keep the proxy as the Resource and not ResourceData, but this adds more problems than it solves. The second was for every ResourceData to have a link to the Resource that it contains. I thought this would solve all our problems. Then I realized that a ResourceData can be contained by multiple Resources. And there came the third, your, solution -<br>
<br><div class="gmail_quote">On Tue, May 11, 2010 at 1:50 PM, Sebastian Trüg <span dir="ltr"><<a href="mailto:trueg@kde.org">trueg@kde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
One solution to the proxy situation would be to not use proxies but let<br>
ResourceData have backlinks to all its Resource instances. Then one<br>
ResourceData could be replaced with another like so:<br>
<br>
void ResourceData::replaceWith( ResourceData* other )<br>
{<br>
foreach( Resource* res, m_resourceWrappers ) {<br>
res->m_data = other;<br>
other->m_resourceWrappers.append(res);<br>
}<br>
m_resourceWrappers.clear();<br>
delete this;<br>
}<br>
<br>
m_resourceWrappers is a bad name though.<br>
<br></blockquote><div><br>It seems to solves almost all our problems. Just so that everyone is on the same page, I'm going to state all the problems -<br><br>1. Proxies are confusing, and clutter up the ResourceData code.<br>
2. CleanUpCache currently does not handle proxies correctly (read prev emails)<br>3. Reference counting is wacky with proxies - ResourceData's are reference counted as long as they don't have a proxy, and after they have one, they use the same reference count instead of using the proxies. I find this confusing.<br>
4. The Metadata mover service currently cleans up the cache on every move. This coupled with the fact that Metadata mover currently tracks hidden files ( your .kde directory ) makes it clean up the cache very frequently. (Look at the metadata mover code)<br>
<br></div><div>There may be more problems which I'm not aware of.<br><br>Okay, now please look at the patch provided. It's a huge change, and patch is kinda messy. It will be improved. One minor thing -<br><br><b>Reference Counting - </b>Every time a Resource is created it needs to do reference counting and manage the ResourceData's m_resources list (I didn't like the name m_resourceWrappers) Because of this m_resources is currently public (temp). One possible solution is to handle the m_resources along with ResourceData's ref() and deref() functions. This would be a lot more intuitive. <br>
<br>Can/Should we remove reference counting via the AtomicInt variable entirely? Reference counting can still be done via m_resources.size(). But then there is the thing that all operations on an AtomicInt are guaranteed to be atomic.<br>
<br>This patch doesn't solve problem 4.<br><br><b>Other stuff not related to this proxy patch -</b> <br><br><b>tools.h - </b>This has an overloaded function called <b>convertResourceList</b>,<b> </b>which in one case converts into into a ResourceList and in the other case converts it from a ResourceList. Can we depreciate this function and add more appropriate <b>convertToResourceList</b> and <b>convertFromResourceList</b> function?<br>
<br><b>ResourceManagerPrivate - </b>This is a really stupid thing. The name ResourceManagerPrivate is a little misleading. Couldn't we name is ResourceDataManger instead? The class isn't accessible to the outside world, and we wouldn't be invalidating any APIs.<br>
<b><br>Resource::operator== - </b>Shouldn't it check if the data Cache is full and accordingly delete? Just like Resource's destructor.<br><br><b>Resource Class -</b> This is another really really stupid thing, and probably has no solution, but I'm still voicing it. The ResouceClass has too many dependencies. By dependencies I mean ontologies. It cannot function without these ontologies, and in a way that's the point. It provides convenience functions for many of those ontologies, but it lands up creating dependencies. I would typically want the core to be as isolated as possible and then have helper classes add extra dependencies. That way it would be a simple task to port it to another platform, say Meego, which doesn't need/want all those ontologies. <br>
This is just something we should think about.<br><b><br>Random Idea - </b>When clearing up the cache instead of just deleting the unused ones, maybe we could delete them based on some timestamps. This currently isn't required. <br>
</div></div>