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

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Justin Lebar (not reading bugmail)]

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).

Comment 1

[Justin Lebar (not reading bugmail)]

> 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.

Comment 2

[Nicholas Nethercote [inactive]]

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...

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 4

[Justin Lebar (not reading bugmail)]

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.

Comment 5

[Justin Lebar (not reading bugmail)]

(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.)

Comment 6

[Mike Hommey [:glandium]]

(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.

Comment 7

[Justin Lebar (not reading bugmail)]

> 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.

Comment 8

[Mike Hommey [:glandium]]

(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.

Comment 9

[Justin Lebar (not reading bugmail)]

> 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.  :)

Comment 10

[Paul Bone [:pbone]]

I think this is INVALID since Bug 1567366