incremental GC: fix phantom-byte counting

pkgs/racket-test-core/tests/racket/phantom-bytes.rkt

@@ -0,0 +1,54 @@
+#lang racket/base
+
+;; An extra test of phantom bytes.
+
+(define (make-one)
+  (make-phantom-bytes (expt 2 29)))
+
+(define pbs (list (make-one)))
+
+(define (check)
+  (unless (> (current-memory-use) (* (length pbs)
+                                     (expt 2 29)))
+    (error "failed"))
+  (for ([pb (in-list pbs)])
+    (set-phantom-bytes! pb 0))
+  (unless (< (current-memory-use) (expt 2 29))
+    (error "failed after zeros:" (current-memory-use)))
+  (for ([pb (in-list pbs)])
+    (set-phantom-bytes! pb (expt 2 29)))
+  (unless (> (current-memory-use) (* (length pbs)
+                                     (expt 2 29)))
+    (error "failed after restore:" (current-memory-use))))
+
+(check)
+(collect-garbage)
+(check)
+
+(define mem (make-bytes (* 250 1024 1024)))
+(check)
+(collect-garbage)
+(check)
+(set! pbs (cons (make-one) pbs))
+(check)
+(collect-garbage)
+(check)
+(collect-garbage)
+(check)
+
+(void (bytes-length mem))
+
+'ok
+
+(module test racket/base
+  (require compiler/find-exe
+           racket/system)
+  
+  (define exe (find-exe))
+  (unless (system* exe "-l" "tests/racket/phantom-bytes")
+    (error "run failed"))
+  
+  ;; Also try in incremental mode
+  (void (putenv "PLT_INCREMENTAL_GC" "yes"))
+  (unless (system* exe "-l" "tests/racket/phantom-bytes")
+    (error "run failed")))

racket/src/racket/gc2/mem_account.c

@@ -529,12 +529,11 @@ static void BTC_do_accounting(NewGC *gc)
     last = cur;
     while(cur) {
       int owner = custodian_to_owner_set(gc, cur);
-      uintptr_t save_count = gc->phantom_count;
 
       GC_ASSERT(owner >= 0);
       GC_ASSERT(owner <= gc->owner_table_size);
       
-      gc->phantom_count = 0;
+      gc->acct_phantom_count = 0;
 
       gc->current_mark_owner = owner;
       GCDEBUG((DEBUGOUTF,"MARKING THREADS OF OWNER %i (CUST %p)\n", owner, cur));
@@ -550,8 +549,7 @@ static void BTC_do_accounting(NewGC *gc)
 
       owner_table = gc->owner_table;
       owner_table[owner]->memory_use = add_no_overflow(owner_table[owner]->memory_use, 
-                                                       gcBYTES_TO_WORDS(gc->phantom_count));
-      gc->phantom_count = save_count;
+                                                       gcBYTES_TO_WORDS(gc->acct_phantom_count));
     }
 
     release_master_btc_mark(gc);

racket/src/racket/gc2/newgc.c

@@ -178,6 +178,7 @@ inline static int page_mmu_type(mpage *page);
 inline static int page_mmu_protectable(mpage *page);
 static void free_mpage(mpage *page);
 static void gen_half_free_mpage(NewGC *gc, mpage *work);
+static int inc_marked_gen1(NewGC *gc, void *p);
 
 #if defined(MZ_USE_PLACES) && defined(GC_DEBUG_PAGES)
 static FILE* gcdebugOUT(NewGC *gc) {
@@ -1647,10 +1648,19 @@ uintptr_t add_no_overflow(uintptr_t a, uintptr_t b)
   return c;
 }
 
+uintptr_t subtract_no_underflow(uintptr_t a, uintptr_t b)
+{
+  if (a >= b)
+    return a-b;
+  else
+    return 0;
+}
+
 int GC_allocate_phantom_bytes(void *pb, intptr_t request_size_bytes)
 {
   NewGC *gc = GC_get_GC();
   mpage *page;
+  int inc_count;
 
 #ifdef NEWGC_BTC_ACCOUNT
   if (request_size_bytes > 0) {
@@ -1668,20 +1678,30 @@ int GC_allocate_phantom_bytes(void *pb, intptr_t request_size_bytes)
 
   page = pagemap_find_page(gc->page_maps, pb);
 
+  if (page->generation >= AGE_GEN_1)
+    inc_count = inc_marked_gen1(gc, pb);
+  else
+    inc_count = 0;
+
   if (request_size_bytes < 0) {
     request_size_bytes = -request_size_bytes;
-    if (!page || (page->generation < AGE_GEN_1)) {
-      if (gc->gen0_phantom_count > request_size_bytes)
-        gc->gen0_phantom_count -= request_size_bytes;
-    } else {
-      if (gc->memory_in_use > request_size_bytes)
-        gc->memory_in_use -= request_size_bytes;
+    if (!page || (page->generation < AGE_GEN_1))
+      gc->gen0_phantom_count = subtract_no_underflow(gc->gen0_phantom_count, request_size_bytes);
+    else {
+      gc->memory_in_use = subtract_no_underflow(gc->memory_in_use, request_size_bytes);
+      gc->phantom_count = subtract_no_underflow(gc->phantom_count, request_size_bytes);
+      if (inc_count)
+        gc->inc_phantom_count = subtract_no_underflow(gc->inc_phantom_count, request_size_bytes);
     }
   } else {
     if (!page || (page->generation < AGE_GEN_1))
       gc->gen0_phantom_count = add_no_overflow(gc->gen0_phantom_count, request_size_bytes);
-    else
+    else {
       gc->memory_in_use = add_no_overflow(gc->memory_in_use, request_size_bytes);
+      gc->phantom_count = add_no_overflow(gc->phantom_count, request_size_bytes);
+      if (inc_count)
+        gc->inc_phantom_count = add_no_overflow(gc->inc_phantom_count, request_size_bytes);
+    }
   }
 
   /* If we've allocated enough phantom bytes, then force a GC */
@@ -2059,6 +2079,23 @@ inline static int marked(NewGC *gc, const void *p)
   }
 }
 
+/* Used outside of GC when an incremental GC might be in progress */
+static int inc_marked_gen1(NewGC *gc, void *p)
+{
+  if (gc->started_incremental) {
+    int r;
+    GC_ASSERT(!gc->check_gen1);
+    GC_ASSERT(!gc->inc_gen1);
+    gc->check_gen1 = 1;
+    gc->inc_gen1 = 1;
+    r = marked(gc, p);
+    gc->check_gen1 = 0;
+    gc->inc_gen1 = 0;
+    return r;
+  } else
+    return 0;
+}
+
 static int is_in_generation_half(NewGC *gc, const void *p)
 {
   mpage *page;
@@ -2594,10 +2631,22 @@ static int mark_phantom(void *p, struct NewGC *gc)
   Phantom_Bytes *pb = (Phantom_Bytes *)p;
 
   if (!gc->during_backpointer) {
-    if (gc->inc_gen1)
+    if (gc->doing_memory_accounting)
+      gc->acct_phantom_count = add_no_overflow(gc->acct_phantom_count, pb->count);
+    else if (gc->inc_gen1)
       gc->inc_phantom_count = add_no_overflow(gc->inc_phantom_count, pb->count);
-    else
-      gc->phantom_count = add_no_overflow(gc->phantom_count, pb->count);
+    else {
+      mpage *page = ((gc->use_gen_half && !gc->inc_gen1)
+                     ? pagemap_find_page(gc->page_maps, pb)
+                     : NULL);
+      if (page && (page->generation == AGE_GEN_HALF)) {
+        gc->gen0_phantom_count = add_no_overflow(gc->gen0_phantom_count, pb->count);
+      } else {
+        gc->phantom_count = add_no_overflow(gc->phantom_count, pb->count);
+        if (gc->started_incremental && !gc->gc_full)
+          gc->inc_phantom_count = add_no_overflow(gc->inc_phantom_count, pb->count);
+      }
+    }
   }
 
   return gcBYTES_TO_WORDS(sizeof(Phantom_Bytes));
@@ -5289,10 +5338,10 @@ static void repair_heap(NewGC *gc)
     }
   }
 
+  /* This calculation will be ignored for a full GC: */
   memory_in_use += gen_half_size_in_use(gc);
   memory_in_use = add_no_overflow(memory_in_use, gc->phantom_count);
   gc->memory_in_use = memory_in_use;
-  
 
 #if CHECK_NO_MISSED_FIXUPS
   /* Double-check that no fixups apply to live objects at this point */
@@ -6093,8 +6142,15 @@ static void garbage_collect(NewGC *gc, int force_full, int no_full, int switchin
 #endif
     park_for_inform_callback(gc);
     gc->GC_collect_inform_callback(is_master, gc->gc_full, do_incremental,
-                                   old_mem_use + old_gen0, gc->memory_in_use, 
-                                   old_mem_allocated, mmu_memory_allocated(gc->mmu)+gc->phantom_count,
+                                   /* original memory use: */
+                                   old_mem_use + old_gen0,
+                                   /* new memory use; gen0_phantom_count can be non-zero due to
+                                      phantom-bytes record in generation 1/2: */
+                                   gc->memory_in_use + gc->gen0_phantom_count,
+                                   /* original memory use, including adminstrative structures: */
+                                   old_mem_allocated,
+                                   /* new memory use with adminstrative structures: */
+                                   mmu_memory_allocated(gc->mmu)+gc->phantom_count+gc->gen0_phantom_count,
                                    gc->child_gc_total);
     unpark_for_inform_callback(gc);
   }

racket/src/racket/gc2/newgc.h

@@ -292,9 +292,10 @@ typedef struct NewGC {
   unsigned short cust_box_tag;
   unsigned short phantom_tag;
 
-  uintptr_t phantom_count;
-  uintptr_t gen0_phantom_count;
-  uintptr_t inc_phantom_count;
+  uintptr_t phantom_count; /* old-generation count; included in `memory_in_use`, except during a minor collection */
+  uintptr_t gen0_phantom_count; /* count for generation 0 + 1/2 */
+  uintptr_t inc_phantom_count; /* accumulated count for an incremental collection */
+  uintptr_t acct_phantom_count; /* count that is set during memory accounting */
 
   Roots roots;  
   struct MMU     *mmu;

racket/src/racket/src/thread.c

@@ -8325,8 +8325,10 @@ static Scheme_Object *make_phantom_bytes(int argc, Scheme_Object *argv[])
   pb->size = SCHEME_INT_VAL(argv[0]);
 
 # ifdef MZ_PRECISE_GC
-  if (!GC_allocate_phantom_bytes(pb, pb->size))
+  if (!GC_allocate_phantom_bytes(pb, pb->size)) {
+    pb->size = 0;
     scheme_raise_out_of_memory("make-phantom-bytes", NULL);
+  }
 # endif
 
   return (Scheme_Object *)pb;
@@ -8335,7 +8337,7 @@ static Scheme_Object *make_phantom_bytes(int argc, Scheme_Object *argv[])
 static Scheme_Object *set_phantom_bytes(int argc, Scheme_Object *argv[])
 {
   Scheme_Phantom_Bytes *pb;
-  intptr_t amt;
+  intptr_t old_size, amt;
 
   if (!SAME_TYPE(SCHEME_TYPE(argv[0]), scheme_phantom_bytes_type))
     scheme_wrong_contract("set-phantom-bytes!", "phantom-bytes?", 0, argc, argv);
@@ -8345,13 +8347,16 @@ static Scheme_Object *set_phantom_bytes(int argc, Scheme_Object *argv[])
   pb = (Scheme_Phantom_Bytes *)argv[0];
   amt = SCHEME_INT_VAL(argv[1]);
 
-# ifdef MZ_PRECISE_GC
-  if (!GC_allocate_phantom_bytes(pb, amt - pb->size))
-    scheme_raise_out_of_memory("make-phantom-bytes", NULL);
-# endif
-
+  old_size = pb->size;
   pb->size = amt;
 
+# ifdef MZ_PRECISE_GC
+  if (!GC_allocate_phantom_bytes(pb, amt - old_size)) {
+    pb->size = old_size;
+    scheme_raise_out_of_memory("make-phantom-bytes", NULL);
+  }
+# endif
+
   return scheme_void;
 }