On Mon, Mar 19, 2018 at 7:00 AM, John Spray <jspray@xxxxxxxxxx> wrote: > On Sun, Mar 18, 2018 at 4:44 AM, Radoslaw Zarzynski <rzarzyns@xxxxxxxxxx> wrote: >> Hello, >> >> some time ago I've promised to Greg a post highlighting performance >> issue that was discovered couple weeks ago: many calls to the costly >> `md_config_t::get_val<T>` on OSD's hot paths. >> >> The problem is that the method really needs to acquire a mutex and >> isn't supposed to be used in performance-critical situations. >> For handling them the config rework designated the `md_config_obs_t` >> interface. Implementing it allows clients to locally cache setting's value >> giving very fast access. >> >> Though, it's understandable that one, single call is much easier to use >> as it doesn't require any boilerplate. Because of that a grain of sugar >> was added: `md_config_cacher_t` [1]. It handles registration/value >> update/unregistration simplifying the observer usage in typical cases. >> >> The idea [2] is to make `md_config_cacher_t<SettingT>` a member >> of a longly living instance (e.g. shard), then initialize it with a name of >> interesting configurable (+ a reference to md_config_t). Obtaining >> the cached value is simplified thanks to the implicit conversion to >> `SettingT`. >> >> At the moment there is a limitation on `SettingT`: it can't be std::string >> due to the std::atomic<> config cacher uses inside. >> >> Surely, there is no business in converting everything to use the config >> observer pattern. `get_val<T>` is perfectly fine during initialization, >> error handling and generally on all infrequent paths. Only the hottest >> ones might be an issue. > > Excellent! This is a very useful compliment to the new-style config stuff. I've just looked up the difference between compliment and complement, and I meant the latter :-) John > > John > >> Best regards. >> Radosław Zarzyński >> >> >> `md_config_cacher_t` implementation: >> [1] https://github.com/ceph/ceph/blob/master/src/common/config_cacher.h >> >> Example of usage: >> [2] https://github.com/ceph/ceph/pull/20320/commits/248750fea441efd16c1a431617cfbd1f868a6eeb >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
