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

[Mark Michelson]

Mark Michelson 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.

-- 
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-HasComments: No