[Nepomuk] Nepomuk Core - Questions & Patches

[Sebastian Trüg]

Hi Vishesh,

On 05/31/2010 11:03 AM, Vishesh Handa wrote:
>     I also moved store() and load() calls back into ResourceData. This is
>     rather trivial. But then you mentioned how ugly it was that ResourceData
>     could delete itself and how one would have to take care of which methods
>     to call.
>     So I changed determineUri() so that it returns the actual ResourceData
>     to use instead of deleting itself. That way Resource *could* simply do:
> 
>     m_data = m_data->determineUri()
> 
>     and it would be enough. And we would not even need the m_resources list.
>     But then the same thing would have to be done for each copy of that
>     resource using the ResourceData in question. Thus, I added the method
>     Resource::determineFInalResourceData which basically does what
>     ResourceData::replaceWith did before.
> 
> 
> Alright. Just so that we both are on the same page, I'm going to tell
> what my plans were after this humongous patch.
> 1. Possible merge both the lists (If you allow!)

This I still do not understand. How is that possible?

> 2. Convert the ResourceData m_kickOffUri into a list AND make sure that
> while determining one URI it adds all other cases to the lists as well.
> 
> Number 1 is more of a convenience, but *2* is really important. You've
> done half the job ( I thought we'll take care of it in another patch )
> 
> Now, about the comments in determineFinalResourceData(). The flaw with
> our, not so little, proxy removal plan was that if -
> Resource r1("foo");
> r1.determineUri()
> 
> Resource r2( foo's nie:url );
> r2.determineUri() // The proxy thing would be activated ( could be
> avoided via 2 )
>
> Resource r3( foo's nie:url );
> r3.determineUri() // The proxy thing is AGAIN activated as the nie:url
> wasn't added to the list.
> 
> With your patch you seem to have fixed the problem. But I would have
> preferred the more concrete solution via 2.

Agreed.

>     So far so good and already confusing enough. But then there is the
>     problem of the kickoff lists. With proxies we did not have to care about
>     the old kickoff ids and uris since the ResourceDatas using proxies were
>     still there "redirecting" to the proxies. Now we delete these old ones.
>     Thus, if another Resource would be created with the same kickoff id or
>     uri the whole process would be restarted. That is why I changed the
>     kickoff id and uri in ResourceData into lists and simply added the new
>     ResourceData multiple times to the kickoff lists in
>     ResourceManagerPrivate.
> 
> 
> Yup ^^
> 
> BTW, we'll need to fix cleanUpCache as well. Currently, (haven't tested)
> it should crash. This is because it would try to remove the same
> ResourceData multiple times. The fix is a simple conversion of the list
> into a set. :) 

right.

> Another problem would be determineAllUris(). It could crash cause
> determineUri may delete one of the members to be accessed.

man, this code is too scattered. I already tried to implement
determineFinalResourceData by using the Resource constructor but that
would crash since the ResourceData would be deleted before I unlocked
its mutex...

>     I hope that you are not completely confused now. :P
>     I am still not totally happy with it since it it still rather complex
>     although having no proxies is already nice...
> 
> 
> I'm kinda having second thoughts about this patch. We're completely
> removing proxies but in the process we've imploded the code into a
> rather complex (actually it isn't that much) solution. But then I never
> liked the idea of proxies.
> 
> So, then my question to you is - "How big of a overhead would it be to
> derive ResourceData from *QObject*?"

what for? IMHO there is no reason to do that.

> BTW, Should I incorporate 1 or 2 in the patch and fix determineAll and
> cleanUpCache?

2 can go into the patch, yes. As for 1: first I need to understand how
that can be done. :)

Cheers,
Sebastian

> - Vishesh Handa
> 
> 
>     Cheers,
>     Sebastian
> 
> 
>     On 05/29/2010 04:37 PM, Vishesh Handa wrote:
>     >
>     > I think it should work now. I removed the MutexLocker from the
>     inside of
>     > determineUri().
>     >
>     > - Vishesh Handa
>     >
>     > On Sat, May 29, 2010 at 7:32 PM, Vishesh Handa
>     <handa.vish at gmail.com <mailto:handa.vish at gmail.com>
>     > <mailto:handa.vish at gmail.com <mailto:handa.vish at gmail.com>>> wrote:
>     >
>     >
>     >     On Sat, May 29, 2010 at 7:17 PM, Sebastian Trüg <trueg at kde.org
>     <mailto:trueg at kde.org>
>     >     <mailto:trueg at kde.org <mailto:trueg at kde.org>>> wrote:
>     >
>     >         On 05/29/2010 03:34 PM, Vishesh Handa wrote:
>     >         > On Sat, May 29, 2010 at 5:46 PM, Sebastian Trüg
>     <trueg at kde.org <mailto:trueg at kde.org>
>     >         <mailto:trueg at kde.org <mailto:trueg at kde.org>>
>     >         > <mailto:trueg at kde.org <mailto:trueg at kde.org>
>     <mailto:trueg at kde.org <mailto:trueg at kde.org>>>> wrote:
>     >         >
>     >         >     Hehe, this does not help at all. Think about it: in my
>     >         example there are
>     >         >     2 Resource instances involved. Thus: 2 mutexes which are
>     >         locked
>     >         >     independent of each other. :)
>     >         >     The mutex is already there in ResourceData. It simply
>     >         needs to be locked
>     >         >     in Resource instead of ResourceData::determineUri.
>     >         >
>     >         >
>     >         > Uhh I'm confused. Why don't you handle the multi-threading?
>     >
>     >         Sure, I can do that. :)
>     >
>     >
>     >     Wait! Please don't. Let me try. I understand it now. (I think)
>     >
>     >
>     >
>     >         > I should really learn about multi-threading. If you have a
>     >         couple of
>     >         > spare minutes could you explain why my method won't work?
>     >         >
>     >         > My rationale -
>     >         >
>     >         > Thread 1 :
>     >         > Resource r1("foo");
>     >         > r1.property( nao:numericRating )
>     >         > -> the mutex is locked
>     >         > -> performs whatever and determines the uri
>     >         >
>     >         > Thread 2 :
>     >         > Resource r2("foo");
>     >         > r2.setProperty( whatever )
>     >         > -> the mutex can't get locked so it waits till it does
>     >         > -> mutex now locked. Thread 1 should have determined the uri
>     >         by now
>     >         > -> perform operation
>     >
>     >         Simple: r1 and r2 have different mutex intances. Thus, locking
>     >         one does
>     >         not prevent the other from being locked. The idea is that both
>     >         threads
>     >         need to lock the same mutex. And that is only possible if the
>     >         mutex is
>     >         stored in ResourceData.
>     >
>     >
>     >     Oh. Of course. Thanks for explanation. :-)
>     >
>     >     - Vishesh Handa
>     >
>     >         Cheers,
>     >         Sebastian
>     >
>     >
>     >
> 
>