Split registered_mutation_observers in two

[ferjm]

#22743 introduces a NodeRareData struct to store ... well... rare data. Ideally, this data should only be lazily initialized by setters, but the existing mutation observers getter is also initializing it, and that breaks the sole purpose of this struct.

[gatoWololo]

Hello! I'm currently looking at fixing this issue.

I think I have a sense of what's happening. The problem is:

    /// Return all registered mutation observers for this node.
    /// XXX(ferjm) This should probably be split into two functions,
    /// `registered_mutation_observers`, which returns an Option or
    /// an empty slice or something, and doesn't create the rare data,
    /// and `registered_mutation_observers_mut`, which does lazily
    /// initialize the raredata.
    pub fn registered_mutation_observers(&self) -> RefMut<Vec<RegisteredObserver>> {
        RefMut::map(self.ensure_rare_data(), |rare_data| {
            &mut rare_data.mutation_observers
        })
    }

As the comment helpfully points out, this is a getter that initializes rare_data via ensure_rare_data():

        fn ensure_rare_data(&self) -> RefMut<Box<$type>> {
            let mut rare_data = self.rare_data.borrow_mut();
            if rare_data.is_none() {
                *rare_data = Some(Default::default());
            }
            RefMut::map(rare_data, |rare_data| {
                rare_data.as_mut().unwrap()
            })
        }

Where if the data is not there, we just init with default(). I'm curious what the motivation behind lazy initialization is?

Looking at NodeRareData:

#[derive(Default, JSTraceable, MallocSizeOf)]
#[must_root]
pub struct NodeRareData {
    /// The shadow root the node belongs to.
    /// This is None if the node is not in a shadow tree or
    /// if it is a ShadowRoot.
    pub containing_shadow_root: Option<Dom<ShadowRoot>>,
    /// Registered observers for this node.
    pub mutation_observers: Vec<RegisteredObserver>,
}

Default just inits containing_shadow_root to None, and mutation_observers to empty vector. So it should be cheap?

Splitting registered_mutation_observers() into two methods seems fairly straightforward. Or am I missing something?

[jdm]

ensure_rare_data returns a Box value, which is an unconditional allocation (unlike an empty Vec, for example). The goal here is to add a new registered_mutation_observers variant that returns an Option and does not initialize the rare data if it does not already exist, and update callers of registered_mutation_observers that do not mutate the result to use this new method instead.

[gatoWololo]

I see, thanks. I'll try working on this then.