----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4193/#review13836 -----------------------------------------------------------
/team/mjordan/12-threadpool/main/stasis.c <https://reviewboard.asterisk.org/r/4193/#comment24305> I think this should be configurable. - Joshua Colp On Nov. 18, 2014, 6:40 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4193/ > ----------------------------------------------------------- > > (Updated Nov. 18, 2014, 6:40 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24533 > https://issues.asterisk.org/jira/browse/ASTERISK-24533 > > > Repository: Asterisk > > > Description > ------- > > Note: don't be fooled by the size of this review. Most of the changes are > small and are in stasis.c/stasis_message_router.c. > > Rob (and CDR on the asterisk-users list, although he never followed up when > we asked for more information) discovered that we were creating two > additional threads per SIP peer in chan_sip. This actually would occur for a > large variety of 'endpoints' in Asterisk, regardless of the channel driver. > The two threads were stasis subscriptions, specifically for MWI and for > endpoints detecting the destruction of related channels. > > Stasis subscriptions currently get a dedicated thread. In contrast, prior to > r400178 (see review https://reviewboard.asterisk.org/r/2881/), the > subscriptions shared a thread pool. It was discovered that for a low > subscription count with high message throughput, the threadpool was not as > performant as simply having a dedicated thread per subscriber. > > Since then, our subscription pattern has changed slightly. While we still > have plenty of subscriptions that would follow the model just described (AMI, > CDRs, CEL, etc.) - there are plenty that also fall into the following two > categories: > * Large number of subscriptions, specifically those tied to endpoints/peers. > * Low number of messages. Some subscriptions exist specifically to coordinate > a single message - the subscription is created, a message is published, the > delivery is synchronized, and the subscription is destroyed. > In both of the latter two cases, creating a dedicated thread is wasteful (and > in the case of a large number of peers/endpoints, harmful). > > This patch adds the ability of a subscriber to Stasis to choose whether or > not their messages are dispatched on a dedicated thread or on a threadpool. > The threadpool is currently hard coded in this patch; if configuration is > necessary, we could re-add the configuration options in r2881. Pre-mature > optimization and all that led me to simply go with a simpler model for now. > > Note that the approach taken here was to add additional API calls rather than > modify existing ones. If we'd rather the thread dispatch model be dictated by > a parameter, the new API calls can be removed in trunk and the old API calls > modified appropriately. > > > Diffs > ----- > > /team/mjordan/12-threadpool/tests/test_stasis.c 428167 > /team/mjordan/12-threadpool/res/res_xmpp.c 428167 > /team/mjordan/12-threadpool/res/res_stasis_device_state.c 428167 > /team/mjordan/12-threadpool/res/res_pjsip_refer.c 428167 > /team/mjordan/12-threadpool/res/res_pjsip_pubsub.c 428167 > /team/mjordan/12-threadpool/res/res_pjsip_mwi.c 428167 > /team/mjordan/12-threadpool/res/res_jabber.c 428167 > /team/mjordan/12-threadpool/res/parking/parking_bridge_features.c 428167 > /team/mjordan/12-threadpool/res/parking/parking_applications.c 428167 > /team/mjordan/12-threadpool/main/stasis_message_router.c 428167 > /team/mjordan/12-threadpool/main/stasis_channels.c 428167 > /team/mjordan/12-threadpool/main/stasis_cache.c 428167 > /team/mjordan/12-threadpool/main/stasis.c 428167 > /team/mjordan/12-threadpool/main/endpoints.c 428167 > /team/mjordan/12-threadpool/include/asterisk/stasis_message_router.h 428167 > /team/mjordan/12-threadpool/include/asterisk/stasis_internal.h 428167 > /team/mjordan/12-threadpool/include/asterisk/stasis.h 428167 > /team/mjordan/12-threadpool/channels/sig_pri.c 428167 > /team/mjordan/12-threadpool/channels/chan_skinny.c 428167 > /team/mjordan/12-threadpool/channels/chan_sip.c 428167 > /team/mjordan/12-threadpool/channels/chan_mgcp.c 428167 > /team/mjordan/12-threadpool/channels/chan_iax2.c 428167 > /team/mjordan/12-threadpool/channels/chan_dahdi.c 428167 > > Diff: https://reviewboard.asterisk.org/r/4193/diff/ > > > Testing > ------- > > New unit tests were written to cover the new subscription types. This tests > receiving messages sent using the threadpool. > > The MWI tests and channel driver tests in the Test Suite were run and passed. > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- 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
