On Fri, Mar 16, 2012 at 11:54 AM, Patrick Ohly <[email protected]> wrote:
> On Fri, 2012-03-16 at 11:33 +0100, Patrick Ohly wrote:
>> On Thu, 2012-03-15 at 14:43 +0100, Patrick Ohly wrote:
>> > I still think resources should be added to m_waitingResources before
>> > construction is complete. Activating the object and notifying the
>> > caller
>> > can wait until the resource is really usable (= helper ready).
>>
>> And one more observation: running the helper should get delayed until it
>> is really needed, which probably means until the resource is ready to
>> become active.
>>
>> Imagine having 100 pending session requests. An entry in
>> m_waitingResources is cheap enough to handle that (admittedly unlikely)
>> case, but combine that with 100 syncevo-dbus-helper processes and we
>> have a problem.
>
> Speaking of more than one entry in m_waitingResources: does the current
> PriorityCompare really implement a strict weak ordering, as required by
> std::set, the storage used for m_waitingResource?
>
> I don't think it does, because two different resources with the same
> priority will return false regardless which way you compare them, and
> that despite not being equivalent. I bet that as a result, you currently
> cannot store two resources with the same priority in m_waitingResource.
> Even if you could, what would be the relative order? Everything else
> being equal, we want first-come-first-serve.
>
oops, you're right. That should have been a multiset.
Here is a small test I'd written to make sure the ordering was correct.
// constructing multisets
#include <iostream>
#include <set>
using namespace std;
struct classComp {
bool operator() (const string& lhs, const string& rhs) const
{return lhs[0]>rhs[0];}
};
int main ()
{
multiset<string, classComp> setComp;
multiset<string>::iterator it;
setComp.insert("ace");
setComp.insert("bud");
setComp.insert("bad");
setComp.insert("bla");
setComp.insert("cup");
setComp.insert("boo");
// show content:
for ( it=setComp.begin() ; it != setComp.end(); it++ )
cout << (*it) << endl;
return 0;
}
The ordering works but for some reason, the "multi" prefix didn't make
it into the server.
> Another concern is that the outcome of PriorityCompare depends on being
> able to lock shared pointers. If a resource goes away, then the
> invariants for the set might get violated (comparison no longer returns
> the same results as before). I don't want to find out how std::set deals
> with that...
>
I believe I'm handling the case where the weak pointer goes away
already. If the lock is not obtained for the already-in-queue Resource
AND the to-insert Resource is locked the we move on to the next
comparison. If the to-insert Resource is gone then we stop right
there. I does get inserted but it'll be cleaned up in short time.
> In a nutshell, I think we need to go back to the original std::list and
> its brute-force insertion sort. Agreed?
>
I found it a bit nicer to use the comparison class but if you're not
convinced feel free to revert that.
Cheers,
Chris
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
