unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
To: Stefan Monnier <monnier@IRO.UMontreal.CA>
Cc: John Wiegley <jwiegley@gmail.com>,
	Paul Eggert <eggert@cs.ucla.edu>,
	22114@debbugs.gnu.org
Subject: bug#22114: 24.5; [PATCH] Allow profiler.el to display reports after stopping
Date: Mon, 14 Dec 2015 09:28:36 +0100	[thread overview]
Message-ID: <20151214082836.GA806@odonien.fritz.box> (raw)
In-Reply-To: <jwv8u4xhdsg.fsf-monnier+emacsbugs@gnu.org>

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

> Your testing was too superficial.  IIRC The problem to which the comments
> refer can only show up if you modify the hash-table.
> 
> The problem is corner-case, but real.  And there's no reason not to plug
> that hole.

I see, I can trigger this by removing a key from the log.  I haven't
thought about that possibility yet, because, why would you do that?
Nevertheless, if I'd want to fix this, I'd just need to use
`copy-hash-table` on the log and return its result?

> E.g. I don't think you need to the C-side changes to get the Elisp
> side to provide the functionality you're looking for.

I've initially provided an elisp-only patch that works around the
profiler resetting the log with an extra level of indirection:  Before
stopping the profiler or accessing the log while it's still running, the
log is saved to a variable, the other code just accesses that variable
instead of retrieving the log directly.  I'm not really happy with it
though as it does need more code than the other variant and does only
fix the UI aspect of viewing a report after the profiler has been
stopped.  In case you haven't seen it yet, I'll attach it to this
message.

[-- Attachment #2: 0001-Allow-for-retrieving-profiler-logs-after-stopping.patch --]
[-- Type: text/x-diff, Size: 3675 bytes --]

From 3464446b83151ad894ff4ee7f62ed1cb7852b1b6 Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date: Tue, 8 Dec 2015 09:01:15 +0100
Subject: [PATCH] Allow for retrieving profiler logs after stopping

---
 lisp/profiler.el | 59 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/lisp/profiler.el b/lisp/profiler.el
index f28bbfe..10cdcfb 100644
--- a/lisp/profiler.el
+++ b/lisp/profiler.el
@@ -214,21 +214,23 @@ Optional argument MODE means only check for the specified mode (cpu or mem)."
         (t (or (profiler-running-p 'cpu)
                (profiler-running-p 'mem)))))
 
+(defvar profiler-cpu-log nil)
+
+(defvar profiler-memory-log nil)
+
 (defun profiler-cpu-profile ()
   "Return CPU profile."
-  (when (profiler-running-p 'cpu)
-    (profiler-make-profile
-     :type 'cpu
-     :timestamp (current-time)
-     :log (profiler-cpu-log))))
+  (profiler-make-profile
+   :type 'cpu
+   :timestamp (current-time)
+   :log profiler-cpu-log))
 
 (defun profiler-memory-profile ()
   "Return memory profile."
-  (when (profiler-memory-running-p)
-    (profiler-make-profile
-     :type 'memory
-     :timestamp (current-time)
-     :log (profiler-memory-log))))
+  (profiler-make-profile
+   :type 'memory
+   :timestamp (current-time)
+   :log profiler-memory-log))
 
 \f
 ;;; Calltrees
@@ -828,7 +830,11 @@ Also, if MODE is `mem' or `cpu+mem', then memory profiler will be started."
 (defun profiler-stop ()
   "Stop started profilers.  Profiler logs will be kept."
   (interactive)
-  (let ((cpu (if (fboundp 'profiler-cpu-stop) (profiler-cpu-stop)))
+  (when (and (fboundp 'profiler-cpu-running-p) (profiler-cpu-running-p))
+    (setq profiler-cpu-log (profiler-cpu-log)))
+  (when (profiler-memory-running-p)
+    (setq profiler-memory-log (profiler-memory-log)))
+  (let ((cpu (when (fboundp 'profiler-cpu-stop) (profiler-cpu-stop)))
         (mem (profiler-memory-stop)))
     (message "%s profiler stopped"
              (cond ((and mem cpu) "CPU and memory")
@@ -839,26 +845,31 @@ Also, if MODE is `mem' or `cpu+mem', then memory profiler will be started."
 (defun profiler-reset ()
   "Reset profiler logs."
   (interactive)
-  (when (fboundp 'profiler-cpu-log)
-    (ignore (profiler-cpu-log)))
-  (ignore (profiler-memory-log))
-  t)
+  (when (and (fboundp 'profiler-cpu-running-p) (profiler-cpu-running-p))
+    (profiler-cpu-stop))
+  (when (profiler-memory-running-p)
+    (profiler-memory-stop))
+  (setq profiler-cpu-log nil
+        profiler-memory-log nil))
 
 (defun profiler-report-cpu ()
-  (let ((profile (profiler-cpu-profile)))
-    (when profile
-      (profiler-report-profile-other-window profile))))
+  (when profiler-cpu-log
+    (profiler-report-profile-other-window (profiler-cpu-profile))))
 
 (defun profiler-report-memory ()
-  (let ((profile (profiler-memory-profile)))
-    (when profile
-      (profiler-report-profile-other-window profile))))
+  (when profiler-memory-log
+    (profiler-report-profile-other-window (profiler-memory-profile))))
 
 (defun profiler-report ()
   "Report profiling results."
-  (interactive)
-  (profiler-report-cpu)
-  (profiler-report-memory))
+  (when (and (fboundp 'profiler-cpu-running-p) (profiler-cpu-running-p))
+    (setq profiler-cpu-log (profiler-cpu-log)))
+  (when (profiler-memory-running-p)
+    (setq profiler-memory-log (profiler-memory-log)))
+  (if (and (not profiler-cpu-log) (not profiler-memory-log))
+      (user-error "No profiler run recorded")
+    (profiler-report-cpu)
+    (profiler-report-memory)))
 
 ;;;###autoload
 (defun profiler-find-profile (filename)
-- 
2.6.2


  reply	other threads:[~2015-12-14  8:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-08  8:13 bug#22114: 24.5; [PATCH] Allow profiler.el to display reports after stopping Vasilij Schneidermann
2015-12-08 16:22 ` Eli Zaretskii
2015-12-08 16:32   ` Vasilij Schneidermann
2015-12-08 17:28     ` Eli Zaretskii
2015-12-08 17:42       ` John Wiegley
2015-12-08 17:56         ` Vasilij Schneidermann
2015-12-08 18:02           ` John Wiegley
2015-12-08 18:12             ` Vasilij Schneidermann
2015-12-08 18:08         ` Eli Zaretskii
2015-12-08 18:14           ` John Wiegley
2015-12-10  9:18           ` Vasilij Schneidermann
2015-12-13 20:33           ` Vasilij Schneidermann
2015-12-13 22:18             ` Vasilij Schneidermann
2015-12-14  4:21               ` Stefan Monnier
2015-12-14  8:28                 ` Vasilij Schneidermann [this message]
2015-12-14 14:43                   ` Stefan Monnier
2015-12-14 18:23                     ` Vasilij Schneidermann
2015-12-15  4:29                       ` Stefan Monnier
2019-06-27 18:18                   ` Lars Ingebrigtsen
2019-06-30  9:21                     ` Vasilij Schneidermann
2019-06-30 12:34                       ` Basil L. Contovounesios
2015-12-08 16:40   ` Vasilij Schneidermann
2015-12-08 17:38     ` Eli Zaretskii
2015-12-08 17:44       ` Vasilij Schneidermann
2015-12-08 18:10         ` Eli Zaretskii
2015-12-08 19:15   ` Vasilij Schneidermann
2015-12-08 19:21     ` John Wiegley
2015-12-08 20:12       ` Eli Zaretskii
2015-12-08 20:39         ` John Wiegley
2015-12-08 20:10     ` Eli Zaretskii
2019-06-25 14:46   ` Lars Ingebrigtsen
2019-06-27 14:09     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151214082836.GA806@odonien.fritz.box \
    --to=v.schneidermann@gmail.com \
    --cc=22114@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    --cc=jwiegley@gmail.com \
    --cc=monnier@IRO.UMontreal.CA \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).