Whilst looking at the performance of the search for an available queue member
I've come across code that handles the ringinuse flag that seems to count busy
an inuse as the same, that doesn't make sense to me. I've also come across
very long standing code that means ringinuse disables state checks entirely,
allowing the code ot fall through to the, very expensive, compare_weights code.
Finally, the compare_weights code seems to waste a lot of time trying to match
a member in queues that don't have higher weights.
Looking at the current trunk version, revision 325483 introduced additional
"case" lines into, what is now, is_member_available, which treat busy and inuse
the same:
2310 case AST_DEVICE_INUSE:
2311 case AST_DEVICE_BUSY:
2312 case AST_DEVICE_RINGING:
2313 case AST_DEVICE_RINGINUSE:
2314 case AST_DEVICE_ONHOLD:
2315 if (!mem->ringinuse) {
2316 break;
2317 }
Whilst this postdates the code we are using, so doesn't directly affect us,
this seems wrong to me; I thought the whole subtlety behind calling it
ringinuse, not ringbusy, is that there is a difference between busy and in use.
Could someone explain?
Next, in can_ring_entry, ringinuse set completely bypasses the device state
check:
4169 if (!call->member->ringinuse &&
!member_status_available(call->member->status)) {
4170 ast_debug(1, "%s not available, can't receive
call\n", call->interface);
4171 return 0;
4172 }
The consequence of this is that, if ringinuse is set, even if the member is
unavailable (e.g. agent logged out), or definitively busy, the code falls
through to:
4182 if (use_weight && compare_weight(qe->parent, call->member)) {
4183 ast_debug(1, "Priority queue delaying call to
%s:%s\n",
4184 qe->parent->name, call->interface);
4185 return 0;
4186 }
compare_weight is very expensive, because it iterates over all queues and all
members of them. Also, at least in the version we are using, although I haven't
checked this in trunk, there is a lock held on the queue of interest and locks
are taken, in enumeration sequence, on all the other queues. This looks to me
that it will cause serialisation of access to that function.
So, finally, looking at compare_weight, this line:
4080 if (q->weight >
rq->weight && q->count >= num_available_members(q)) {
Is run after this line:
4078 if ((mem = ao2_find(q->members,
member, OBJ_POINTER))) {
Whilst the second term is expensive, involving a scan of all the members, the
first term is cheap. In our case, there are lots of members in common between
lots of queues, a lot of which are not at high priority. It seems to me that
doing the cheap test before the search for the member, would quickly eliminate
queues that could never pre-empt the agent.. Even if ao2_find hashes, it is
still going to be expensive compared with a compare of two, one level indirect,
scalars.
One other observation, is that compare_weight doesn't account for members that
are unavailable to the conflicting queue because there is an even higher
priority queue. NB the member in question need not be the same member as is
being considered for the call. (Without a more efficient algorithm, recursing
the check would add additional multipliers to the computational complexity, so
could well be unworkable.)
BTS Holdings PLC - Registered office: BTS House, Manor Road, Wallington, SM6
0DD - Registered in England: 1517630--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
