Merge pull request mono#2871 from BrzVlad/feature-conc-sweep-nrs

[sgen] Run sweep concurrently with nursery collections

mono/sgen/sgen-cardtable.c

@@ -410,7 +410,7 @@ static void
 sgen_card_table_clear_cards (void)
 {
 	/*XXX we could do this in 2 ways. using mincore or iterating over all sections/los objects */
-	sgen_major_collector_iterate_live_block_ranges (clear_cards);
+	sgen_major_collector_iterate_block_ranges (clear_cards);
 	sgen_los_iterate_live_block_ranges (clear_cards);
 }
 
@@ -431,7 +431,7 @@ sgen_card_table_scan_remsets (ScanCopyContext ctx)
 #ifdef SGEN_HAVE_OVERLAPPING_CARDS
 	/*FIXME we should have a bit on each block/los object telling if the object have marked cards.*/
 	/*First we copy*/
-	sgen_major_collector_iterate_live_block_ranges (move_cards_to_shadow_table);
+	sgen_major_collector_iterate_block_ranges (move_cards_to_shadow_table);
 	sgen_los_iterate_live_block_ranges (move_cards_to_shadow_table);
 
 	/*Then we clear*/

mono/sgen/sgen-gc.c

@@ -3152,6 +3152,12 @@ sgen_major_collector_iterate_live_block_ranges (sgen_cardtable_block_callback ca
 	major_collector.iterate_live_block_ranges (callback);
 }
 
+void
+sgen_major_collector_iterate_block_ranges (sgen_cardtable_block_callback callback)
+{
+	major_collector.iterate_block_ranges (callback);
+}
+
 SgenMajorCollector*
 sgen_get_major_collector (void)
 {

mono/sgen/sgen-gc.h

@@ -571,6 +571,7 @@ sgen_update_reference (GCObject **p, GCObject *o, gboolean allow_null)
 
 typedef void (*sgen_cardtable_block_callback) (mword start, mword size);
 void sgen_major_collector_iterate_live_block_ranges (sgen_cardtable_block_callback callback);
+void sgen_major_collector_iterate_block_ranges (sgen_cardtable_block_callback callback);
 
 typedef enum {
 	ITERATE_OBJECTS_SWEEP = 1,
@@ -625,6 +626,7 @@ struct _SgenMajorCollector {
 	void (*pin_major_object) (GCObject *obj, SgenGrayQueue *queue);
 	void (*scan_card_table) (CardTableScanType scan_type, ScanCopyContext ctx);
 	void (*iterate_live_block_ranges) (sgen_cardtable_block_callback callback);
+	void (*iterate_block_ranges) (sgen_cardtable_block_callback callback);
 	void (*update_cardtable_mod_union) (void);
 	void (*init_to_space) (void);
 	void (*sweep) (void);

mono/sgen/sgen-marksweep.c

@@ -194,16 +194,23 @@ static SgenArrayList allocated_blocks = SGEN_ARRAY_LIST_INIT (NULL, NULL, NULL,
 static void *empty_blocks = NULL;
 static size_t num_empty_blocks = 0;
 
+/*
+ * We can iterate the block list also while sweep is in progress but we
+ * need to account for blocks that will be checked for sweeping and even
+ * freed in the process.
+ */
 #define FOREACH_BLOCK_NO_LOCK(bl) {					\
 	volatile gpointer *slot;						\
-	SGEN_ASSERT (0, !sweep_in_progress (), "Can't iterate blocks while sweep is in progress."); \
 	SGEN_ARRAY_LIST_FOREACH_SLOT (&allocated_blocks, slot) {	\
-		(bl) = BLOCK_UNTAG (*slot);
+		(bl) = BLOCK_UNTAG (*slot);				\
+		if (!(bl))						\
+			continue;
 #define FOREACH_BLOCK_HAS_REFERENCES_NO_LOCK(bl,hr) {			\
 	volatile gpointer *slot;						\
-	SGEN_ASSERT (0, !sweep_in_progress (), "Can't iterate blocks while sweep is in progress."); \
 	SGEN_ARRAY_LIST_FOREACH_SLOT (&allocated_blocks, slot) {	\
 		(bl) = (MSBlockInfo *) (*slot);			\
+		if (!(bl))						\
+			continue;					\
 		(hr) = BLOCK_IS_TAGGED_HAS_REFERENCES ((bl));		\
 		(bl) = BLOCK_UNTAG ((bl));
 #define END_FOREACH_BLOCK_NO_LOCK	} SGEN_ARRAY_LIST_END_FOREACH_SLOT; }
@@ -549,16 +556,6 @@ ms_alloc_block (int size_index, gboolean pinned, gboolean has_references)
 
 	add_free_block (free_blocks, size_index, info);
 
-	/*
-	 * Adding to the allocated_blocks array is racy with the removal of nulls when
-	 * sweeping. We wait for sweep to finish to avoid that.
-	 *
-	 * The memory barrier here and in `sweep_job_func()` are required because we need
-	 * `allocated_blocks` synchronized between this and the sweep thread.
-	 */
-	major_finish_sweep_checking ();
-	mono_memory_barrier ();
-
 	sgen_array_list_add (&allocated_blocks, BLOCK_TAG (info), 0, FALSE);
 
 	SGEN_ATOMIC_ADD_P (num_major_sections, 1);
@@ -1313,6 +1310,7 @@ set_block_state (MSBlockInfo *block, gint32 new_state, gint32 expected_state)
 {
 	SGEN_ASSERT (6, block->state == expected_state, "Block state incorrect before set");
 	block->state = new_state;
+	binary_protocol_block_set_state (block, MS_BLOCK_SIZE, expected_state, new_state);
 }
 
 /*
@@ -1426,6 +1424,8 @@ sweep_start (void)
 		for (j = 0; j < num_block_obj_sizes; ++j)
 			free_blocks [j] = NULL;
 	}
+
+	sgen_array_list_remove_nulls (&allocated_blocks);
 }
 
 static void sweep_finish (void);
@@ -1580,9 +1580,12 @@ static void
 sweep_blocks_job_func (void *thread_data_untyped, SgenThreadPoolJob *job)
 {
 	volatile gpointer *slot;
+	MSBlockInfo *bl;
 
 	SGEN_ARRAY_LIST_FOREACH_SLOT (&allocated_blocks, slot) {
-		sweep_block (BLOCK_UNTAG (*slot));
+		bl = BLOCK_UNTAG (*slot);
+		if (bl)
+			sweep_block (bl);
 	} SGEN_ARRAY_LIST_END_FOREACH_SLOT;
 
 	mono_memory_write_barrier ();
@@ -1629,8 +1632,6 @@ sweep_job_func (void *thread_data_untyped, SgenThreadPoolJob *job)
 		}
 	}
 
-	sgen_array_list_remove_nulls (&allocated_blocks);
-
 	/*
 	 * Concurrently sweep all the blocks to reduce workload during minor
 	 * pauses where we need certain blocks to be swept. At the start of
@@ -2219,6 +2220,18 @@ major_print_gc_param_usage (void)
 /*
  * This callback is used to clear cards, move cards to the shadow table and do counting.
  */
+static void
+major_iterate_block_ranges (sgen_cardtable_block_callback callback)
+{
+	MSBlockInfo *block;
+	gboolean has_references;
+
+	FOREACH_BLOCK_HAS_REFERENCES_NO_LOCK (block, has_references) {
+		if (has_references)
+			callback ((mword)MS_BLOCK_FOR_BLOCK_INFO (block), MS_BLOCK_SIZE);
+	} END_FOREACH_BLOCK_NO_LOCK;
+}
+
 static void
 major_iterate_live_block_ranges (sgen_cardtable_block_callback callback)
 {
@@ -2418,12 +2431,15 @@ static void
 major_scan_card_table (CardTableScanType scan_type, ScanCopyContext ctx)
 {
 	MSBlockInfo *block;
-	gboolean has_references;
+	gboolean has_references, was_sweeping, skip_scan;
 
 	if (!concurrent_mark)
 		g_assert (scan_type == CARDTABLE_SCAN_GLOBAL);
 
-	major_finish_sweep_checking ();
+	if (scan_type != CARDTABLE_SCAN_GLOBAL)
+		SGEN_ASSERT (0, !sweep_in_progress (), "Sweep should be finished when we scan mod union card table");
+	was_sweeping = sweep_in_progress ();
+
 	binary_protocol_major_card_table_scan_start (sgen_timestamp (), scan_type & CARDTABLE_SCAN_MOD_UNION);
 	FOREACH_BLOCK_HAS_REFERENCES_NO_LOCK (block, has_references) {
 #ifdef PREFETCH_CARDS
@@ -2441,8 +2457,36 @@ major_scan_card_table (CardTableScanType scan_type, ScanCopyContext ctx)
 
 		if (!has_references)
 			continue;
+		skip_scan = FALSE;
 
-		scan_card_table_for_block (block, scan_type, ctx);
+		if (scan_type == CARDTABLE_SCAN_GLOBAL) {
+			gpointer *card_start = (gpointer*) sgen_card_table_get_card_scan_address ((mword)MS_BLOCK_FOR_BLOCK_INFO (block));
+			gboolean has_dirty_cards = FALSE;
+			int i;
+			for (i = 0; i < CARDS_PER_BLOCK / sizeof(gpointer); i++) {
+				if (card_start [i]) {
+					has_dirty_cards = TRUE;
+					break;
+				}
+			}
+			if (!has_dirty_cards) {
+				skip_scan = TRUE;
+			} else {
+				/*
+				 * After the start of the concurrent collections, blocks change state
+				 * to marking. We should not sweep it in that case. We can't race with
+				 * sweep start since we are in a nursery collection. Also avoid CAS-ing
+				 */
+				if (sweep_in_progress ()) {
+					skip_scan = !ensure_block_is_checked_for_sweeping (__index, TRUE, NULL);
+				} else if (was_sweeping) {
+					/* Recheck in case sweep finished after dereferencing the slot */
+					skip_scan = *sgen_array_list_get_slot (&allocated_blocks, __index) == 0;
+				}
+			}
+		}
+		if (!skip_scan)
+			scan_card_table_for_block (block, scan_type, ctx);
 	} END_FOREACH_BLOCK_NO_LOCK;
 	binary_protocol_major_card_table_scan_end (sgen_timestamp (), scan_type & CARDTABLE_SCAN_MOD_UNION);
 }
@@ -2579,6 +2623,7 @@ sgen_marksweep_init_internal (SgenMajorCollector *collector, gboolean is_concurr
 	collector->pin_major_object = pin_major_object;
 	collector->scan_card_table = major_scan_card_table;
 	collector->iterate_live_block_ranges = major_iterate_live_block_ranges;
+	collector->iterate_block_ranges = major_iterate_block_ranges;
 	if (is_concurrent) {
 		collector->update_cardtable_mod_union = update_cardtable_mod_union;
 		collector->get_cardtable_mod_union_for_reference = major_get_cardtable_mod_union_for_reference;

mono/sgen/sgen-pinning-stats.c

@@ -85,6 +85,8 @@ sgen_pin_stats_register_address (char *addr, int pin_type)
 	PinStatAddress *node;
 	int pin_type_bit = 1 << pin_type;
 
+	if (!do_pin_stats)
+		return;
 	while (*node_ptr) {
 		node = *node_ptr;
 		if (addr == node->addr) {