1730534 - Create an iterator for JSHolderMap

Status: RESOLVED FIXED

(Core :: XPCOM, task, P3)

Description

[Jon Coppeard (:jonco)]

Most of the gray marking work is marking the JS holders. Currently this uses a ForEach method that takes a lambda which is applied to every entry in the map. Changing this to use a iterator object would allow us mark these incrementally by preserving the iterator state between slices.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Part 1: Assert state of SegmentedVector iterator is valid r=mccr8

The next patches remove elements from a segmented vector that is being
iterated. This patch adds assertions to ensure that we don't attempt to use an
iterator that points to a removed element.

The assertions are added to Done() because all the other methods call that.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Part 2: Add an iterator for JSHolderMap's EntryVector r=mccr8

This replaces use of the ForEach method that takes an EntryVector with an iterator.

Depends on D125426

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Rearrange JSHolderMap class to put the private declarations first r?mccr8

This just rearranges the order of class definition.

Depends on D125427

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Part 3: Add an iterator for JSHolderMap r=mccr8

This adds an iterator over JSHolderMap and replaces use of the ForEach method
with it.

Depends on D125428

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Part 4: Assert that there's only one JSHolderMap::Iter at any time r=mccr8

This iterator can update the map for removed items so it's not safe to have
more than one live at any one time.

Depends on D125429

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83d53ed76e0f
Part 1: Assert state of SegmentedVector iterator is valid r=mccr8
https://hg.mozilla.org/integration/autoland/rev/007cbeea4325
Part 2: Add an iterator for JSHolderMap's EntryVector r=mccr8
https://hg.mozilla.org/integration/autoland/rev/0f72a8c5f8c5
Part 3: Add an iterator for JSHolderMap r=mccr8
https://hg.mozilla.org/integration/autoland/rev/fc881c57d119
Part 4: Assert that there's only one JSHolderMap::Iter at any time r=mccr8

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/83d53ed76e0f
https://hg.mozilla.org/mozilla-central/rev/007cbeea4325
https://hg.mozilla.org/mozilla-central/rev/0f72a8c5f8c5
https://hg.mozilla.org/mozilla-central/rev/fc881c57d119

Comment 8

[Andrew McCreight [:mccr8]]

For part 4, you made them release asserts, but the checks are still behind #ifdef DEBUG, so they're not going to do anything in release builds.

Comment 9

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Fix release mode assertions so they actually happen in release builds r?mccr8

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a445650b1858
Fix release mode assertions so they actually happen in release builds r=mccr8

Comment 11

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/a445650b1858

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/ef8f419b016a
Backed out changeset a445650b1858 for conflicts with backout of Bug 1536061. a=backout

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for cycle collection crashes (bug 1731432) on request from Release Management:
https://hg.mozilla.org/mozilla-central/rev/29d6504debf5b2028a9da65026ae86a06b5a655d

Comment 14

[Jon Coppeard (:jonco)]

Attachment: Bug 1730534 - Part 5: Add some basic unit tests for JSHolderMap r?mccr8

Depends on D125431

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e194c3e1470c
Part 1: Assert state of SegmentedVector iterator is valid r=mccr8
https://hg.mozilla.org/integration/autoland/rev/bca9d2d15111
Part 2: Add an iterator for JSHolderMap's EntryVector r=mccr8
https://hg.mozilla.org/integration/autoland/rev/4ec8f15b2546
Part 3: Add an iterator for JSHolderMap r=mccr8
https://hg.mozilla.org/integration/autoland/rev/af7b55df6c6b
Part 4: Assert that there's only one JSHolderMap::Iter at any time r=mccr8
https://hg.mozilla.org/integration/autoland/rev/cd825de5206c
Part 5: Add some basic unit tests for JSHolderMap r=mccr8

Comment 16

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/e194c3e1470c
https://hg.mozilla.org/mozilla-central/rev/bca9d2d15111
https://hg.mozilla.org/mozilla-central/rev/4ec8f15b2546
https://hg.mozilla.org/mozilla-central/rev/af7b55df6c6b
https://hg.mozilla.org/mozilla-central/rev/cd825de5206c