Aaron Ecay <aarone...@gmail.com> writes: > 2012ko azaroak 16an, Eric Schulte-ek idatzi zuen: > >> >> The attached patch adds a "really-silent" results header argument. To >> see the impact compare the running time of the following two code >> blocks. > > Unfortunately, the attached patch doesn’t work correctly. This can be > seen by replacing the “seq” command in your examples with a command that > has side effects – notify-send, aplay, etc. In the :results none” case, > the command is not run at all. > > That’s because the “(funcall cmd body params)” call at l. 574 of ob.el > (patched version) has been put in a branch that is only run if :results > != none. That funcall is responsible for actually evaluating the code > block. >
Oh!, thanks for catching this, I just pushed up a fix. > > (The indentation of the patch as applied isn’t correct – the two > branches of the if on l. 565 are indented at the same depth as each > other, and as the if. So it’s possible that the problem is due to a > paren in the wrong place. But I cannot see a way to make this approach > work.) > Yes, sometimes I find this approach to be preferable to make the diffs more readable and to avoid over-indentation in very large functions. > > The code generating the slowdown is in backend-specific files. So, for > example, a new branch needs to be added to the case statements in > org-babel-R-evaluate(-session), to detect the :results none case and not > read the result. I suspect that each backend will need this treatment > (unless some of them share code). > > In the meantime, attached to this email is a patch that implements a > size check on reading results. If the results file is over 10kb, it > asks the user whether to proceed. You can test this by evaluating: > > #+begin_src sh :results silent > seq 10000 > #+end_src > > 10kb of results actually doesn’t result in a very serious hang (~5sec on > my system). But I chose the value as more of a sanity check – odds are > (?) that very few people want to see 10k of results in the (mini)buffer. > The value could be made customizable. > > I also chose the polarity of the y-or-n-p so that picking the default > (yes) option does the sensible thing of not hanging emacs, although it > thus does discard data. I’m not sure which is the worse problem. > I may be outvoted, but I find this approach too be overly complicated. Also, size may frequently not be the best proxy for the time which Emacs will take to ingest the results. Best, > > From 1053f3acfc21f24fc994ae85adff6779838b0ce7 Mon Sep 17 00:00:00 2001 > From: Aaron Ecay <aarone...@gmail.com> > Date: Sat, 17 Nov 2012 19:26:43 -0500 > Subject: [PATCH] lisp/ob.el: add a size check to > `org-babel-import-elisp-from-file' > > Reading large results can cause emacs to hang for a long time. Ask the > user whether to proceed in such cases. > > Signed-off-by: Aaron Ecay <aarone...@gmail.com> > --- > lisp/ob.el | 42 ++++++++++++++++++++++++------------------ > 1 file changed, 24 insertions(+), 18 deletions(-) > > diff --git a/lisp/ob.el b/lisp/ob.el > index bf4b455..b2385e9 100644 > --- a/lisp/ob.el > +++ b/lisp/ob.el > @@ -2462,24 +2462,30 @@ appropriate." > (defun org-babel-import-elisp-from-file (file-name &optional separator) > "Read the results located at FILE-NAME into an elisp table. > If the table is trivial, then return it as a scalar." > - (let (result) > - (save-window-excursion > - (with-temp-buffer > - (condition-case err > - (progn > - (org-table-import file-name separator) > - (delete-file file-name) > - (setq result (mapcar (lambda (row) > - (mapcar #'org-babel-string-read row)) > - (org-table-to-lisp)))) > - (error (message "Error reading results: %s" err) nil))) > - (if (null (cdr result)) ;; if result is trivial vector, then scalarize > it > - (if (consp (car result)) > - (if (null (cdr (car result))) > - (caar result) > - result) > - (car result)) > - result)))) > + (let* ((file-size (nth 7 (file-attributes file-name))) > + (can-load (or (< file-size (* 10 1024)) ; 10kb > + (not (y-or-n-p (concat "Displaying the block's large " > + "results may hang emacs; skip " > + "reading them?")))))) > + (when can-load > + (let (result) > + (save-window-excursion > + (with-temp-buffer > + (condition-case err > + (progn > + (org-table-import file-name separator) > + (delete-file file-name) > + (setq result (mapcar (lambda (row) > + (mapcar #'org-babel-string-read > row)) > + (org-table-to-lisp)))) > + (error (message "Error reading results: %s" err) nil))) > + (if (null (cdr result)) ;; if result is trivial vector, then > scalarize it > + (if (consp (car result)) > + (if (null (cdr (car result))) > + (caar result) > + result) > + (car result)) > + result)))))) > > (defun org-babel-string-read (cell) > "Strip nested \"s from around strings." > -- > 1.8.0 -- Eric Schulte http://cs.unm.edu/~eschulte
