[Nepomuk] Nepomuk Core - Questions & Patches

[Vishesh Handa]

Hi Sebastian

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 -

On Tue, May 11, 2010 at 1:50 PM, Sebastian Trüg <trueg at kde.org> wrote:

> One solution to the proxy situation would be to not use proxies but let
> ResourceData have backlinks to all its Resource instances. Then one
> ResourceData could be replaced with another like so:
>
> void ResourceData::replaceWith( ResourceData* other )
> {
>    foreach( Resource* res, m_resourceWrappers ) {
>       res->m_data = other;
>       other->m_resourceWrappers.append(res);
>    }
>    m_resourceWrappers.clear();
>    delete this;
> }
>
> m_resourceWrappers is a bad name though.
>
>
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 -

1. Proxies are confusing, and clutter up the ResourceData code.
2. CleanUpCache currently does not handle proxies correctly (read prev
emails)
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.
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)

There may be more problems which I'm not aware of.

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 -

*Reference Counting - *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.

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.

This patch doesn't solve problem 4.

*Other stuff not related to this proxy patch -*

*tools.h - *This has an overloaded function called
*convertResourceList*,* *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 *convertToResourceList* and *convertFromResourceList* function?

*ResourceManagerPrivate - *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.
*
Resource::operator== - *Shouldn't it check if the data Cache is full and
accordingly delete? Just like Resource's destructor.

*Resource Class -* 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.
This is just something we should think about.
*
Random Idea - *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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20100516/d63137bb/attachment.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: removeProxyPatch.diff
Type: text/x-patch
Size: 12363 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/nepomuk/attachments/20100516/d63137bb/attachment.diff