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.

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...

In a nutshell, I think we need to go back to the original std::list and
its brute-force insertion sort. Agreed?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.


_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution

Reply via email to