[Gc] Workaround for problems with GetWriteWatch and fix for
GC_get_heap_size/bytes_free values on unmapping
hans.boehm at hp.com
Sat Aug 1 11:49:13 PDT 2009
> From: Ivan Maidanski
> 1. MPROTECT_VDB only. As in some cases on Win32 the
> exception-based approach of incremental collection may work
> better than GetWriteWatch-based one then there should be a
> way to select the one. So, GC_PREFER_MPROTECT_VDB macro and
> GC_USE_GETWRITEWATCH environment variable are introduced.
This part looks good to me.
> 2. The values returned by GC_get_heap_size() and
> GC_get_free_bytes() if USE_MUNMAP. If the memory is unmapped
> (returned to the OS) then the values indicating the current
> heap size and the free memory size should be updated
> accordingly (otherwise, the total heap size of several
> applications running on a system may become larger than the
> system virtual memory size, and the memory considered as
> "free" by one application may be, in fact, in use by another one).
This worries me a bit, though I don't immediately see anything that would break. The problem is that both GC_large_free_bytes and GC_heapsize now mean somewhat different things depending on whether we're unmapping memory. Up until now, unmapping memory was only an optimization that didn't much affect the rest of the collector logic. Unmapped memory was still considered to be part of the large object free list, and part of the heap. Since these variables are used in some performance heusristics, we now have to convince ourselves that they make sense in both scnearios.
If the underlying problem here is the values we're reporting back to the user, does it make more sense to just keep track of the total amount of unmapped memory in a separate variable, and subtract it in GC_get_heap_size and GC_get_free_bytes, leaving the internals alone? That strikes me as safer and easier to maintain. Plus we have more information for debugging.
> The patch is, in fact, resembling diff52, diff75, diff83
> partly. The patch also fixes some typos and update the
> comments (in the same files).
> ChangeLog entries:
> * include/private/gcconfig.h
> (GC_PREFER_MPROTECT_VDB): New macro
> recognized (only if MPROTECT_VDB).
> * os_dep.c (detect_GetWriteWatch): Recognize
> environment variable (only if MPROTECT_VDB, if the variable is
> unset when GC_PREFER_MPROTECT_VDB macro controls the
> * doc/README.environment (GC_USE_GETWRITEWATCH): New variable.
> * Makefile.direct (DONT_USE_USER32_DLL,
> Add the comments for.
> * Makefile.direct (MARK_BIT_PER_OBJ, PRINT_BLACK_LIST,
> USE_PROC_FOR_LIBRARIES): Fix typo in the comments.
> * Makefile.direct (USE_MMAP, USE_MUNMAP, THREAD_LOCAL_ALLOC,
> PARALLEL_MARK, STATIC): Update the comments.
> * include/private/gcconfig.h (MPROTECT_VDB): Add FIXME for
> USE_MUNMAP and PARALLEL_MARK cases (to relax the conditions in
> the future).
> * os_dep.c (detect_GetWriteWatch): Use multi-byte (A)
> variant of
> Win32 GetModuleFileName().
> * os_dep.c (GC_unmap, GC_unmap_gap): Decrease GC_heapsize and
> GC_large_free_bytes values appropriately on heap
> shrunk (the memory
> is returned to the OS).
> * os_dep.c (GC_remap): Increase GC_heapsize and
> values appropriately when memory is mapped again.
> * os_dep.c (GC_dirty_init): Move "ifdef GWW_VDB" block out of
> "ifdef MSWIN32" one (for Cygwin).
> PS. I've also fixed issues with data races in the public
> getters/setters (mostly in the same as I'd described in one
> of my previous posts) but the patch is not ready yet.
More information about the Gc