Re: Bug? R: Org babel block execution *drastically* slower than in ESS session directly

[Eric Schulte <schulte.eric@gmail.com>]

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

Aaron Ecay <aaronecay@gmail.com> writes:

> 2012ko azaroak 17an, Eric Schulte-ek idatzi zuen:
>
>> Oh!, thanks for catching this, I just pushed up a fix.
>
> This is no longer a (complete) fix for the original problem, though.  (A
> large part of) the slowdown comes from reading the results from a temp
> file and transforming them into an elispy format, which is handled by
> the backends.  Your code only prevents them from being echoed to the
> minibuffer.
>
> You can see this by trying your original “seq” tests.  You should see
> them taking the same (very long) amount of time.
>
> If you set debug-on-quit to t (before evaluating the block) and
> interrupt emacs with C-g when it hangs, you’ll see a backtrace that goes
> through ‘org-babel-execute:sh’ and then
> ‘org-babel-import-elisp-from-file’.  The presence of the former in the
> call stack is why I claim that per-backend fixes are ultimately needed.
> The latter is where my patch addresses the problem in a temporary way.
>

I apologize for rushing through your previous email and missing
important portions of the content.

>
>> 
>> 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.
>
> I agree that my patch is a stopgap.  But until per-backend fixes are
> available, this allows certain code to run that otherwise wouldn’t (at
> least not without hacks, such as putting NULL at the end of an R source
> block so that the “.Last.value” in the block is trivial).

I'm attaching a new patch which should serve as a more permanent
solution, however, given that it touches almost every backend I want to
give the list a chance to try it out before committing it.  This patch
introduces a new macro (`org-babel-result-cond') which attempts to unify
the results processing.  Not only does this check for ":results none"
and ignore all results in that case, but it also checks whether the
results should be parsed as a scalar or a vector.  Hopefully this will
serve as useful refactoring and reduce the total amount of code as well
as provide for more uniform results processing across languages.

Please let me know if this looks like a good permanent solution.

I see that I'm now getting 5 test failures with this patch applied
locally, but I won't have time to look at this further until early next
week at the earliest, so I'm sharing the existing patch now.

Thanks,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-babel-result-cond-unified-handling-of-results.patch --]
[-- Type: text/x-patch, Size: 17068 bytes --]

From a2878534832923e42aef49176b87d9993c04af71 Mon Sep 17 00:00:00 2001
From: Eric Schulte <eric.schulte@gmx.com>
Date: Sun, 18 Nov 2012 18:02:09 -0700
Subject: [PATCH] org-babel-result-cond - unified handling of results

* lisp/ob.el (org-babel-result-cond): This function should now be used
  by all language backends to handle the processing of raw code block
  output into scalar results, table results, or ignored results
  depending on the value of result-params.

* lisp/ob-C.el (org-babel-C-execute): Use org-babel-result-cond.
* lisp/ob-R.el (org-babel-R-evaluate-external-process): Use
  org-babel-result-cond.
  (org-babel-R-evaluate-session): Use org-babel-result-cond.
* lisp/ob-awk.el (org-babel-execute:awk): Use org-babel-result-cond.
* lisp/ob-clojure.el (org-babel-execute:clojure): Use
  org-babel-result-cond.
* lisp/ob-emacs-lisp.el (org-babel-execute:emacs-lisp): Use
  org-babel-result-cond.
* lisp/ob-fortran.el (org-babel-execute:fortran): Use
  org-babel-result-cond.
* lisp/ob-io.el (org-babel-io-evaluate): Use org-babel-result-cond.
* lisp/ob-java.el (org-babel-execute:java): Use org-babel-result-cond.
* lisp/ob-lisp.el (org-babel-execute:lisp): Use org-babel-result-cond.
* lisp/ob-maxima.el (org-babel-execute:maxima): Use
  org-babel-result-cond.
* lisp/ob-picolisp.el (org-babel-execute:picolisp): Use
  org-babel-result-cond.
* lisp/ob-python.el (org-babel-python-evaluate-external-process): Use
  org-babel-result-cond.
  (org-babel-python-evaluate-session): Use org-babel-result-cond.
* lisp/ob-scala.el (org-babel-scala-evaluate): Use org-babel-result-cond.
* lisp/ob-sh.el (org-babel-sh-evaluate): Use org-babel-result-cond.
* lisp/ob-shen.el (org-babel-execute:shen): Use org-babel-result-cond.
* lisp/ob-sql.el (org-babel-execute:sql): Use org-babel-result-cond.
* lisp/ob-sqlite.el (org-babel-execute:sqlite): Use
  org-babel-result-cond.
---
 lisp/ob-C.el          | 10 +++++-----
 lisp/ob-R.el          | 18 ++++++++----------
 lisp/ob-awk.el        |  6 ++----
 lisp/ob-clojure.el    |  5 ++---
 lisp/ob-emacs-lisp.el |  9 ++++-----
 lisp/ob-fortran.el    | 10 +++++-----
 lisp/ob-io.el         |  4 ++--
 lisp/ob-java.el       |  8 ++++----
 lisp/ob-lisp.el       |  4 ++--
 lisp/ob-maxima.el     |  6 ++----
 lisp/ob-picolisp.el   |  9 ++-------
 lisp/ob-python.el     | 14 ++++----------
 lisp/ob-scala.el      |  4 ++--
 lisp/ob-sh.el         |  6 ++----
 lisp/ob-shen.el       |  5 ++---
 lisp/ob-sql.el        |  8 ++------
 lisp/ob-sqlite.el     | 32 +++++++++++++++-----------------
 lisp/ob.el            | 14 ++++++++++++++
 18 files changed, 79 insertions(+), 93 deletions(-)

diff --git a/lisp/ob-C.el b/lisp/ob-C.el
index ba50722..55e4b40 100644
--- a/lisp/ob-C.el
+++ b/lisp/ob-C.el
@@ -106,11 +106,11 @@ or `org-babel-execute:C++'."
 		     (org-babel-process-file-name tmp-src-file)) ""))))
     ((lambda (results)
        (org-babel-reassemble-table
-	(if (member "vector" (cdr (assoc :result-params params)))
-	    (let ((tmp-file (org-babel-temp-file "c-")))
-	      (with-temp-file tmp-file (insert results))
-	      (org-babel-import-elisp-from-file tmp-file))
-	  (org-babel-read results))
+	(org-babel-result-cond (cdr (assoc :result-params params))
+	  (org-babel-read results)
+	  (let ((tmp-file (org-babel-temp-file "c-")))
+	    (with-temp-file tmp-file (insert results))
+	    (org-babel-import-elisp-from-file tmp-file)))
 	(org-babel-pick-name
 	 (cdr (assoc :colname-names params)) (cdr (assoc :colnames params)))
 	(org-babel-pick-name
diff --git a/lisp/ob-R.el b/lisp/ob-R.el
index 3dedb39..5711f38 100644
--- a/lisp/ob-R.el
+++ b/lisp/ob-R.el
@@ -302,11 +302,10 @@ last statement in BODY, as elisp."
 			       (format "{function ()\n{\n%s\n}}()" body)
 			       (org-babel-process-file-name tmp-file 'noquote)))
        (org-babel-R-process-value-result
-	(if (or (member "scalar" result-params)
-		(member "verbatim" result-params))
-	    (with-temp-buffer
-	      (insert-file-contents tmp-file)
-	      (buffer-string))
+	(org-babel-result-cond result-params
+	  (with-temp-buffer
+	    (insert-file-contents tmp-file)
+	    (buffer-string))
 	  (org-babel-import-elisp-from-file tmp-file '(16)))
 	column-names-p)))
     (output (org-babel-eval org-babel-R-command body))))
@@ -335,11 +334,10 @@ last statement in BODY, as elisp."
 		  "FALSE")
 		".Last.value" (org-babel-process-file-name tmp-file 'noquote)))
        (org-babel-R-process-value-result
-	(if (or (member "scalar" result-params)
-		(member "verbatim" result-params))
-	    (with-temp-buffer
-	      (insert-file-contents tmp-file)
-	      (buffer-string))
+	(org-babel-result-cond result-params
+	  (with-temp-buffer
+	    (insert-file-contents tmp-file)
+	    (buffer-string))
 	  (org-babel-import-elisp-from-file tmp-file '(16)))
 	column-names-p)))
     (output
diff --git a/lisp/ob-awk.el b/lisp/ob-awk.el
index 6e13996..6ce67f4 100644
--- a/lisp/ob-awk.el
+++ b/lisp/ob-awk.el
@@ -78,10 +78,8 @@ called by `org-babel-execute-src-block'"
     (org-babel-reassemble-table
      ((lambda (results)
 	(when results
-	  (if (or (member "scalar" result-params)
-		  (member "verbatim" result-params)
-		  (member "output" result-params))
-	      results
+	  (org-babel-result-cond result-params
+	    results
 	    (let ((tmp (org-babel-temp-file "awk-results-")))
 	      (with-temp-file tmp (insert results))
 	      (org-babel-import-elisp-from-file tmp)))))
diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el
index f389404..f0361d3 100644
--- a/lisp/ob-clojure.el
+++ b/lisp/ob-clojure.el
@@ -79,9 +79,8 @@
     (insert (org-babel-expand-body:clojure body params))
     ((lambda (result)
        (let ((result-params (cdr (assoc :result-params params))))
-	 (if (or (member "scalar" result-params)
-		 (member "verbatim" result-params))
-	     result
+	 (org-babel-result-cond result-params
+	   result
 	   (condition-case nil (org-babel-script-escape result)
 	     (error result)))))
      (slime-eval
diff --git a/lisp/ob-emacs-lisp.el b/lisp/ob-emacs-lisp.el
index d83ca24..e2e4c6c 100644
--- a/lisp/ob-emacs-lisp.el
+++ b/lisp/ob-emacs-lisp.el
@@ -56,11 +56,10 @@
   "Execute a block of emacs-lisp code with Babel."
   (save-window-excursion
     ((lambda (result)
-       (if (or (member "scalar" (cdr (assoc :result-params params)))
-	       (member "verbatim" (cdr (assoc :result-params params))))
-	   (let ((print-level nil)
-		 (print-length nil))
-	     (format "%S" result))
+       (org-babel-result-cond (cdr (assoc :result-params params))
+	 (let ((print-level nil)
+	       (print-length nil))
+	   (format "%S" result))
 	 (org-babel-reassemble-table
 	  result
 	  (org-babel-pick-name (cdr (assoc :colname-names params))
diff --git a/lisp/ob-fortran.el b/lisp/ob-fortran.el
index 7f2d1a8..fb688bc 100644
--- a/lisp/ob-fortran.el
+++ b/lisp/ob-fortran.el
@@ -62,11 +62,11 @@
 		     (org-babel-process-file-name tmp-src-file)) ""))))
     ((lambda (results)
        (org-babel-reassemble-table
-	(if (member "vector" (cdr (assoc :result-params params)))
-	    (let ((tmp-file (org-babel-temp-file "f-")))
-	      (with-temp-file tmp-file (insert results))
-	      (org-babel-import-elisp-from-file tmp-file))
-	  (org-babel-read results))
+	(org-babel-result-cond (cdr (assoc :result-params params))
+	  (org-babel-read results)
+	  (let ((tmp-file (org-babel-temp-file "f-")))
+	    (with-temp-file tmp-file (insert results))
+	    (org-babel-import-elisp-from-file tmp-file)))
 	(org-babel-pick-name
 	 (cdr (assoc :colname-names params)) (cdr (assoc :colnames params)))
 	(org-babel-pick-name
diff --git a/lisp/ob-io.el b/lisp/ob-io.el
index 881f35a..f47bef8 100644
--- a/lisp/ob-io.el
+++ b/lisp/ob-io.el
@@ -98,8 +98,8 @@ in BODY as elisp."
                   (wrapper (format org-babel-io-wrapper-method body)))
              (with-temp-file src-file (insert wrapper))
              ((lambda (raw)
-                (if (member "code" result-params)
-                    raw
+                (org-babel-result-cond result-params
+		  raw
                   (org-babel-io-table-or-string raw)))
               (org-babel-eval
                (concat org-babel-io-command " " src-file) ""))))))
diff --git a/lisp/ob-java.el b/lisp/ob-java.el
index 75afda1..99f66b9 100644
--- a/lisp/ob-java.el
+++ b/lisp/ob-java.el
@@ -58,11 +58,11 @@
       (make-directory packagename 'parents))
     ((lambda (results)
        (org-babel-reassemble-table
-	(if (member "vector" (cdr (assoc :result-params params)))
-	    (let ((tmp-file (org-babel-temp-file "c-")))
+	(org-babel-result-cond (cdr (assoc :result-params params))
+	  (org-babel-read results)
+	  (let ((tmp-file (org-babel-temp-file "c-")))
 	      (with-temp-file tmp-file (insert results))
-	      (org-babel-import-elisp-from-file tmp-file))
-	  (org-babel-read results))
+	      (org-babel-import-elisp-from-file tmp-file)))
 	(org-babel-pick-name
 	 (cdr (assoc :colname-names params)) (cdr (assoc :colnames params)))
 	(org-babel-pick-name
diff --git a/lisp/ob-lisp.el b/lisp/ob-lisp.el
index 71e80bd..1b120a8 100644
--- a/lisp/ob-lisp.el
+++ b/lisp/ob-lisp.el
@@ -76,8 +76,8 @@ current directory string."
   (require 'slime)
   (org-babel-reassemble-table
    ((lambda (result)
-      (if (member "output" (cdr (assoc :result-params params)))
-	  (car result)
+      (org-babel-result-cond (cdr (assoc :result-params params))
+	(car result)
 	(condition-case nil
 	    (read (org-babel-lisp-vector-to-list (cadr result)))
 	  (error (cadr result)))))
diff --git a/lisp/ob-maxima.el b/lisp/ob-maxima.el
index 06fa3cf..7e5a556 100644
--- a/lisp/ob-maxima.el
+++ b/lisp/ob-maxima.el
@@ -89,10 +89,8 @@ called by `org-babel-execute-src-block'."
 	    (org-babel-eval cmd "")))))
     (if (org-babel-maxima-graphical-output-file params)
 	nil
-      (if (or (member "scalar" result-params)
-	      (member "verbatim" result-params)
-	      (member "output" result-params))
-	  result
+      (org-babel-result-cond result-params
+	result
 	(let ((tmp-file (org-babel-temp-file "maxima-res-")))
 	  (with-temp-file tmp-file (insert result))
 	  (org-babel-import-elisp-from-file tmp-file))))))
diff --git a/lisp/ob-picolisp.el b/lisp/ob-picolisp.el
index 025993c..1972d29 100644
--- a/lisp/ob-picolisp.el
+++ b/lisp/ob-picolisp.el
@@ -123,13 +123,8 @@
            (t full-body))))
 
     ((lambda (result)
-       (if (or (member "verbatim" result-params)
-               (member "scalar" result-params)
-               (member "output" result-params)
-               (member "code" result-params)
-               (member "pp" result-params)
-               (= (length result) 0))
-           result
+       (org-babel-result-cond result-params
+	 result
          (read result)))
      (if (not (string= session-name "none"))
          ;; session based evaluation
diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 041f4b5..4d94ecf 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -216,11 +216,8 @@ If RESULT-TYPE equals 'output then return standard output as a
 string.  If RESULT-TYPE equals 'value then return the value of the
 last statement in BODY, as elisp."
   ((lambda (raw)
-     (if (or (member "code" result-params)
-	     (member "pp" result-params)
-	     (and (member "output" result-params)
-		  (not (member "table" result-params))))
-	 raw
+     (org-babel-result-cond result-params
+       raw
        (org-babel-python-table-or-string (org-babel-trim raw))))
    (case result-type
      (output (org-babel-eval org-babel-python-command
@@ -269,11 +266,8 @@ last statement in BODY, as elisp."
 		       (funcall send-wait))))
     ((lambda (results)
        (unless (string= (substring org-babel-python-eoe-indicator 1 -1) results)
-	 (if (or (member "code" result-params)
-		 (member "pp" result-params)
-		 (and (member "output" result-params)
-		      (not (member "table" result-params))))
-	     results
+	 (org-babel-result-cond result-params
+	   results
 	   (org-babel-python-table-or-string results))))
      (case result-type
        (output
diff --git a/lisp/ob-scala.el b/lisp/ob-scala.el
index ea3c3f2..df344ff 100644
--- a/lisp/ob-scala.el
+++ b/lisp/ob-scala.el
@@ -104,8 +104,8 @@ in BODY as elisp."
             (wrapper (format org-babel-scala-wrapper-method body)))
        (with-temp-file src-file (insert wrapper))
        ((lambda (raw)
-          (if (member "code" result-params)
-              raw
+          (org-babel-result-cond result-params
+	    raw
             (org-babel-scala-table-or-string raw)))
         (org-babel-eval
          (concat org-babel-scala-command " " src-file) ""))))))
diff --git a/lisp/ob-sh.el b/lisp/ob-sh.el
index 1cb607f..9b21d05 100644
--- a/lisp/ob-sh.el
+++ b/lisp/ob-sh.el
@@ -141,10 +141,8 @@ return the value of the last statement in BODY."
   ((lambda (results)
      (when results
        (let ((result-params (cdr (assoc :result-params params))))
-	 (if (or (member "scalar" result-params)
-		 (member "verbatim" result-params)
-		 (member "output" result-params))
-	     results
+	 (org-babel-result-cond result-params
+	   results
 	   (let ((tmp-file (org-babel-temp-file "sh-")))
 	     (with-temp-file tmp-file (insert results))
 	     (org-babel-import-elisp-from-file tmp-file))))))
diff --git a/lisp/ob-shen.el b/lisp/ob-shen.el
index 8f4b132..220efcf 100644
--- a/lisp/ob-shen.el
+++ b/lisp/ob-shen.el
@@ -66,9 +66,8 @@ This function is called by `org-babel-execute-src-block'"
 	 (result-params (cdr (assoc :result-params params)))
          (full-body (org-babel-expand-body:shen body params)))
     ((lambda (results)
-       (if (or (member 'scalar result-params)
-	       (member 'verbatim result-params))
-	   results
+       (org-babel-result-cond result-params 
+	 results
 	 (condition-case nil (org-babel-script-escape results)
 	   (error results))))
      (with-temp-buffer
diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el
index 20a136a..90a6e0a 100644
--- a/lisp/ob-sql.el
+++ b/lisp/ob-sql.el
@@ -103,12 +103,8 @@ This function is called by `org-babel-execute-src-block'."
        (org-babel-expand-body:sql body params)))
     (message command)
     (shell-command command)
-    (if (or (member "scalar" result-params)
-	    (member "verbatim" result-params)
-	    (member "html" result-params)
-	    (member "code" result-params)
-	    (equal (point-min) (point-max)))
-	(with-temp-buffer
+    (org-babel-result-cond result-params
+      (with-temp-buffer
 	  (progn (insert-file-contents-literally out-file) (buffer-string)))
       (with-temp-buffer
 	;; need to figure out what the delimiter is for the header row
diff --git a/lisp/ob-sqlite.el b/lisp/ob-sqlite.el
index b1696d9..1f8a6c0 100644
--- a/lisp/ob-sqlite.el
+++ b/lisp/ob-sqlite.el
@@ -98,23 +98,21 @@ This function is called by `org-babel-execute-src-block'."
 	  (cons "db " db)))
 	;; body of the code block
 	(org-babel-expand-body:sqlite body params)))
-      (if (or (member "scalar" result-params)
-	      (member "verbatim" result-params)
-	      (member "html" result-params)
-	      (member "code" result-params)
-	      (equal (point-min) (point-max)))
-	  (buffer-string)
-	(org-table-convert-region (point-min) (point-max)
-				  (if (or (member :csv others)
-					  (member :column others)
-					  (member :line others)
-					  (member :list others)
-					  (member :html others) separator)
-				      nil
-				    '(4)))
-	(org-babel-sqlite-table-or-scalar
-	 (org-babel-sqlite-offset-colnames
-	  (org-table-to-lisp) headers-p))))))
+      (org-babel-result-cond result-params
+	(buffer-string)
+	(if (equal (point-min) (point-max))
+	    ""
+	  (org-table-convert-region (point-min) (point-max)
+				    (if (or (member :csv others)
+					    (member :column others)
+					    (member :line others)
+					    (member :list others)
+					    (member :html others) separator)
+					nil
+				      '(4)))
+	  (org-babel-sqlite-table-or-scalar
+	   (org-babel-sqlite-offset-colnames
+	    (org-table-to-lisp) headers-p)))))))
 
 (defun org-babel-sqlite-expand-vars (body vars)
   "Expand the variables held in VARS in BODY."
diff --git a/lisp/ob.el b/lisp/ob.el
index 84b1180..e0192a1 100644
--- a/lisp/ob.el
+++ b/lisp/ob.el
@@ -2561,6 +2561,20 @@ additionally processed by `shell-quote-argument'"
 Used by `org-babel-temp-file'.  This directory will be removed on
 Emacs shutdown."))
 
+(defmacro org-babel-result-cond (result-params scalar-form &rest table-forms)
+  "Call the code to parse raw string results according to RESULT-PARAMS."
+  (declare (indent 1))
+  `(unless (member "none" result-params)
+     (if (or (member "scalar" result-params)
+	     (member "verbatim" result-params)
+	     (member "html" result-params)
+	     (member "code" result-params)
+	     (member "pp" result-params)
+	     (and (member "output" result-params)
+		  (not (member "table" result-params))))
+	 ,scalar-form
+       ,@table-forms)))
+
 (defun org-babel-temp-file (prefix &optional suffix)
   "Create a temporary file in the `org-babel-temporary-directory'.
 Passes PREFIX and SUFFIX directly to `make-temp-file' with the
-- 
1.8.0


[-- Attachment #3: Type: text/plain, Size: 46 bytes --]


-- 
Eric Schulte
http://cs.unm.edu/~eschulte