Store a separate coverage hashtable per MonoDomain and init/cleanup when initing/freeing the domain

richard-fine · joncham · commit 04d0eb722795 · 2019-01-07T13:02:27.000-05:00
diff --git a/mono/metadata/domain.c b/mono/metadata/domain.c
@@ -453,6 +453,7 @@ mono_domain_create (void)
 #endif
 
 	mono_debug_domain_create (domain);
+	mono_profiler_coverage_domain_init (domain);
 
 	if (create_domain_hook)
 		create_domain_hook (domain);
@@ -1270,6 +1271,8 @@ mono_domain_free (MonoDomain *domain, gboolean force)
 
 	if (domain == mono_root_domain)
 		mono_root_domain = NULL;
+
+	mono_profiler_coverage_domain_free(domain);
 }
 
 /**
diff --git a/mono/metadata/profiler-private.h b/mono/metadata/profiler-private.h
@@ -51,7 +51,7 @@ typedef struct {
 
 	gboolean code_coverage;
 	mono_mutex_t coverage_mutex;
-	GHashTable *coverage_hash;
+	MonoDomainCoverage *coverage_domains;
 
 	MonoProfilerHandle sampling_owner;
 	MonoSemType sampling_semaphore;
@@ -112,8 +112,18 @@ mono_profiler_installed (void)
 	return !!mono_profiler_state.profilers;
 }
 
-MonoProfilerCoverageInfo *mono_profiler_coverage_alloc (MonoMethod *method, guint32 entries);
-void mono_profiler_coverage_free (MonoMethod* method, MonoProfilerCoverageInfo* info);
+MonoProfilerCoverageInfo *mono_profiler_coverage_alloc (MonoDomain* domain, MonoMethod *method, guint32 entries);
+
+struct _MonoDomainCoverage
+{
+	MonoDomain* domain;
+	GHashTable *coverage_hash;
+	mono_mutex_t mutex;
+	MonoDomainCoverage *next;
+};
+
+void mono_profiler_coverage_domain_init (MonoDomain* domain);
+void mono_profiler_coverage_domain_free (MonoDomain* domain);
 
 struct _MonoProfilerCallContext {
 	/*
diff --git a/mono/metadata/profiler.c b/mono/metadata/profiler.c
@@ -248,7 +248,6 @@ mono_profiler_enable_coverage (void)
 		return FALSE;
 
 	mono_os_mutex_init (&mono_profiler_state.coverage_mutex);
-	mono_profiler_state.coverage_hash = g_hash_table_new (NULL, NULL);
 
 	if (!mono_debug_enabled ())
 		mono_debug_init (MONO_DEBUG_FORMAT_MONO);
@@ -279,17 +278,89 @@ mono_profiler_set_coverage_filter_callback (MonoProfilerHandle handle, MonoProfi
 }
 
 static void
-coverage_lock (void)
+coverage_domains_lock (void)
 {
 	mono_os_mutex_lock (&mono_profiler_state.coverage_mutex);
 }
 
 static void
-coverage_unlock (void)
+coverage_domains_unlock (void)
 {
 	mono_os_mutex_unlock (&mono_profiler_state.coverage_mutex);
 }
 
+static MonoDomainCoverage *
+get_coverage_for_domain(MonoDomain* domain)
+{
+	coverage_domains_lock();
+	MonoDomainCoverage* cov = mono_profiler_state.coverage_domains;
+	while (cov)
+	{
+		if (cov->domain == domain)
+			break;
+		cov = cov->next;
+	}
+	coverage_domains_unlock();
+	return cov;
+}
+
+void
+mono_profiler_coverage_domain_init(MonoDomain* domain)
+{
+	if (!mono_profiler_state.code_coverage)
+		return;
+
+	MonoDomainCoverage* cov = g_new0(MonoDomainCoverage, 1);
+	cov->domain = domain;
+	cov->coverage_hash = g_hash_table_new(NULL, NULL);
+	mono_os_mutex_init(&cov->mutex);
+
+	coverage_domains_lock();
+	cov->next = mono_profiler_state.coverage_domains;
+	mono_profiler_state.coverage_domains = cov;
+	coverage_domains_unlock();
+}
+
+void
+mono_profiler_coverage_domain_free(MonoDomain* domain)
+{
+	if (!mono_profiler_state.code_coverage)
+		return;
+
+	coverage_domains_lock();
+	
+	MonoDomainCoverage* cov = mono_profiler_state.coverage_domains;
+	MonoDomainCoverage** prev = &mono_profiler_state.coverage_domains;
+	while (cov)
+	{
+		if (cov->domain == domain)
+			break;
+
+		prev = &cov->next;
+		cov = cov->next;
+	}
+
+	if (cov != NULL)
+	{
+        *prev = cov->next;
+		
+		GHashTableIter iter;
+		g_hash_table_iter_init (&iter, cov->coverage_hash);
+		
+		MonoProfilerCoverageInfo *info;
+		while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &info))
+			g_free (info);
+
+		g_hash_table_destroy(cov->coverage_hash);
+
+		mono_os_mutex_destroy(&cov->mutex);
+
+		g_free(cov);
+	}
+
+	coverage_domains_unlock();
+}
+
 /**
  * mono_profiler_get_coverage_data:
  *
@@ -313,11 +384,13 @@ mono_profiler_get_coverage_data (MonoProfilerHandle handle, MonoMethod *method,
 	if ((method->flags & METHOD_ATTRIBUTE_ABSTRACT) || (method->iflags & METHOD_IMPL_ATTRIBUTE_RUNTIME) || (method->iflags & METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL) || (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL))
 		return FALSE;
 
-	coverage_lock ();
+	MonoDomainCoverage* domain = get_coverage_for_domain(mono_domain_get());
 
-	MonoProfilerCoverageInfo *info = g_hash_table_lookup (mono_profiler_state.coverage_hash, method);
+	mono_os_mutex_lock(&domain->mutex);
 
-	coverage_unlock ();
+	MonoProfilerCoverageInfo *info = g_hash_table_lookup (domain->coverage_hash, method);
+
+	mono_os_mutex_unlock(&domain->mutex);
 
 	MonoError error;
 	MonoMethodHeader *header = mono_method_get_header_checked (method, &error);
@@ -475,11 +548,13 @@ mono_profiler_get_all_coverage_data(MonoProfilerHandle handle, MonoProfilerCover
 	info.cb = cb;
 	info.handle = handle;
 
-	coverage_lock ();
+	MonoDomainCoverage* domain = get_coverage_for_domain(mono_domain_get());
+
+	mono_os_mutex_lock(&domain->mutex);
 
-	g_hash_table_foreach (mono_profiler_state.coverage_hash, invoke_coverage_callback_for_hashtable_entry, &info);
+	g_hash_table_foreach (domain->coverage_hash, invoke_coverage_callback_for_hashtable_entry, &info);
 
-	coverage_unlock ();
+	mono_os_mutex_unlock(&domain->mutex);
 
 	return TRUE;
 }
@@ -493,11 +568,13 @@ mono_profiler_reset_coverage(MonoMethod* method)
 	if ((method->flags & METHOD_ATTRIBUTE_ABSTRACT) || (method->iflags & METHOD_IMPL_ATTRIBUTE_RUNTIME) || (method->iflags & METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL) || (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL))
 		return FALSE;
 
-	coverage_lock ();
+	MonoDomainCoverage* domain = get_coverage_for_domain(mono_domain_get());
 
-	MonoProfilerCoverageInfo *info = g_hash_table_lookup (mono_profiler_state.coverage_hash, method);
+	mono_os_mutex_lock(&domain->mutex);
 
-	coverage_unlock ();
+	MonoProfilerCoverageInfo *info = g_hash_table_lookup (domain->coverage_hash, method);
+
+	mono_os_mutex_unlock(&domain->mutex);
 
 	if (!info)
 		return TRUE;
@@ -521,17 +598,19 @@ void mono_profiler_reset_all_coverage()
 	if (!mono_profiler_state.code_coverage)
 		return;
 
-	coverage_lock ();
+	MonoDomainCoverage* domain = get_coverage_for_domain(mono_domain_get());
+
+	mono_os_mutex_lock(&domain->mutex);
 
-	g_hash_table_foreach (mono_profiler_state.coverage_hash, reset_coverage_for_hashtable_entry, NULL);
+	g_hash_table_foreach (domain->coverage_hash, reset_coverage_for_hashtable_entry, NULL);
 
-	coverage_unlock ();
+	mono_os_mutex_unlock(&domain->mutex);
 }
 
 #endif
 
 MonoProfilerCoverageInfo *
-mono_profiler_coverage_alloc (MonoMethod *method, guint32 entries)
+mono_profiler_coverage_alloc (MonoDomain *domain, MonoMethod *method, guint32 entries)
 {
 	if (!mono_profiler_state.code_coverage)
 		return FALSE;
@@ -551,33 +630,21 @@ mono_profiler_coverage_alloc (MonoMethod *method, guint32 entries)
 	if (!cover)
 		return NULL;
 
-	coverage_lock ();
+	MonoDomainCoverage* covdomain = get_coverage_for_domain(domain);
+
+	mono_os_mutex_lock(&covdomain->mutex);
 
 	MonoProfilerCoverageInfo *info = g_malloc0 (sizeof (MonoProfilerCoverageInfo) + SIZEOF_VOID_P * 2 * entries);
 
 	info->entries = entries;
 
-	g_hash_table_insert (mono_profiler_state.coverage_hash, method, info);
+	g_hash_table_insert (covdomain->coverage_hash, method, info);
 
-	coverage_unlock ();
+	mono_os_mutex_unlock(&covdomain->mutex);
 
 	return info;
 }
 
-void mono_profiler_coverage_free (MonoMethod* method, MonoProfilerCoverageInfo* info)
-{
-	if (!mono_profiler_state.code_coverage)
-		return;
-
-	coverage_lock ();
-
-	g_hash_table_remove(mono_profiler_state.coverage_hash, method);
-
-	coverage_unlock ();
-
-	g_free (info);
-}
-
 /**
  * mono_profiler_enable_sampling:
  *
@@ -972,21 +1039,6 @@ mono_profiler_cleanup (void)
 		g_free (cur);
 	}
 
-	if (mono_profiler_state.code_coverage) {
-		mono_os_mutex_destroy (&mono_profiler_state.coverage_mutex);
-
-		GHashTableIter iter;
-
-		g_hash_table_iter_init (&iter, mono_profiler_state.coverage_hash);
-
-		MonoProfilerCoverageInfo *info;
-
-		while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &info))
-			g_free (info);
-
-		g_hash_table_destroy (mono_profiler_state.coverage_hash);
-	}
-
 	if (mono_profiler_state.sampling_owner)
 		mono_os_sem_destroy (&mono_profiler_state.sampling_semaphore);
 }
diff --git a/mono/metadata/profiler.h b/mono/metadata/profiler.h
@@ -38,6 +38,8 @@ typedef struct {
 	uint32_t column;
 } MonoProfilerCoverageData;
 
+typedef struct _MonoDomainCoverage MonoDomainCoverage;
+
 typedef mono_bool (*MonoProfilerCoverageFilterCallback) (MonoProfiler *prof, MonoMethod *method);
 typedef void (*MonoProfilerCoverageCallback) (MonoProfiler *prof, const MonoProfilerCoverageData *data);
 
diff --git a/mono/mini/method-to-ir.c b/mono/mini/method-to-ir.c
@@ -7255,7 +7255,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 	}
 
 	if (cfg->method == method)
-		cfg->coverage_info = mono_profiler_coverage_alloc (cfg->method, header->code_size);
+		cfg->coverage_info = mono_profiler_coverage_alloc (cfg->domain, cfg->method, header->code_size);
 	if (cfg->compile_aot && cfg->coverage_info)
 		g_error ("Coverage profiling is not supported with AOT.");
 

Original file line number	Diff line number	Diff line change
`@@ -7255,7 +7255,7 @@ mono_method_to_ir (MonoCompile cfg, MonoMethod method, MonoBasicBlock *start_b`
`7255`	`7255`	`}`
`7256`	`7256`
`7257`	`7257`	`if (cfg->method == method)`
`7258`		`- cfg->coverage_info = mono_profiler_coverage_alloc (cfg->method, header->code_size);`
	`7258`	`+ cfg->coverage_info = mono_profiler_coverage_alloc (cfg->domain, cfg->method, header->code_size);`
`7259`	`7259`	`if (cfg->compile_aot && cfg->coverage_info)`
`7260`	`7260`	`g_error ("Coverage profiling is not supported with AOT.");`
`7261`	`7261`