[Asterisk-code-review] res pjsip: Cache global config options. (asterisk[14])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: res_pjsip: Cache global config options.
......................................................................


Patch Set 1:

> > > I'm not a fan of this change. Specifically, this will have an
 > > > impact on those that store global configs in realtime. They now
 > > > would need to issue a reload of res_pjsip in order to get
 > global
 > > > configuration changes put in.
 > > >
 > > > I think a better change would be to alter various res_sorcery_*
 > > > modules to optimize for the case where all objects are
 > requested.
 > > > res_sorcery_config, for instance, could basically do what
 > you're
 > > > doing here. If you make that change, then *all* sorcery objects
 > > get
 > > > the benefit presented here. Since sorcery objects are
 > immutable,
 > > > you wouldn't be introducing anything untoward by doing that
 > > either.
 > > > You'd just have to ensure that the containers returned by
 > sorcery
 > > > queries are also treated as immutable (I'm pretty sure they
 > are).
 > >
 > > I agree in principle but globals don't change that often that
 > doing
 > > a reload would be a hardship.  Many realtimers leave globals in
 > > pjsip.conf anyway.
 > 
 > I concede that you are likely correct on both points. My point
 > still stands that changing behavior at a lower level gives you the
 > optimization for all sorcery object types, not just globals.
 > 
 > I suppose I'm fine with this change going in as long as you also
 > update CHANGES to indicate that realtime global settings now
 > require a res_pjsip.so reload to take effect.

I hadn't considered realtime when creating this patch.  However, realtime is more expensive than a config file back-end even if you ignore the database lookup time.  With a full memory cache it looks about the same as a config file back-end.  Still accessing the same sorcery object hundreds of times a second is itself expensive.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ice16c7a4cbca4614da344aaea21a072b86263ef7
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No