1947687 - MayPurgeStep takes the lock a second time after purging an arena.

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Description

[Paul Bone [:pbone]]

moz_may_purge_one_now takes the lock again to "peek" to determine if there are more purge requests queued. It's better to return true if there "may be" more requests and let the caller call Purge() again if it wants to.

Comment 1

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - Don't peek at the list after processing an arena r=glandium

Comment 2

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 4. Simplify some code with a do-while loop r=jstutte

Processing purge requests won't fail if there are none, so there's no point
peeking before attempting to process any.

Comment 3

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 8. Perform multiple purge steps without checking the list r=glandium

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - Move the delayed purge API to mozmemory_utils.h r=glandium

Comment 5

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 1. Correct some comments r=glandium

Comment 6

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 2. found is already known to be true here r=glandium

Comment 7

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 3. Remove the arena from the list while we still hold the lock r=jstutte

This avoids releasing the lock only to acquire it a few lines later.

Comment 9

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 7. Move the purge API between sections of malloc_decls.h r=glandium

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/589e781f35c2
pt 1. Correct some comments r=glandium
https://hg.mozilla.org/integration/autoland/rev/e128af5f021f
pt 2. found is already known to be true here r=glandium
https://hg.mozilla.org/integration/autoland/rev/0f2a907ba40a
pt 3. Remove the arena from the list while we still hold the lock r=jstutte
https://hg.mozilla.org/integration/autoland/rev/c13b82d1c924
pt 4. Simplify some code with a do-while loop r=jstutte
https://hg.mozilla.org/integration/autoland/rev/816244cab3f5
pt 5. Move the purge API between sections of malloc_decls.h r=glandium
https://hg.mozilla.org/integration/autoland/rev/9012bfccb2cd
pt 6. Perform multiple purge steps without checking the list r=glandium

Comment 11

[Atila Butkovits]

Backed out for causing SM bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/2375604263792c9a7a48244f49828e1741eaed2a

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=9012bfccb2cd91d14c27e7c8bf059bd0b7bf1514&selectedTaskRun=Y8fzayILT3aDB5UbFT0iEA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=497793976&repo=autoland&lineNumber=5748

Comment 12

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 5. Move the dirty page modifier code into #ifdefs r=nical

The dirty page functions only make sense with MOZ_MEMORY, without it they're
no-ops. A later patch will move them to a more appropriate section of
malloc_decls.h which will make the undefined without MOZ_MEMORY. Therefore
this patch ifdef's out code that uses them if MOZ_MEMORY isn't defined.

The patch also makes these file-local functions static.

Comment 13

[Paul Bone [:pbone]]

Attachment: Bug 1947687 - pt 6. ifdef out use of moz_set_max_dirty_page_modifier() r=smaug

Another patch will move moz_set_max_dirty_page_modifier() into a different
section of malloc_decls.h where it will be unavailable without defining
MOZ_MEMORY. This patch places #ifdefs around some of its uses.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aaf41eb3d828
pt 1. Correct some comments r=glandium
https://hg.mozilla.org/integration/autoland/rev/f320b975ef05
pt 2. found is already known to be true here r=glandium
https://hg.mozilla.org/integration/autoland/rev/791361c24764
pt 3. Remove the arena from the list while we still hold the lock r=jstutte
https://hg.mozilla.org/integration/autoland/rev/b2dec082e21a
pt 4. Simplify some code with a do-while loop r=jstutte
https://hg.mozilla.org/integration/autoland/rev/31e229ff65b0
pt 5. Move the dirty page modifier code into #ifdefs r=nical
https://hg.mozilla.org/integration/autoland/rev/dd640a024c06
pt 6. ifdef out use of moz_set_max_dirty_page_modifier() r=smaug
https://hg.mozilla.org/integration/autoland/rev/de6152e95629
pt 7. Move the purge API between sections of malloc_decls.h r=glandium
https://hg.mozilla.org/integration/autoland/rev/e35dca5c1c1e
pt 8. Perform multiple purge steps without checking the list r=glandium

Comment 15

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/aaf41eb3d828
https://hg.mozilla.org/mozilla-central/rev/f320b975ef05
https://hg.mozilla.org/mozilla-central/rev/791361c24764
https://hg.mozilla.org/mozilla-central/rev/b2dec082e21a
https://hg.mozilla.org/mozilla-central/rev/31e229ff65b0
https://hg.mozilla.org/mozilla-central/rev/dd640a024c06
https://hg.mozilla.org/mozilla-central/rev/de6152e95629
https://hg.mozilla.org/mozilla-central/rev/e35dca5c1c1e