<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 11/20/2014 07:31 AM, Dave WOOLLEY
wrote:<br>
</div>
<blockquote
cite="mid:A416928F598C064AA19BBF21620CD32902CB269CDF@artoo.bts.co.uk"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
<div class="WordSection1">
<p class="MsoNormal">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.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">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:<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">2310 case
AST_DEVICE_INUSE: <o:p></o:p></p>
<p class="MsoNormal">2311 case
AST_DEVICE_BUSY: <o:p></o:p></p>
<p class="MsoNormal">2312 case
AST_DEVICE_RINGING: <o:p></o:p></p>
<p class="MsoNormal">2313 case
AST_DEVICE_RINGINUSE: <o:p></o:p></p>
<p class="MsoNormal">2314 case
AST_DEVICE_ONHOLD: <o:p></o:p></p>
<p class="MsoNormal">2315
if (!mem->ringinuse) {
<o:p></o:p></p>
<p class="MsoNormal">2316
break;
<o:p></o:p></p>
<p class="MsoNormal">2317
} <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">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?</p>
</div>
</blockquote>
<br>
Yeah that looks wrong to me. When I was last involved in writing
queue behavior code, the queue was supposed to never attempt to
send calls to busy members, ever. The ringinuse option should only
have an effect on members who are in use. It should have no effect
on members who are busy. It's possible that the code moved in its
current direction "organically" through community patches, or it
could just be a mistake.<br>
<br>
<blockquote
cite="mid:A416928F598C064AA19BBF21620CD32902CB269CDF@artoo.bts.co.uk"
type="cite">
<div class="WordSection1">
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Next, in can_ring_entry, ringinuse set
completely bypasses the device state check:<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">4169 if
(!call->member->ringinuse &&
!member_status_available(call->member->status)) {
<o:p></o:p></p>
<p class="MsoNormal">4170 ast_debug(1, "%s
not available, can't receive call\n", call->interface);
<o:p></o:p></p>
<p class="MsoNormal">4171 return 0; <o:p></o:p></p>
<p class="MsoNormal">4172 }<o:p></o:p></p>
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal">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:<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">4182 if (use_weight &&
compare_weight(qe->parent, call->member)) {
<o:p></o:p></p>
<p class="MsoNormal">4183 ast_debug(1,
"Priority queue delaying call to %s:%s\n",
<o:p></o:p></p>
<p class="MsoNormal">4184
qe->parent->name, call->interface);
<o:p></o:p></p>
<p class="MsoNormal">4185 return 0; <o:p></o:p></p>
<p class="MsoNormal">4186 }<o:p></o:p></p>
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal">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.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">So, finally, looking at compare_weight,
this line:<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">4080
if (q->weight > rq->weight && q->count
>= num_available_members(q)) {<o:p></o:p></p>
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal">Is run after this line:<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">4078
if ((mem = ao2_find(q->members, member, OBJ_POINTER))) {<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">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.</p>
</div>
</blockquote>
<br>
This is a case where patches are certainly welcome. The
q->members container should be a hash table for all queue
strategies aside from the linear strategy. That requires the use of
a linked list. Assuming the worst case at all times, you're right
that the search for a specific member is going to be more expensive
than the check of the queue weight. This could probably be
refactored to compare weights first, then find the specified member,
and then finally check the number of available members in the queue.<br>
<br>
<blockquote
cite="mid:A416928F598C064AA19BBF21620CD32902CB269CDF@artoo.bts.co.uk"
type="cite">
<div class="WordSection1">
<p class="MsoNormal"><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">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.)<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<br clear="all">
BTS Holdings PLC - Registered office: BTS House, Manor Road,
Wallington, SM6 0DD - Registered in England: 1517630<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
<br>
</body>
</html>