m_config: add fine-grained option reporting mechanism

This adds m_config_cache_get_next_changed() and the change_flags field
in m_config_cache. Both can be used to determine whether specific
options changed (rather than the entire sub-group).

Not sure if I'm very happy with that. The former rather compact
update_options() is now a bit of a mess, because it needs to be
incremental. m_config_cache_get_next_changed() will not be too nice to
use, and change_flags still relies on global "allocation" of change
flags (see UPDATE_* defines in m_option.h). If C weren't such a
primitive language that smells like grandpa, it would be nice to define
per-option change callbacks or so.

This compares options by value to determine whether they have changed.
This makes it slower in theory, but in practice it probably doesn't
matter (options are rarely changed after initialization). The
alternative would have been per-option change counters (wastes too much
memory; not a practical problem but too ugly), or keep all
m_config_caches in a global list and have bitmaps with per-option change
bits (sounds complicated). I guess the current way is OK.

Technically, this changes semantics slightly by ignoring setting an
option to the same value. Technically this wasn't a no-op, although the
effect was almost almost no-op. Some code would actually become cleaner
by ignoring such redundant change events, and them being no-op is
probably also what the user would normally assume.
This commit is contained in:
wm4 2019-11-28 21:19:41 +01:00
parent 1e6c57d4e4
commit 591494b271
2 changed files with 133 additions and 32 deletions

View File

@ -102,6 +102,9 @@ struct config_cache {
struct m_config_shadow *shadow; // global metadata struct m_config_shadow *shadow; // global metadata
uint64_t ts; // timestamp of this data copy uint64_t ts; // timestamp of this data copy
bool in_list; // part of m_config_shadow->listeners[] bool in_list; // part of m_config_shadow->listeners[]
int upd_group; // for "incremental" change notification
int upd_opt;
// --- Implicitly synchronized by setting/unsetting wakeup_cb. // --- Implicitly synchronized by setting/unsetting wakeup_cb.
struct mp_dispatch_queue *wakeup_dispatch_queue; struct mp_dispatch_queue *wakeup_dispatch_queue;
@ -611,6 +614,19 @@ static void add_sub_group(struct m_config *config, const char *name_prefix,
shadow->groups[group_index].group_count = shadow->num_groups - group_index; shadow->groups[group_index].group_count = shadow->num_groups - group_index;
} }
static uint64_t get_option_change_mask(struct m_config_shadow *shadow,
int group_index, int group_root,
const struct m_option *opt)
{
uint64_t changed = opt->flags & UPDATE_OPTS_MASK;
while (group_index != group_root) {
struct m_config_group *g = &shadow->groups[group_index];
changed |= g->group->change_flags;
group_index = g->parent_group;
}
return changed;
}
struct m_config_option *m_config_get_co_raw(const struct m_config *config, struct m_config_option *m_config_get_co_raw(const struct m_config *config,
struct bstr name) struct bstr name)
{ {
@ -1285,46 +1301,74 @@ struct m_config_cache *m_config_cache_alloc(void *ta_parent,
cache->opts = in->data->gdata[0].udata; cache->opts = in->data->gdata[0].udata;
in->upd_group = -1;
return cache; return cache;
} }
static bool update_options(struct m_config_data *dst, struct m_config_data *src) static void update_next_option(struct m_config_cache *cache, void **p_opt)
{ {
assert(dst->shadow == src->shadow); struct config_cache *in = cache->internal;
struct m_config_data *dst = in->data;
struct m_config_data *src = in->src;
bool res = false; assert(src->group_index == 0); // must be the option root currently
// Must be from same root, but they can have arbitrary overlap. *p_opt = NULL;
int group_s = MPMAX(dst->group_index, src->group_index);
int group_e = MPMIN(dst->group_index + dst->num_gdata, while (in->upd_group < dst->group_index + dst->num_gdata) {
src->group_index + src->num_gdata); struct m_group_data *gsrc = m_config_gdata(src, in->upd_group);
assert(group_s >= 0 && group_e <= dst->shadow->num_groups); struct m_group_data *gdst = m_config_gdata(dst, in->upd_group);
for (int n = group_s; n < group_e; n++) {
struct m_config_group *g = &dst->shadow->groups[n];
const struct m_option *opts = g->group->opts;
struct m_group_data *gsrc = m_config_gdata(src, n);
struct m_group_data *gdst = m_config_gdata(dst, n);
assert(gsrc && gdst); assert(gsrc && gdst);
if (gdst->ts >= gsrc->ts) if (gdst->ts < gsrc->ts) {
continue; struct m_config_group *g = &dst->shadow->groups[in->upd_group];
gdst->ts = gsrc->ts; const struct m_option *opts = g->group->opts;
res = true;
for (int i = 0; opts && opts[i].name; i++) { while (opts && opts[in->upd_opt].name) {
const struct m_option *opt = &opts[i]; const struct m_option *opt = &opts[in->upd_opt];
if (opt->offset >= 0 && opt->type->size) { if (opt->offset >= 0 && opt->type->size) {
m_option_copy(opt, gdst->udata + opt->offset, void *dsrc = gsrc->udata + opt->offset;
gsrc->udata + opt->offset); void *ddst = gdst->udata + opt->offset;
if (!m_option_equal(opt, ddst, dsrc)) {
uint64_t ch = get_option_change_mask(dst->shadow,
in->upd_group, dst->group_index, opt);
if (cache->debug) {
char *vdst = m_option_print(opt, ddst);
char *vsrc = m_option_print(opt, dsrc);
mp_warn(cache->debug, "Option '%s' changed from "
"'%s' to' %s' (flags = 0x%"PRIx64")\n",
opt->name, vdst, vsrc, ch);
talloc_free(vdst);
talloc_free(vsrc);
} }
m_option_copy(opt, ddst, dsrc);
cache->change_flags |= ch;
in->upd_opt++; // skip this next time
*p_opt = ddst;
return;
} }
} }
return res; in->upd_opt++;
} }
bool m_config_cache_update(struct m_config_cache *cache) gdst->ts = gsrc->ts;
}
in->upd_group++;
in->upd_opt = 0;
}
in->upd_group = -1;
}
static bool cache_check_update(struct m_config_cache *cache)
{ {
struct config_cache *in = cache->internal; struct config_cache *in = cache->internal;
struct m_config_shadow *shadow = in->shadow; struct m_config_shadow *shadow = in->shadow;
@ -1336,13 +1380,47 @@ bool m_config_cache_update(struct m_config_cache *cache)
return false; return false;
in->ts = new_ts; in->ts = new_ts;
in->upd_group = in->data->group_index;
in->upd_opt = 0;
return true;
}
bool m_config_cache_update(struct m_config_cache *cache)
{
struct config_cache *in = cache->internal;
struct m_config_shadow *shadow = in->shadow;
if (!cache_check_update(cache))
return false;
pthread_mutex_lock(&shadow->lock); pthread_mutex_lock(&shadow->lock);
bool res = update_options(in->data, in->src); bool res = false;
while (1) {
void *p;
update_next_option(cache, &p);
if (!p)
break;
res = true;
}
pthread_mutex_unlock(&shadow->lock); pthread_mutex_unlock(&shadow->lock);
return res; return res;
} }
bool m_config_cache_get_next_changed(struct m_config_cache *cache, void **opt)
{
struct config_cache *in = cache->internal;
struct m_config_shadow *shadow = in->shadow;
*opt = NULL;
if (!cache_check_update(cache) && in->upd_group < 0)
return false;
pthread_mutex_lock(&shadow->lock);
update_next_option(cache, opt);
pthread_mutex_unlock(&shadow->lock);
return !!*opt;
}
void m_config_notify_change_co(struct m_config *config, void m_config_notify_change_co(struct m_config *config,
struct m_config_option *co) struct m_config_option *co)
{ {
@ -1369,13 +1447,7 @@ void m_config_notify_change_co(struct m_config *config,
pthread_mutex_unlock(&shadow->lock); pthread_mutex_unlock(&shadow->lock);
} }
int changed = co->opt->flags & UPDATE_OPTS_MASK; int changed = get_option_change_mask(shadow, co->group_index, 0, co->opt);
int group_index = co->group_index;
while (group_index >= 0) {
struct m_config_group *g = &shadow->groups[group_index];
changed |= g->group->change_flags;
group_index = g->parent_group;
}
if (config->option_change_callback) { if (config->option_change_callback) {
config->option_change_callback(config->option_change_callback_ctx, co, config->option_change_callback(config->option_change_callback_ctx, co,

View File

@ -266,6 +266,14 @@ struct m_config_cache {
// The struct as indicated by m_config_cache_alloc's group parameter. // The struct as indicated by m_config_cache_alloc's group parameter.
// (Internally the same as internal->gdata[0]->udata.) // (Internally the same as internal->gdata[0]->udata.)
void *opts; void *opts;
// Accumulated change flags. The user can set this to 0 to unset all flags.
// They are set when calling any of the update functions. A flag is only set
// once the new value is visible in ->opts.
uint64_t change_flags;
// Set to non-NULL for logging all option changes as they are retrieved
// with one of the update functions (like m_config_cache_update()).
struct mp_log *debug;
// Do not access. // Do not access.
struct config_cache *internal; struct config_cache *internal;
@ -277,6 +285,9 @@ struct m_config_cache {
// Keep in mind that a m_config_cache object is not thread-safe; it merely // Keep in mind that a m_config_cache object is not thread-safe; it merely
// provides thread-safe access to the global options. All API functions for // provides thread-safe access to the global options. All API functions for
// the same m_config_cache object must synchronized, unless otherwise noted. // the same m_config_cache object must synchronized, unless otherwise noted.
// This does not create an initial change event (m_config_cache_update() will
// return false), but note that a change might be asynchronously signaled at any
// time.
// ta_parent: parent for the returned allocation // ta_parent: parent for the returned allocation
// global: option data source // global: option data source
// group: the option group to return. This can be GLOBAL_CONFIG for the global // group: the option group to return. This can be GLOBAL_CONFIG for the global
@ -307,8 +318,26 @@ void m_config_cache_set_dispatch_change_cb(struct m_config_cache *cache,
// some options have changed). // some options have changed).
// Keep in mind that while the cache->opts pointer does not change, the option // Keep in mind that while the cache->opts pointer does not change, the option
// data itself will (e.g. string options might be reallocated). // data itself will (e.g. string options might be reallocated).
// New change flags are or-ed into cache->change_flags with this call (if you
// use them, you should probably do cache->change_flags=0 before this call).
bool m_config_cache_update(struct m_config_cache *cache); bool m_config_cache_update(struct m_config_cache *cache);
// Check for changes and return fine grained change information.
// Warning: this conflicts with m_config_cache_update(). If you call
// m_config_cache_update(), all options will be marked as "not changed",
// and this function will return false. Also, calling this function and
// then m_config_cache_update() is not supported, and may skip updating
// some fields.
// This returns true as long as there is a changed option, and false if all
// changed options have been returned.
// If multiple options have changed, the new option value is visible only once
// this function has returned the change for it.
// out_ptr: pointer to a void*, which is set to the cache->opts field associated
// with the changed option if the function returns true; set to NULL
// if no option changed.
// returns: *out_ptr!=NULL (true if there was a changed option)
bool m_config_cache_get_next_changed(struct m_config_cache *cache, void **out_ptr);
// Like m_config_cache_alloc(), but return the struct (m_config_cache->opts) // Like m_config_cache_alloc(), but return the struct (m_config_cache->opts)
// directly, with no way to update the config. Basically this returns a copy // directly, with no way to update the config. Basically this returns a copy
// with a snapshot of the current option values. // with a snapshot of the current option values.