| 00:24.21 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 00:42.20 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 00:47.47 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-179-40-170.cust.bezeqint.net) |
| 00:47.47 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 00:52.09 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 01:03.10 | coreyfarrell | gtjoseph: I just added you to https://gerrit.asterisk.org/6661 since you wrote most of the vector stuff, I'm interested in your opinion of what to do. |
| 01:03.25 | gtjoseph | are you talking about the jenkins failures? |
| 01:03.54 | coreyfarrell | yes, my change caused test_vector to not compile |
| 01:04.33 | gtjoseph | yeah i saw but didn't have time to dig. i'll take a look in the AM. |
| 01:04.50 | gtjoseph | it was something weird like invalid initializer or something |
| 01:05.05 | gtjoseph | but it looked perfectly acceptable |
| 01:05.41 | coreyfarrell | it doesn't like that 'char CCC2[4];' is being passed a the elem argument.. |
| 01:06.19 | coreyfarrell | I didn't figure you'd look tonight just wanted to point it out to you so you can look whenever you have a chance. res_pjproject is the only thing effected and it only leaks 10k |
| 01:06.41 | gtjoseph | i'll definitely look in the morning. |
| 01:11.38 | *** join/#asterisk-dev kharwell (~kharwell@67.231.202.253) |
| 01:11.38 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 01:18.44 | *** part/#asterisk-dev kharwell (~kharwell@67.231.202.253) |
| 01:19.55 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 01:27.20 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-179-40-170.cust.bezeqint.net) |
| 01:27.20 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 01:38.08 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 02:08.54 | *** join/#asterisk-dev DanQuinney (sid18169@gateway/web/irccloud.com/x-wvaewqetntztknll) |
| 02:08.56 | *** join/#asterisk-dev X-Rob (sid14615@gateway/web/irccloud.com/x-ydkbxhywxerwehvs) |
| 02:24.49 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 02:43.01 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 03:29.41 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 03:51.16 | *** join/#asterisk-dev jkroon (~jkroon@165.16.204.166) |
| 03:57.32 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 04:15.40 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 05:02.26 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 05:20.31 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 05:30.17 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 05:48.11 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 05:58.05 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 06:16.16 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 06:50.13 | *** join/#asterisk-dev CELYA_ (~Thunderbi@82-64-21-217.subs.proxad.net) |
| 07:04.04 | *** join/#asterisk-dev pchero_work (~pchero@109.70.54.56) |
| 07:12.59 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 07:43.24 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 07:48.36 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 09:11.43 | *** join/#asterisk-dev jkroon (~jkroon@uls-154-73-35-201.wall.uls.co.za) |
| 09:16.06 | *** join/#asterisk-dev hajekd (~hajekd@212.24.153.186) |
| 12:24.04 | *** join/#asterisk-dev stefan27 (~stefan27@static-212-247-4-149.cust.tele2.se) |
| 12:41.43 | gtjoseph | coreyfarrell: __auto_type is the answer but although it's supported by gcc and clang, it's too new. |
| 13:46.53 | *** join/#asterisk-dev hajekd (~hajekd@212.24.153.186) |
| 13:54.42 | *** join/#asterisk-dev hajekd (~hajekd@212.24.153.186) |
| 14:04.27 | *** join/#asterisk-dev newtonr (newtonr@nat/digium/x-epdrbdecoidgcpfg) |
| 14:04.41 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 14:36.26 | *** join/#asterisk-dev rmudgett (rmudgett@nat/digium/x-uoqtgnxofgwlynwn) |
| 14:36.26 | *** mode/#asterisk-dev [+o rmudgett] by ChanServ |
| 14:47.52 | coreyfarrell | is res_sdp_translator_pjmedia completely new to Asterisk 15 or is that code in another module in 13/14? |
| 14:48.56 | file | new. |
| 14:49.23 | coreyfarrell | thanks |
| 14:56.34 | coreyfarrell | I spent a bunch of time last night troubleshooting the leaks in one of the pjsip basic calls tests, I believe they're all caused by line 2163 of res_pjsip_session.c in master: 'inv_session->mod_data[session_module.id] = ao2_bump(session);' |
| 14:57.51 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 14:58.29 | coreyfarrell | I have no idea where this should be cleaned, but I suspect this is a pretty serious leak (appears to be in the INVITE path). |
| 15:00.06 | coreyfarrell | I'm about to be on a call for $dayjob so I'll be busy the next hour or so, just wanted to post here so someone who knows pjsip can give it some thought. |
| 15:01.28 | file | I recall when making media sessions go away early, it goes away when the dialog goes away |
| 15:01.33 | file | which is 32 seconds after call termination |
| 15:08.36 | *** join/#asterisk-dev hajekd (~hajekd@81.90.251.91) |
| 15:12.13 | gtjoseph | rmudgett: about the heap... still not seeing the advantage of the heap over a vector using AST_VECTOR_ADD_SORTED given that heap isn't even inlined. each heap push still calls the cmp function plus the need for the non-obvious calc_height thing. why the heap size is expressed as a power of 2 is beyond me but that's neither here nor there. :) |
| 15:21.45 | rmudgett | The heap hight calculation is used to determine the size of the heap so it doesn't have to dynamically grow. |
| 15:28.54 | rmudgett | There really isn't much advantage to using the insertion sort over the heap for small numbers. A priority heap is definitely better for larger numbers. |
| 15:32.02 | gtjoseph | I know what the heap height does and it's not about the sorting algorithm. it's about using heap code that we rarely use, is harder to understand, is not inlined, and is no better performing than the more familiar vector. |
| 15:44.47 | *** join/#asterisk-dev dtascom (~dtascom@cpe-172-248-98-33.socal.res.rr.com) |
| 15:51.59 | rmudgett | Most of the inlining in the code just adds bloat. Inlining is supposed to be used for trivial functions where the overhead of the subroutine stack handling exceeds the execution of the routine. Another place where inlining is beneficial is when the routine is only used once and the compiler can take advantage of optimizations available when the routine is pulled into the calling routine. |
| 15:52.55 | rmudgett | The heap is extensively used by the pbx code. |
| 16:09.34 | coreyfarrell | file: where can I find that dialog termination code? |
| 16:13.54 | file | we receive state changes as a result of it, it's in res_pjsip_session.c |
| 16:14.05 | file | PJSIP is responsible for the dialog termination and destruction and that whole shindig |
| 16:18.10 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-dvsfflpimuvxswxu) |
| 16:18.11 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 16:29.32 | gtjoseph | rmudgett: let me ask a simple question... can you use a vector instead of a heap without losing functionality or performance? |
| 16:30.28 | gtjoseph | if the answer is "no", explain why. |
| 16:31.17 | rmudgett | So you really want me to use a defective AST_VECTOR_ADD_SORTED? |
| 16:31.23 | rmudgett | :) |
| 16:32.23 | gtjoseph | no i want you to use a much more common construct |
| 16:58.22 | *** join/#asterisk-dev jkroon (~jkroon@165.16.204.166) |
| 18:07.09 | coreyfarrell | file: how would you feel about reducing the minimum/default timer_b when REF_DEBUG and TEST_FRAMEWORK are both defined? |
| 18:08.02 | gtjoseph | that's a good idea |
| 18:08.43 | gtjoseph | no one should even be running a prod system with both of those turned on |
| 18:08.56 | coreyfarrell | that alone didn't get the memory to free, I also added sleep(1) to the beginning of res_pjsip_session's module_unload. obviously that would also be only for the REF_DEBUG testsuite. |
| 18:09.18 | gtjoseph | file may be somewhere over ohio right now |
| 18:09.40 | coreyfarrell | ah |
| 18:11.11 | coreyfarrell | so I just ran a basic call test with timer_b default set to 500ms, 0 reference leaks. I did it so if timer_b is set in pjsip.conf it'll still respect that. |
| 18:12.10 | coreyfarrell | going to try it with valgrind now. |
| 18:12.14 | gtjoseph | that's fair. if you make it an even 1s does it still work? |
| 18:13.09 | gtjoseph | trying to think of cases where the ref_debug imposed load on a smoewhat constrained system might make 500ms a little too short |
| 18:13.51 | coreyfarrell | couldn't that cause the 1 second shutdown delay to be inadequate? |
| 18:14.18 | gtjoseph | true but make that 2 sec :) |
| 18:14.57 | gtjoseph | not really important though |
| 18:15.20 | coreyfarrell | I'm thinking that ref_debug / memory testing should be done on more powerful systems.. being on a constrained system doesn't change memory management, so nothing to gain by testing it on a weak system. |
| 18:16.20 | gtjoseph | yeah i'm thinking more of running it in a virtualized environment or docker containers like our jenkins infrastructure. it's be nice to run the ref_debug tests agaion. |
| 18:18.03 | coreyfarrell | ah ok. I'd rather try a smaller delay first, if jenkins has trouble then we can increase it. ref_debug already causes the testsuite to take a ton of extra time. |
| 18:18.13 | gtjoseph | no worries |
| 18:21.34 | coreyfarrell | fyi I just ran the basic call test with valgrind, the only non-suppressed leaks were 2 threads left running from threadpool.c |
| 18:22.00 | gtjoseph | cool |
| 18:30.35 | *** join/#asterisk-dev jkroon (~jkroon@165.16.204.166) |
| 18:32.49 | *** part/#asterisk-dev kharwell (kharwell@nat/digium/x-dvsfflpimuvxswxu) |
| 18:45.56 | *** join/#asterisk-dev tsearle (~tsearle@216.239.164.82) |
| 18:45.57 | *** mode/#asterisk-dev [+o tsearle] by ChanServ |
| 19:17.07 | file | Moo |
| 19:17.16 | file | I was clearing customs and then security at the time |
| 19:20.29 | coreyfarrell | gtjoseph: I stand corrected.. on my laptop 500ms wasn't good enough for valgrind (it only passed once). seems to be working with timerb==1500 and 2 second shutdown delay as long as I don't add --track-origins=yes to the valgrind args. |
| 19:20.46 | gtjoseph | ah |
| 19:24.04 | gtjoseph | file: in toronto? |
| 19:24.45 | file | Yes |
| 19:25.14 | gtjoseph | long layover? what time do you get home? |
| 19:25.41 | file | Flight is delayed by 30 mins, so departs in an hour |
| 19:25.50 | file | Flight is two hours long |
| 19:26.20 | gtjoseph | may the jet stream be with you. |
| 19:26.45 | gtjoseph | my flight from denver to boston last week left denver on time and arrived in boston an hour early. :) |
| 19:27.14 | file | https://usercontent.irccloud-cdn.com/file/PMbSd5kE/irccloudcapture1433206273.jpg |
| 19:27.24 | file | That's not.. Right |
| 19:28.01 | gtjoseph | those paper bags are expensive |
| 19:28.26 | file | I wonder what chedder is |
| 19:28.30 | gtjoseph | it also looks like the original date was oct 05 :) |
| 19:31.13 | file | I didn't buy it |
| 19:31.19 | file | I had a sandwich on my other flight |
| 19:31.41 | gtjoseph | wise choice |
| 19:36.13 | gtjoseph | coreyfarrell: if you get jenkins failures that are obviously not related to the change like the chan_sip failure on the pjsip fake_auth review, no need to regate |
| 19:36.37 | gtjoseph | given that all the pjsip tests passed we'd just verify +2 and submit |
| 19:37.11 | coreyfarrell | ah good to know. |
| 19:38.54 | gtjoseph | the strings uninitialized value one is a little more interesting because the same test failed for both 15 and master but passed for 13 and 14. |
| 19:42.28 | coreyfarrell | and 15 had a segfault, but that seems unrelated, probably something that was not shutdown using something that was shutdown. |
| 19:50.59 | coreyfarrell | gtjoseph: about https://gerrit.asterisk.org/6657, should endpoint->media.topology be recreated when it already exists or left alone? |
| 19:51.21 | gtjoseph | i think if the codecs changed it needs to be recreated |
| 19:51.22 | coreyfarrell | I'm guessing leave it alone since we should get a new endpoint object if the config actually changed? |
| 19:51.40 | gtjoseph | it depends |
| 19:52.10 | coreyfarrell | oh fields set by sorcery aren't immutable? |
| 19:52.11 | gtjoseph | when coming from realtime, apply gets called every time the object has to be re-hydrated |
| 19:52.31 | gtjoseph | they are generally |
| 19:53.20 | gtjoseph | but iirc when i was doing the transport stuff, there were cases when i got the same object if the config file sorcery backend was used. |
| 19:53.34 | gtjoseph | i'd run a test just to be sure |
| 19:53.46 | file | yes, once an object is returned from ast_sorcery_* it's immutable |
| 19:54.08 | file | and you can, and will, get the same object back at times |
| 19:54.22 | gtjoseph | yeah that' |
| 19:54.27 | gtjoseph | s what i remember |
| 19:54.49 | gtjoseph | look at config_transport.c |
| 19:56.02 | gtjoseph | the apply functions special cases |
| 19:56.17 | gtjoseph | s/functions/functions are/ |
| 19:58.02 | coreyfarrell | first thing I'm seeing is maybe pjsip_configuration apply function should maybe lock endpoint? |
| 19:59.30 | file | sorcery objects have no lock as they are immutable |
| 19:59.38 | gtjoseph | probably. the thing is that iirc you can't check to see if the object with that id already exists in sorcery because it'll just call the apply function again. |
| 19:59.42 | file | only additional constructs outside of them have locks, like state things |
| 19:59.49 | gtjoseph | duh yeah |
| 19:59.55 | file | why does apply get called multiple times? |
| 20:00.35 | gtjoseph | i don't remember. it was a while ago and it was related to which backend was in use |
| 20:01.23 | file | okay, that needs to be determined - to know if it's okay or not |
| 20:01.46 | gtjoseph | apply was getting called even if there were no semantic changes to the object. |
| 20:01.58 | coreyfarrell | file: 6fa3ed0679 res_pjsip: improve realtime performance #2 added load_all_endpoints to module startup |
| 20:02.46 | coreyfarrell | that function uses ao2_callback to run the apply function on all endpoints. |
| 20:03.05 | file | okay, I don't think that has to be done |
| 20:03.31 | file | apply automatically gets applied |
| 20:03.43 | file | so it can probably be reduced to retrieve all, then unref the container |
| 20:04.50 | coreyfarrell | let me try that, maybe I don't need to touch the apply function at all. |
| 20:05.43 | file | ooh ready to begin advance boarding |
| 20:05.57 | file | cheers |
| 20:07.46 | *** join/#asterisk-dev hajekd (~hajekd@79.98.154.179) |
| 20:14.10 | gtjoseph | coreyfarrell: if you don't need to run apply twice, then you may still have to check how persistent_state is "indexed". if the endpoint isn't the same object, it may still be the same logical entity (same sorcery id) and the same resources so may resolve to the same persistent endpoint. |
| 20:18.31 | *** join/#asterisk-dev hajekd (~hajekd@79.98.154.179) |
| 20:22.48 | coreyfarrell | gtjoseph: sorry I'm not following |
| 20:23.28 | gtjoseph | i think i said in one of my review comments not bumping the ref on persistent_endpoint or something |
| 20:23.37 | gtjoseph | but you may have to |
| 20:23.50 | gtjoseph | it may be referenced by 2 different endpoint objects |
| 20:23.55 | gtjoseph | with the same id |
| 20:24.07 | gtjoseph | one in sorcery and one not yet in sorcery |
| 20:24.51 | gtjoseph | when the one in sorcery gets unreffed for the final time you don't want it pulling the persistent_endpoint out from under the new endpoint |
| 20:25.11 | gtjoseph | or maybe it's froday afternoon and i'm too tired to keep it straight now |
| 20:25.26 | gtjoseph | in which case, pay no attention to me |
| 20:26.38 | coreyfarrell | I think you're right. I think I'm starting over, I'm going to remove the direct call to apply from the load_all_endpoints (startup) and make it so persistent->aors is reset if needed. |
| 20:26.53 | gtjoseph | kk |
| 20:26.58 | gtjoseph | sorry if i confused things |
| 20:27.04 | gtjoseph | it's time for my nap :) |
| 20:27.25 | coreyfarrell | it's cool, thanks for your help |
| 20:28.40 | gtjoseph | starts some docker build things and heads for the sofa |
| 20:45.18 | *** join/#asterisk-dev NirS (~nirs@67.231.202.253) |
| 20:48.16 | NirS | hello all, anybody home ? |
| 20:57.44 | *** join/#asterisk-dev dtascom (~dtascom@cpe-172-248-98-33.socal.res.rr.com) |
