| 00:16.57 | *** part/#asterisk-dev kharwell (kharwell@nat/digium/x-yzpttkrnxhmiebgl) |
| 00:37.06 | snuff-work | ASTERISK-27620 |
| 00:37.42 | snuff-work | mm.. i thought there was a bot or similar that spammed the url to an issue.. |
| 00:45.07 | rmudgett | That bot's been dead for a long time. |
| 00:48.33 | snuff-work | ah well RIP ;) |
| 01:18.48 | *** join/#asterisk-dev infobot (ibot@rikers.org) |
| 01:18.48 | *** topic/#asterisk-dev is Asterisk Development Discussion -=- http://www.asterisk.org/developers -=- Tier 2 and 3.14159265 support is in #asterisk -=- Check out our blog! blogs.asterisk.org -=- Follow on Twitter at @AsteriskDev |
| 02:15.16 | *** join/#asterisk-dev sh_smith (~sh_smith@cpe-76-174-26-91.socal.res.rr.com) |
| 04:12.41 | *** join/#asterisk-dev leedm777 (~textual@50-81-29-44.client.mchsi.com) |
| 04:12.41 | *** mode/#asterisk-dev [+o leedm777] by ChanServ |
| 06:31.48 | *** join/#asterisk-dev jkroon (~jkroon@uls-154-73-35-201.wall.uls.co.za) |
| 07:27.07 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 07:44.20 | *** join/#asterisk-dev tsearle (~tsearle@37.19.10.33) |
| 07:44.21 | *** mode/#asterisk-dev [+o tsearle] by ChanServ |
| 08:01.36 | *** join/#asterisk-dev CELYA_ (~Thunderbi@2a01:e0a:d:1111:8968:38a7:666d:3bff) |
| 08:19.31 | *** join/#asterisk-dev tzafrir (~tzafrir@local.xorcom.com) |
| 08:19.31 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 08:23.44 | *** join/#asterisk-dev pchero_work (~pchero@109.70.54.56) |
| 13:14.35 | *** join/#asterisk-dev leedm777 (~textual@50-81-29-44.client.mchsi.com) |
| 13:14.35 | *** mode/#asterisk-dev [+o leedm777] by ChanServ |
| 14:27.34 | *** join/#asterisk-dev leedm777 (~textual@50-81-29-44.client.mchsi.com) |
| 14:27.34 | *** mode/#asterisk-dev [+o leedm777] by ChanServ |
| 14:37.55 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 15:01.10 | *** join/#asterisk-dev leedm777 (textual@nat/digium/x-rambdvpuerqjwhjc) |
| 15:01.11 | *** mode/#asterisk-dev [+o leedm777] by ChanServ |
| 15:08.02 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 15:08.27 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-mfvapdonbaxeskus) |
| 15:08.27 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 15:11.48 | *** join/#asterisk-dev bford (d8cff501@gateway/web/freenode/ip.216.207.245.1) |
| 15:11.48 | *** mode/#asterisk-dev [+o bford] by ChanServ |
| 15:44.42 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-onupaukdxhhisgbv) |
| 15:44.42 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 15:46.43 | *** join/#asterisk-dev rmudgett (rmudgett@nat/digium/x-tqaburmlyedfbzoe) |
| 15:46.43 | *** mode/#asterisk-dev [+o rmudgett] by ChanServ |
| 15:57.15 | *** join/#asterisk-dev Worldexe (~Worldexe@95-107-33-134.dsl.orel.ru) |
| 16:35.04 | *** join/#asterisk-dev jkroon (~jkroon@vc-nat-gp-n-41-13-94-187.umts.vodacom.co.za) |
| 17:30.14 | seanbright | module loading in the 15 branch seems... broken? |
| 17:30.26 | seanbright | almost as if modules.conf is not getting read |
| 17:30.37 | file | points at coreyfarrell |
| 17:30.52 | seanbright | ignore me |
| 17:30.56 | seanbright | i'm an idiot |
| 17:31.12 | file | noted |
| 17:31.14 | gtjoseph | autoload? |
| 17:31.16 | coreyfarrell | seanbright: that makes me feel better, I really didn't do much in 15 |
| 17:31.27 | seanbright | i was using [general] instead of [modules] |
| 17:31.31 | seanbright | because consistency |
| 17:32.41 | seanbright | i will say that 'module load<tab>' is slow as dogshit though |
| 17:32.47 | seanbright | could be my VM |
| 17:33.39 | seanbright | 7 seconds to return the list of completions. |
| 17:34.09 | file | that's a bit |
| 17:34.14 | coreyfarrell | module load should probably be converted to the new way.. currently does a directory scan and returns one result at a time (so it starts a directory scan once per match) |
| 17:35.55 | seanbright | have we always had this "don't show running modules in the list" feature or is that new? |
| 17:36.43 | coreyfarrell | seanbright: that's recent |
| 17:37.06 | seanbright | i'm guessing that is the source of the degredation i am seeing, but i don't have time to test |
| 17:37.35 | seanbright | creating a 200 element vector per match maybe? |
| 17:39.15 | coreyfarrell | I don't think so. see the call to filename_completion_function in module_load_helper. to find the 100th match we call filename_completion_function 100 times, each call scans the modules directory. |
| 17:40.44 | seanbright | will test |
| 17:40.47 | coreyfarrell | seanbright: can you confirm you do not get performance issue for any of the other module completions (module unload, module reload) |
| 17:41.08 | seanbright | those are speedy |
| 17:43.49 | seanbright | coreyfarrell: is the use of filename_completion_function new? |
| 17:43.58 | coreyfarrell | no |
| 17:44.09 | seanbright | ok, so something else is going on |
| 17:44.12 | seanbright | i'll work it out |
| 17:49.07 | coreyfarrell | seanbright: when you did 'module load<tab>' did you have a lot of modules that are not loaded? |
| 17:49.36 | seanbright | all of them |
| 17:49.45 | seanbright | save res_sorcery_* |
| 17:50.05 | seanbright | but i wouldn't call that "a lot" |
| 17:50.22 | seanbright | the behavior is definitely different |
| 17:50.41 | seanbright | we used to pass state to filename_completion_function (via complete_fn) |
| 17:50.50 | coreyfarrell | if you installed 400 modules and none were loaded, I'm pretty sure 'module load<tab>' would call filename_completion_function 84255 times. |
| 17:50.51 | seanbright | now we pass idx which is reset each time |
| 17:51.02 | seanbright | so that appears to be wrong |
| 17:51.48 | seanbright | i'm guessing if i remove the while loop from module_load_helper it will resolve the issue |
| 17:53.19 | seanbright | i'm assuming the while loop is needed for the 'already loaded' check |
| 17:53.52 | seanbright | maybe if you start with idx = state? |
| 17:54.09 | seanbright | rather than idx = 0 |
| 17:55.09 | coreyfarrell | that would give us no way to know how many were already skipped before state. |
| 17:55.28 | seanbright | ok |
| 17:55.35 | seanbright | well, it's slow |
| 17:55.37 | seanbright | god speed |
| 17:55.47 | seanbright | at minimum we should only build the list of running modules once |
| 17:55.52 | seanbright | instead of for each match |
| 17:56.27 | coreyfarrell | I just tried this from my laptop with 298 modules installed and none loaded, completion was immediate (not saying your wrong just it's not happening to me) |
| 17:56.35 | seanbright | you're* |
| 17:56.40 | seanbright | no problem, probably just my VM |
| 17:56.58 | coreyfarrell | give me a few minutes I'll post something to gerrit for you to test |
| 17:57.09 | seanbright | i'm already testing something |
| 17:57.18 | seanbright | (removing the already-running check and the while loop) |
| 18:00.31 | coreyfarrell | ok but that already running check was added for a reason. I'm testing something that should address performance without getting rid of the check. |
| 18:01.33 | seanbright | what was the reason? |
| 18:02.49 | seanbright | oh yeah... SO much better without that stuff |
| 18:02.57 | seanbright | maybe we should make it an option in asterisk.conf |
| 18:03.00 | coreyfarrell | if a module is already loaded it's not a valid option to 'module load'. or put another way if 295 out of 300 modules are loaded 'module load<tab>' should show 5 options. |
| 18:03.53 | seanbright | > 7 seconds to instantaneous |
| 18:03.58 | seanbright | probably just my VM |
| 18:07.22 | coreyfarrell | seanbright: please test https://gerrit.asterisk.org/8050 |
| 18:12.21 | seanbright | yup, that's better |
| 18:12.25 | seanbright | cherry pick to 15 please |
| 18:12.27 | coreyfarrell | weird, it looks like gerrit or the mailing list replaces all '_' in the subject line with spaces. |
| 18:15.48 | seanbright | does creating the vector make sense at this point? |
| 18:16.11 | seanbright | the module list is locked the entire time the function is running, why not just iterate it instead of copying into a vector? |
| 18:18.45 | coreyfarrell | probably a good point |
| 18:25.10 | coreyfarrell | seanbright: reviews updated. |
| 18:29.01 | seanbright | the only other optimization would be to have a version of filename_completion_function that just returns a list/vector of files in a given directory instead of the way it is done now |
| 18:29.07 | seanbright | but that is probably splitting hairs |
| 18:31.25 | coreyfarrell | the current patch means if astmoddir contains 400 modules which are not loaded we will run opendir 400 times instead of 84255. if you want to take the next step feel free. |
| 18:32.36 | seanbright | is opendir/readdir deterministic? |
| 18:32.48 | seanbright | i know it's not in alpha order but is it always the same order? |
| 18:35.11 | coreyfarrell | I'm not sure but I expect it's deterministic as long as no files are added/deleted between the runs. |
| 18:44.57 | seanbright | ast_file_read_dirs() would do the trick |
| 18:55.23 | *** join/#asterisk-dev tzafrir (~tzafrir@62-90-199-247.barak.net.il) |
| 18:55.23 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 18:56.54 | *** join/#asterisk-dev jkroon (~jkroon@105.12.4.96) |
| 19:13.34 | coreyfarrell | seanbright: ok you convinced me. using that function is a much cleaner. |
| 19:16.41 | seanbright | i knew you'd fold |
| 19:20.18 | seanbright | coreyfarrell: looks good! |
| 19:22.21 | coreyfarrell | when I realized that ast_file_read_dirs would give me the filename separate from dirname that sold it. |
| 19:26.13 | coreyfarrell | seanbright: at some point you were looking at trying to reduce ast_channel locking. did you determine it was impractical or just run out of time to look? |
| 19:31.50 | seanbright | the latter |
| 19:32.14 | seanbright | i'm expected to do actual company related work now... it's unbelievable |
| 20:23.22 | *** join/#asterisk-dev leedm777 (textual@nat/digium/x-exsgysogvbnvhmcg) |
| 20:23.23 | *** mode/#asterisk-dev [+o leedm777] by ChanServ |
| 20:33.24 | *** join/#asterisk-dev jkroon (~jkroon@165.16.204.168) |
| 20:45.50 | *** join/#asterisk-dev leedm777 (textual@nat/digium/x-vyydtuvdfjwssqoh) |
| 20:45.51 | *** mode/#asterisk-dev [+o leedm777] by ChanServ |
| 23:16.25 | gtjoseph | @coreyfarrell: seanbright just fyi... readdir order is deterministic but it's determined by the file system so in practice, it's not. |
| 23:19.12 | coreyfarrell | gtjoseph: cool that's what I figured. I don't think it really matters for the module loader completion, especially once everything is processed from a single opendir. |
| 23:19.40 | gtjoseph | cool |
| 23:22.58 | gtjoseph | coreyfarrell: don't forget to use the thread safe function. :) |
| 23:23.17 | coreyfarrell | what do you mean? |
| 23:24.23 | gtjoseph | readdir isn't thread safe |
| 23:24.26 | gtjoseph | readdir_r is |
| 23:25.21 | gtjoseph | i _think_ we fixed ast_file_read_dirs() to use the thread safe version but i can't check now |
| 23:26.23 | coreyfarrell | gtjoseph: I used ast_file_read_dirs which uses readdir, but it also has a note that readdir is fine on glibc and a global mutex is used for all other platforms. |
| 23:27.56 | gtjoseph | ok |