1396723 - Use DoublyLinkedList in mozjemalloc

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1396723 - Simplify the trait users of DoublyLinkedList need to define.

While the flexibility of the current trait is nice, it's actually not
used to its fullest anywhere, and is boilerplate-y. While it is useful
to be able to put the links anywhere, there's not much usefulness from
being able to split mNext and mPrev.

So instead of a trait that allows to get/set mNext and mPrev
independently, we just use a trait that tells how to get a reference to
a DoublyLinkedListElement from a list element itself.

Review commit: https://reviewboard.mozilla.org/r/176278/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176278/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1396723 - Use DoublyLinkedList in mozjemalloc.

Mozjemalloc uses its own doubly linked list, which, being inherited from
C code, doesn't do much type checking, and, in practice, is rather
similar to DoublyLinkedList, so use the latter instead.

Review commit: https://reviewboard.mozilla.org/r/176280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176280/

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8904433 [details]
Bug 1396723 - Simplify the trait users of DoublyLinkedList need to define.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176278/diff/1-2/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8904434 [details]
Bug 1396723 - Use DoublyLinkedList in mozjemalloc.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176280/diff/1-2/

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8904433 [details]
Bug 1396723 - Simplify the trait users of DoublyLinkedList need to define.

https://reviewboard.mozilla.org/r/176278/#review181912

Makes sense to me.

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8904434 [details]
Bug 1396723 - Use DoublyLinkedList in mozjemalloc.

https://reviewboard.mozilla.org/r/176280/#review181914

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ca94a450b81
Simplify the trait users of DoublyLinkedList need to define. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/1c0f9d069ade
Use DoublyLinkedList in mozjemalloc. r=froydnj

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=129049166&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/bc13c43f67712786325bdea324246bea41a0b076

Comment 9

[Mike Hommey [:glandium]]

Yay a GCC bug that I didn't hit locally because I'm using gcc 7. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8904433 [details]
Bug 1396723 - Simplify the trait users of DoublyLinkedList need to define.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176278/diff/2-3/

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8904434 [details]
Bug 1396723 - Use DoublyLinkedList in mozjemalloc.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176280/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b744eba9ca78
Simplify the trait users of DoublyLinkedList need to define. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/4f17343164b6
Use DoublyLinkedList in mozjemalloc. r=froydnj

Comment 13

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/c02d7df64e12 for frequent "Assertion failure: !ElementAccess::Get(mHead).mPrev, at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/DoublyLinkedList.h:211" like https://treeherder.mozilla.org/logviewer.html#?job_id=129067118&repo=autoland and https://treeherder.mozilla.org/logviewer.html#?job_id=129088002&repo=autoland and https://treeherder.mozilla.org/logviewer.html#?job_id=129103041&repo=autoland, apparently all OS X.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8904433 [details]
Bug 1396723 - Simplify the trait users of DoublyLinkedList need to define.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176278/diff/3-4/

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8904434 [details]
Bug 1396723 - Use DoublyLinkedList in mozjemalloc.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176280/diff/3-4/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45c68c943e97
Simplify the trait users of DoublyLinkedList need to define. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/c35654690032
Use DoublyLinkedList in mozjemalloc. r=froydnj

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/45c68c943e97
https://hg.mozilla.org/mozilla-central/rev/c35654690032