1147734 - Clear out duplicate attribute mutations more aggressively

Status: NEW

(DevTools :: Inspector, defect, P3)

Description

[Brian Grinstead [:bgrins]]

Even with the changes from Bug 1145914, there are still cases where the pendingMutations list could get very large.

Since we don't prune out duplicates until the call to getMutations() if there were a case where many mutations queue up before a request happens:

    for (var i = 0; i < 10000; i++) {
      document.body.setAttribute("data-toofast", toofast++);
    }

The list of pendingMutations is length 10000 until the duplicates are removed.

There may be a smart way to keep track of duplicates and remove them from the list before adding them in queueMutation.  Although keeping tabs on the set of mutations could become more trouble than it's worth.  We could also just prune out duplicates every N insertions.

Comment 1

[Brian Grinstead [:bgrins]]

I thought of a few options:

1) Simply check if the last pending mutation is a duplicate.  If so, just overwrite it with this one instead of pushing this one.  (simple, but only handles the only case in the example above where a single attribute/target is being modified in a loop)
2) Keep a WeakMap of (mutation.target + "-" + mutation.attributeName, index) that indexes each attribute in pendingMutations.  Upon insertion if a duplicate is encountered, just null it out (instead of splicing it from array).  Then filter out nulls before sending them along in getMutations.
3) Keep all of the mutations in some kind of Map keyed on target and ensures that no duplicates are added.  Have an incrementor attach a 'sort' property onto each mutation, and then sort based on that before sending them along in getMutations.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: subsequent-duplicate-attributes-WIP.patch

This is approach 1 from comment 1.  It's a WIP that still contains some logging to demonstrate the what case it's helping in.  If the commented out line in test_inspector-mutations-attr.html is removed, then this doesn't provide any help, but if it's kept commented out then pendingMutations ends up with length ~ 3 instead of length ~ 1000.

Not sure if it's worth doing something like this since it helps only a specific case, we may want to come up with a solution that limits memory usage more generally.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: null-out-duplicates.patch

Approach 2.  I like that this works well even when multiple attributes are changed in sync.  I also like combining m.target + "-" + m.attributeName so it can be keyed easier (I didn't think of that for 1145914, but it only gets rid of a couple of boilerplate lines, so I don't think it's worth changing that patch before it lands).

Comment 4

[Julian Descottes [:jdescottes]]

Inspector bugs triage. Filter on CLIMBING SHOES.
Falls under refactoring/nice to have -> P3.