5 a.m.
On Fri, Mar 16, 2012 at 11:54 AM, Patrick Ohly <patrick.ohly(a)intel.com> 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