Closed Bug 752342 Opened 12 years ago Closed 2 years ago

JS GC chunk allocator needs an equivalent of jemalloc's DOUBLE_PURGE (definitely Mac, maybe Windows too)

Categories

(Core :: JavaScript Engine, defect)

14 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: justin.lebar+bug, Unassigned)

Details

(Whiteboard: [js:t][MemShrink:P2])

In jemalloc, before we read the process's RSS (for telemetry or about:memory), we go through and actually munmap all pages which have been MADV_FREE'd.  This is necessary for our RSS to match reality, because MADV_DONTNEED/MADV_FREE'd memory is counted against RSS.

This is important for our measurements (telemetry, etc).
> because MADV_DONTNEED/MADV_FREE'd memory is counted against RSS.

On Mac, but not on Linux.  I think MEM_RESET'ed memory may be counted against RSS on Windows; we should check.
Terrence, do you have time to implement this?  It doesn't sound like it would be too hard, and it's important that our "resident" measurements are accurate...
Whiteboard: [MemShrink] → [MemShrink:P1]
This seems like a bad idea. All GC memory needs to be allocated in aligned 1MB chunks. If we released some of this memory, it might be reallocated to someone else (like jemalloc). Then if we needed the page back again, we would have to allocate a new 1MB chunk.
Right.  I think we don't actually need to munmap / mmap the pages but can instead reset the pages (mmap_fixed?).

Which reminds me, I do munmap / mmap in jemalloc.  Which is surely very bad for the reason you point out.
(To be clear, we want something which is like an atomic |munmap mmap| pair.  Certainly the suggestion isn't to unmap and then try to map at some point later.)
(In reply to Justin Lebar [:jlebar] from comment #1)
> > because MADV_DONTNEED/MADV_FREE'd memory is counted against RSS.
> 
> On Mac, but not on Linux.  I think MEM_RESET'ed memory may be counted
> against RSS on Windows; we should check.

MEM_RESET'ed memory is counted in RSS on Windows, which is why we use MEM_DECOMMIT/MEM_COMMIT on windows. As long as your allocation scheme prevents that memory from being used between MEM_DECOMMIT and MEM_COMMIT, it's safe.

For Linux/mac mmap(MAP_FIXED) would work.
> MEM_RESET'ed memory is counted in RSS on Windows, which is why we use MEM_DECOMMIT/MEM_COMMIT on 
> windows.

Sounds like we could (safely!) implement double-purge on Windows with a VirtualFree(MEM_DECOMMIT) / VirtualAlloc(MEM_COMMIT) pair.  That's thread-safe because we're not freeing the virtual memory, just the physical memory that backs it.
(In reply to Justin Lebar [:jlebar] from comment #7)
> Sounds like we could (safely!) implement double-purge on Windows with a
> VirtualFree(MEM_DECOMMIT) / VirtualAlloc(MEM_COMMIT) pair.  That's
> thread-safe because we're not freeing the virtual memory, just the physical
> memory that backs it.

I was under the impression that you'd get an exception between MEM_DECOMMIT and MEM_COMMIT if something accesses it. But maybe I got a false impression.
> I was under the impression that you'd get an exception between MEM_DECOMMIT and MEM_COMMIT if 
> something accesses it. But maybe I got a false impression.

I think you would get an exception, yes, but in the case of GC chunks, we should be able to guarantee that nobody is accessing the chunk while we're frobbing it.

(Contrast this with the current double-purge implementation in jemalloc, where someone could swoop in between the munmap and mmap and steal our virtual memory!)

For those following along at home, this conversation is basically split between here and bug 752339.  :)
Whiteboard: [MemShrink:P1] → [js:t][MemShrink:P1]
Whiteboard: [js:t][MemShrink:P1] → [js:t][MemShrink:P2]
Assignee: general → nobody

I think this is INVALID since Bug 1567366

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.