[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ox-publish.el: Speed up org-publish-cache-file-needs-publish
From: |
Kyle Meyer |
Subject: |
Re: [PATCH] ox-publish.el: Speed up org-publish-cache-file-needs-publishing |
Date: |
Mon, 04 Jan 2021 03:28:19 GMT |
Thank you for the patch.
Emily Bourke writes:
> I found publishing when there were no changes to be slower than
> expected. Profiling showed me that
> `org-publish-cache-file-needs-publishing' was invoking the
> `after-find-file' hooks, which I don't think is necessary.
>
> I've changed it to avoid doing that, by using `with-temp-buffer' and
> `insert-file-contents', and noticed a significant increase in speed.
>
> Is there any reason I'm missing for using `find-file-noselect' in this
> case?
Nothing jumps out to me. For large files that are already visited, I
suppose find-file-noselect returning an existing buffer can be faster,
so relevant factors would include how many Org files a project has, how
large they are, and how many of those are visited in the current
session. My guess is that using with-temp-buffer and
insert-file-contents would be a net gain, though that gain would be
narrowed some if the temporary buffer was put into org-mode rather than
kept in fundamental-mode (more below).
> Subject: [PATCH] ox-publish.el: Speed up
> org-publish-cache-file-needs-publishing
>
> * lisp/ox-publish.el (org-publish-cache-file-needs-publishing): Use
> `with-temp-buffer' with `insert-file-contents' instead of
> `find-file-noselect'. This avoids running the `after-find-file' hook,
> which can make it significantly faster.
This reads to me like after-find-file is the hook itself. Perhaps
something like this would be clearer: "... avoids calling
after-find-file and running find-file-hook, ...".
> diff --git a/lisp/ox-publish.el b/lisp/ox-publish.el
> index 7bb2fed6e..e967286cf 100644
> --- a/lisp/ox-publish.el
> +++ b/lisp/ox-publish.el
> @@ -1290,29 +1290,26 @@ the file including them will be republished as well."
> (org-inhibit-startup t)
> included-files-ctime)
> (when (equal (file-name-extension filename) "org")
> - (let ((visiting (find-buffer-visiting filename))
> - (buf (find-file-noselect filename))
> - (case-fold-search t))
> - (unwind-protect
> - (with-current-buffer buf
> - (goto-char (point-min))
> - (while (re-search-forward "^[ \t]*#\\+INCLUDE:" nil t)
> - (let ((element (org-element-at-point)))
> - (when (eq 'keyword (org-element-type element))
> - (let* ((value (org-element-property :value element))
> - (filename
> - (and (string-match "\\`\\(\".+?\"\\|\\S-+\\)" value)
> - (let ((m (org-strip-quotes
> - (match-string 1 value))))
> - ;; Ignore search suffix.
> - (if (string-match "::.*?\\'" m)
> - (substring m 0 (match-beginning 0))
> - m)))))
> - (when filename
> - (push (org-publish-cache-ctime-of-src
> - (expand-file-name filename))
> - included-files-ctime)))))))
> - (unless visiting (kill-buffer buf)))))
> + (let ((case-fold-search t))
> + (with-temp-buffer
> + (insert-file-contents filename)
> + (goto-char (point-min))
The goto-char call can be dropped now because insert-file-contents inserts
after point.
Unlike the previous code, this doesn't activate org-mode in the buffer.
That gives a speedup. And I don't spot any code downstream that depends
on the major mode being org-mode, so it's probably safe, though perhaps
there's a subtle change in behavior here (e.g., related to syntax
table).
If org-mode isn't called, the org-inhibit-startup binding above could be
dropped.
> + (while (re-search-forward "^[ \t]*#\\+INCLUDE:" nil t)
> + (let ((element (org-element-at-point)))
> + (when (eq 'keyword (org-element-type element))
> + (let* ((value (org-element-property :value element))
> + (filename
> + (and (string-match "\\`\\(\".+?\"\\|\\S-+\\)" value)
> + (let ((m (org-strip-quotes
> + (match-string 1 value))))
> + ;; Ignore search suffix.
> + (if (string-match "::.*?\\'" m)
> + (substring m 0 (match-beginning 0))
> + m)))))
> + (when filename
> + (push (org-publish-cache-ctime-of-src
> + (expand-file-name filename))
This introduces a regression. With the previous code, the
find-file-noselect call led to default-directory being set to the Org
file's directory, and then this expand-file call on the included file
was relative to that. With the new code, default-directory isn't
changed, so it points to a non-existing or incorrect file unless the
current default-directory and the Org file's happen to match.
> + included-files-ctime)))))))))