linebender/druid

Rewrite EnvScope

Open

#1386 opened on Nov 9, 2020

View on GitHub
 (1 comment) (0 reactions) (0 assignees)Rust (9,091 stars) (567 forks)batch import
enhancementhelp wantedwidget

Description

The current implementation of EnvScope has fundamental performance problems and is not fit for purpose.

I would like to propose a two-part solution: first, we more eficiently accomodate the common case, where the items in the Env do not change, and secondly that we do more caching in order to decrease the frequency with which we need to construct a new Env.

First part:

Most of the time, EnvScope is used to override some static set of attributes; there is no special logic. We should accomodate this directly, by allowing the user to specify just some set of keys/values; and we will actually construct a new Env only if the incoming Env differs from the previous incoming Env.

This would look something like this; anytime you needed to get the env for your child, you would call child_env, and it would rebuild only if necessary.

struct EnvScope2<T> {
    prev_super_env: Option<Env>,
    current_child_env: Option<Env>,
    overrides: Env,
    // also a child widget etc here
}

impl EnvScope2 {
    /// The `overrides` is an env containing only keys/values that should be overwritten at this scope.
    pub fn new(overrides: Env, child: impl Widget<T>) -> Self {
        // ...
    }

    fn child_env(&mut self, super_env: &Env) -> &Env {
        let super_same = self.pre_super_env.as_ref().map(|old| old.same(super_env)).unwrap_or(false);
        if !super_same {
            self.current_child_env = Some(super_env.with_overrides(&self.overrides));
            self.prev_super_env = Some(super_env.to_owned());
        }
        self.current_child_env.as_ref().unwrap()
    }
}

Second part:

the second part is similar, and equally simple: If you really need to dynamically change the Env, you can still do that with a closure, but you need a second closure that is used to determine whether or not the change needs to happen right now. This would be called in update, and would return a bool, and we would only actually rebuild the env if this was true, using logic similar to the logic above.

Contributor guide