[Nepomuk] Nepomuk Core - Questions & Patches
Vishesh Handa
handa.vish at gmail.com
Mon May 31 13:44:53 CEST 2010
Hey Sebastian
On Mon, May 31, 2010 at 4:19 PM, Sebastian Trüg <trueg at kde.org> wrote:
> 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?
>
Resources are identified by one of these 3 ways -
1. nao:identifier
2. nie:url (includes the whole filex:/ part)
3. nepomuk:/res/
m_kickOffId uses 1, 2 (minus the filex:/) & 3. And m_kickOffUri uses 2 & 3.
I say, we scrap the m_kickOffId, and store the nao:identifier in the
m_kickOffUri ( perhaps name it to something else? )
The determineUri code would become a lot simpler -
if( m_kickOffUri.isValid() ) {
// Either 2 or 3
}
else {
// nao:identifier
}
The would even clear one of the odd cases where someone tries to initialize
a Resource by providing its nepomuk:/res/ uri as a QString instead a QUrl.
> > 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.
>
How about converting them into hash tables instead of lists? That way we
could easily check if addProperty() or removeProperty() updates any of the
identifying properties in m_kickOddUri. We could then even potentially stop
clearing up the cache after ever metadata move operation.
>
> > 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...
>
Yea. It is really scattered. :-/
>
> > 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.
>
Cancel that. I'd forgotten about the multi-threading problem.
- Vishesh Handa
>
> > 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
> > >
> > >
> > >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20100531/95801107/attachment-0001.htm
More information about the Nepomuk
mailing list