Remove in-place mutation of trees' expanded sets in reducers

RESOLVED FIXED in Firefox 48

Status

P2
normal
RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: fitzgen, Assigned: linclark)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

We might be able to just get away with replacing the Set with a flat array and doing the O(n) lookup time. The expanded sets are probably pretty small, all things considered.

We could also generate a fresh Set for each state reduction. Still O(n), but then we aren't doing an additional O(n) search for each lookup.

Finally, we could either implement or grab an existing persistent immutable set.
Has STR: --- → irrelevant
Duplicate of this bug: 1257253
Assignee: nobody → lclark
(Assignee)

Comment 2

3 years ago
Created attachment 8731399 [details] [diff] [review]
Bug1239436-immutable-set.patch --- Depends on 1253784

This patch replaces the mutable sets with Immutable.js sets. You'll need to apply the patch in 1253784 first.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f87d7721084e
Attachment #8731399 - Flags: review?(nfitzgerald)
(Assignee)

Updated

3 years ago
Depends on: 1253784
(Assignee)

Updated

3 years ago
See Also: → bug 1253376
Comment on attachment 8731399 [details] [diff] [review]
Bug1239436-immutable-set.patch --- Depends on 1253784

Review of attachment 8731399 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent!!!

Thank you, Lin!
Attachment #8731399 - Flags: review?(nfitzgerald) → review+
(Assignee)

Comment 4

3 years ago
You're welcome :)
Keywords: checkin-needed
(Assignee)

Comment 5

3 years ago
Shoot, forgot blocker
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e2280b84527f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.