<br>Hey Sebastian<br><br><div class="gmail_quote">On Mon, May 31, 2010 at 4:19 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;">
Hi Vishesh,<br>
<div class="im"><br>
On 05/31/2010 11:03 AM, Vishesh Handa wrote:<br>
> I also moved store() and load() calls back into ResourceData. This is<br>
> rather trivial. But then you mentioned how ugly it was that ResourceData<br>
> could delete itself and how one would have to take care of which methods<br>
> to call.<br>
> So I changed determineUri() so that it returns the actual ResourceData<br>
> to use instead of deleting itself. That way Resource *could* simply do:<br>
><br>
> m_data = m_data->determineUri()<br>
><br>
> and it would be enough. And we would not even need the m_resources list.<br>
> But then the same thing would have to be done for each copy of that<br>
> resource using the ResourceData in question. Thus, I added the method<br>
> Resource::determineFInalResourceData which basically does what<br>
> ResourceData::replaceWith did before.<br>
><br>
><br>
> Alright. Just so that we both are on the same page, I'm going to tell<br>
> what my plans were after this humongous patch.<br>
> 1. Possible merge both the lists (If you allow!)<br>
<br>
</div>This I still do not understand. How is that possible?<br></blockquote><div><br>Resources are identified by one of these 3 ways -<br>1. nao:identifier<br>2. nie:url (includes the whole filex:/ part)<br>3. nepomuk:/res/<br>
<br>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? ) <br><br>The determineUri code would become a lot simpler -<br>
<br>if( m_kickOffUri.isValid() ) {<br> // Either 2 or 3<br>}<br>else {<br> // nao:identifier<br>}<br><br>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.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> 2. Convert the ResourceData m_kickOffUri into a list AND make sure that<br>
> while determining one URI it adds all other cases to the lists as well.<br>
><br>
> Number 1 is more of a convenience, but *2* is really important. You've<br>
> done half the job ( I thought we'll take care of it in another patch )<br>
><br>
> Now, about the comments in determineFinalResourceData(). The flaw with<br>
> our, not so little, proxy removal plan was that if -<br>
> Resource r1("foo");<br>
> r1.determineUri()<br>
><br>
> Resource r2( foo's nie:url );<br>
> r2.determineUri() // The proxy thing would be activated ( could be<br>
> avoided via 2 )<br>
><br>
> Resource r3( foo's nie:url );<br>
> r3.determineUri() // The proxy thing is AGAIN activated as the nie:url<br>
> wasn't added to the list.<br>
><br>
> With your patch you seem to have fixed the problem. But I would have<br>
> preferred the more concrete solution via 2.<br>
<br>
</div>Agreed.<br></blockquote><div><br>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. <br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> So far so good and already confusing enough. But then there is the<br>
> problem of the kickoff lists. With proxies we did not have to care about<br>
> the old kickoff ids and uris since the ResourceDatas using proxies were<br>
> still there "redirecting" to the proxies. Now we delete these old ones.<br>
> Thus, if another Resource would be created with the same kickoff id or<br>
> uri the whole process would be restarted. That is why I changed the<br>
> kickoff id and uri in ResourceData into lists and simply added the new<br>
> ResourceData multiple times to the kickoff lists in<br>
> ResourceManagerPrivate.<br>
><br>
><br>
> Yup ^^<br>
><br>
> BTW, we'll need to fix cleanUpCache as well. Currently, (haven't tested)<br>
> it should crash. This is because it would try to remove the same<br>
> ResourceData multiple times. The fix is a simple conversion of the list<br>
> into a set. :)<br>
<br>
</div>right.<br>
<div class="im"><br>
> Another problem would be determineAllUris(). It could crash cause<br>
> determineUri may delete one of the members to be accessed.<br>
<br>
</div>man, this code is too scattered. I already tried to implement<br>
determineFinalResourceData by using the Resource constructor but that<br>
would crash since the ResourceData would be deleted before I unlocked<br>
its mutex...<br></blockquote><div><br>Yea. It is really scattered. :-/<br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> I hope that you are not completely confused now. :P<br>
> I am still not totally happy with it since it it still rather complex<br>
> although having no proxies is already nice...<br>
><br>
><br>
> I'm kinda having second thoughts about this patch. We're completely<br>
> removing proxies but in the process we've imploded the code into a<br>
> rather complex (actually it isn't that much) solution. But then I never<br>
> liked the idea of proxies.<br>
><br>
> So, then my question to you is - "How big of a overhead would it be to<br>
</div>> derive ResourceData from *QObject*?"<br>
<br>
what for? IMHO there is no reason to do that.<br></blockquote><div><br>Cancel that. I'd forgotten about the multi-threading problem.<br><br>- Vishesh Handa<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
> BTW, Should I incorporate 1 or 2 in the patch and fix determineAll and<br>
> cleanUpCache?<br>
<br>
</div>2 can go into the patch, yes. As for 1: first I need to understand how<br>
that can be done. :)<br>
<br>
Cheers,<br>
<font color="#888888">Sebastian<br>
</font><div class="im"><br>
> - Vishesh Handa<br>
><br>
><br>
> Cheers,<br>
> Sebastian<br>
><br>
><br>
> On 05/29/2010 04:37 PM, Vishesh Handa wrote:<br>
> ><br>
> > I think it should work now. I removed the MutexLocker from the<br>
> inside of<br>
> > determineUri().<br>
> ><br>
> > - Vishesh Handa<br>
> ><br>
> > On Sat, May 29, 2010 at 7:32 PM, Vishesh Handa<br>
> <<a href="mailto:handa.vish@gmail.com">handa.vish@gmail.com</a> <mailto:<a href="mailto:handa.vish@gmail.com">handa.vish@gmail.com</a>><br>
</div><div class="im">> > <mailto:<a href="mailto:handa.vish@gmail.com">handa.vish@gmail.com</a> <mailto:<a href="mailto:handa.vish@gmail.com">handa.vish@gmail.com</a>>>> wrote:<br>
> ><br>
> ><br>
> > On Sat, May 29, 2010 at 7:17 PM, Sebastian Trüg <<a href="mailto:trueg@kde.org">trueg@kde.org</a><br>
> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
</div><div><div></div><div class="h5">> > <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>>> wrote:<br>
> ><br>
> > On 05/29/2010 03:34 PM, Vishesh Handa wrote:<br>
> > > On Sat, May 29, 2010 at 5:46 PM, Sebastian Trüg<br>
> <<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> > <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>><br>
> > > <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>><br>
> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a> <mailto:<a href="mailto:trueg@kde.org">trueg@kde.org</a>>>>> wrote:<br>
> > ><br>
> > > Hehe, this does not help at all. Think about it: in my<br>
> > example there are<br>
> > > 2 Resource instances involved. Thus: 2 mutexes which are<br>
> > locked<br>
> > > independent of each other. :)<br>
> > > The mutex is already there in ResourceData. It simply<br>
> > needs to be locked<br>
> > > in Resource instead of ResourceData::determineUri.<br>
> > ><br>
> > ><br>
> > > Uhh I'm confused. Why don't you handle the multi-threading?<br>
> ><br>
> > Sure, I can do that. :)<br>
> ><br>
> ><br>
> > Wait! Please don't. Let me try. I understand it now. (I think)<br>
> ><br>
> ><br>
> ><br>
> > > I should really learn about multi-threading. If you have a<br>
> > couple of<br>
> > > spare minutes could you explain why my method won't work?<br>
> > ><br>
> > > My rationale -<br>
> > ><br>
> > > Thread 1 :<br>
> > > Resource r1("foo");<br>
> > > r1.property( nao:numericRating )<br>
> > > -> the mutex is locked<br>
> > > -> performs whatever and determines the uri<br>
> > ><br>
> > > Thread 2 :<br>
> > > Resource r2("foo");<br>
> > > r2.setProperty( whatever )<br>
> > > -> the mutex can't get locked so it waits till it does<br>
> > > -> mutex now locked. Thread 1 should have determined the uri<br>
> > by now<br>
> > > -> perform operation<br>
> ><br>
> > Simple: r1 and r2 have different mutex intances. Thus, locking<br>
> > one does<br>
> > not prevent the other from being locked. The idea is that both<br>
> > threads<br>
> > need to lock the same mutex. And that is only possible if the<br>
> > mutex is<br>
> > stored in ResourceData.<br>
> ><br>
> ><br>
> > Oh. Of course. Thanks for explanation. :-)<br>
> ><br>
> > - Vishesh Handa<br>
> ><br>
> > Cheers,<br>
> > Sebastian<br>
> ><br>
> ><br>
> ><br>
><br>
><br>
</div></div></blockquote></div><br>