Re: MPS: profiler

[Helmut Eller <eller.helmut@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

On Fri, Jun 21 2024, Gerd Möllmann wrote:

>> There is a function mps_arena_busy, that might be useful.  The doc says
>> that it should only be used for debugging.  But the implementation
>> actually checks if the arena holds a lock.
>
> Yes, the SIBPROF handler could at least return early then.

Perhaps something like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Be-careful-while-profiling.patch --]
[-- Type: text/x-diff, Size: 2166 bytes --]

From d86baa8b2af2059b00c9cbec5f404008c73705a8 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Fri, 21 Jun 2024 10:12:28 +0200
Subject: [PATCH] Be careful while profiling

SIGPROF can be delived while the SIGSEGV handler is running.  In this
situation we shouldn't access MPS-managed memory.  E.g. the profiler
should not look inside closures when recording backtraces.

* src/igc.h (igc_busy_p): New.
* src/profiler.c (add_sample): When igc_busy_p, do the same as the old
GC does when it is called during GC, i.e. only increace a counter and
don't record a backtrace.
* src/igc.c (igc_busy_p): Implement it.
---
 src/igc.c      | 6 ++++++
 src/igc.h      | 1 +
 src/profiler.c | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/src/igc.c b/src/igc.c
index ce4058fb25c..7ccde80e329 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -4021,6 +4021,12 @@ igc_alloc_dump (size_t nbytes)
   return base_to_client (block);
 }
 
+bool
+igc_busy_p (void)
+{
+  return mps_arena_busy (global_igc->arena);
+}
+
 /***********************************************************************
 				  Init
  ***********************************************************************/
diff --git a/src/igc.h b/src/igc.h
index 485f23090c2..fc80a92d1a8 100644
--- a/src/igc.h
+++ b/src/igc.h
@@ -154,6 +154,7 @@ #define EMACS_IGC_H
 char *igc_dump_finish_obj (void *client, enum igc_obj_type type,
 			   char *base, char *end);
 void *igc_alloc_dump (size_t nbytes);
+bool igc_busy_p (void);
 
 # define eassert_not_mps() eassert (false)
 #else
diff --git a/src/profiler.c b/src/profiler.c
index 98d83bcf264..c24a92de6d7 100644
--- a/src/profiler.c
+++ b/src/profiler.c
@@ -341,7 +341,11 @@ record_backtrace (struct profiler_log *plog, EMACS_INT count)
 static void
 add_sample (struct profiler_log *plog, EMACS_INT count)
 {
+#ifdef HAVE_MPS
+  if (igc_busy_p ())
+#else
   if (EQ (backtrace_top_function (), QAutomatic_GC)) /* bug#60237 */
+#endif
     /* Special case the time-count inside GC because the hash-table
        code is not prepared to be used while the GC is running.
        More specifically it uses ASIZE at many places where it does
-- 
2.39.2