[Asterisk-code-review] main/pbx: Improve performance of dialplan reloads with large... (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: main/pbx: Improve performance of dialplan reloads with large numbers of hints
......................................................................


Patch Set 4: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/312/4/main/pbx.c
File main/pbx.c:

Line 1164: 	for (; i < AST_VECTOR_SIZE(&hint->devices); i++) {
         : 		struct hintdevice_remove_data remove_data;
         : 		char *device = AST_VECTOR_GET(&hint->devices, i);
         : 
         : 		if (!device) {
         : 			continue;
         : 		}
         : 
         : 		remove_data.device = device;
         : 		remove_data.hint = hint;
         : 
         : 		ao2_t_callback(hintdevices, OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA,
         : 			hintdevice_remove_cb, &remove_data, "Remove device from container");
         : 		AST_VECTOR_REMOVE_UNORDERED(&hint->devices, i);
         : 		ast_free(device);
         : 	}
I just saw this for loop and it seems odd to me. i increases with each iteration, but the vector also shrinks. So it seems like you'll only iterate over half the entries in the vector.

Consider a vector with elements:
    A B C D E F

On your first iteration, you'll remove element A at position 0. When you remove an element from the vector, it shrinks but has to keep the elements packed at the front. One strategy for this might be to move the final element into the spot that was just erased, like so:
    F B C D E

Now i is 1 when you run the loop again. So now you remove element B. Now the vector might look like
    F E C D

Now i is 2 when you run the loop again. So now you remove element C. Now the vector might look like
    F E D

Now when i is 3, it's not less than the size of the vector, so the loop exits. You never visited items F, E, and D though.

Did the original suggestion I made not work correctly? With it, you always address the first item in the vector and allow the shrinking size of the vector dictate when to break out of the loop.


-- 
To view, visit https://gerrit.asterisk.org/312
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iedfc97a69d21070c50fca42275d7b3e714e59360
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes