bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Package: Emacs
Version: 29.0.50


As the title says, I find this command terribly slow, in the sense that
it takes several seconds for Emacs to give me a prompt asking for the
mime type to use.  I usually use this command on PDF attachments (and
the profile below was for a PDF labeled as application/octet-stream).

The profile looks like:

        9957  81% - command-execute
        9957  81%  - call-interactively
        9873  80%   - funcall-interactively
        9863  80%    - gnus-mime-view-part-externally
        9787  80%     - gnus-mime-view-part-as-type
        9374  76%      - seq-filter
        9374  76%       - seq-map
        9374  76%        - apply
        9374  76%         - #<compiled -0x1c9911d9>
        9374  76%          - mapcar
        9374  76%           - #<compiled 0xd7171c>
        9370  76%            - #<compiled 0x1d5f69f8>
        9330  76%             - mailcap-mime-info
        8021  65%              - mailcap-parse-mailcaps
        6455  52%               - mailcap-parse-mailcap
        3775  30%                - insert-file-contents
        3695  30%                 - set-auto-coding
        3647  29%                  - find-auto-coding
        3308  27%                   - auto-coding-alist-lookup
        2439  19%                      assoc-default
        [...]


-- Stefan




In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnux32, X toolkit, cairo version 1.16.0, Xaw3d scroll bars)
 of 2021-10-21 built on milanesa
Repository revision: ef4e752e0a8c5100e1ace10252b933a748ec6dd2
Repository branch: work
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER
PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XAW3D XDBE
XIM XPM LUCID ZLIB

Important settings:
  value of $LANG: fr_CH.UTF-8
  locale-coding-system: utf-8-unix

Major mode: InactiveMinibuffer

Minor modes in effect:
  shell-dirtrack-mode: t
  electric-pair-mode: t
  global-reveal-mode: t
  reveal-mode: t
  auto-insert-mode: t
  savehist-mode: t
  minibuffer-electric-default-mode: t
  global-compact-docstrings-mode: t
  url-handler-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t

Load-path shadows:
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-core hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-core
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-log hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-log
/home/monnier/src/emacs/nongnu/packages/magit/lisp/git-rebase hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/git-rebase
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-pkg hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-blame hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-blame
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-margin hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-margin
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-submodule hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-submodule
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-transient hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-transient
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-wip hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-wip
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-imenu hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-imenu
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-git hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-git
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-ediff hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-ediff
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-push hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-push
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-merge hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-merge
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-sequence hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-sequence
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-diff hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-diff
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-status hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-status
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-bisect hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-bisect
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-clone hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-clone
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-obsolete hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-obsolete
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-stash hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-stash
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-reset hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-reset
/home/monnier/src/emacs/nongnu/packages/magit/lisp/git-commit-pkg hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/git-commit-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-gitignore hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-gitignore
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-section hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-section
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-repos hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-repos
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-subtree hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-subtree
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-reflog hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-reflog
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-commit hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-commit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/git-commit hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/git-commit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-autorevert hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-autorevert
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-notes hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-notes
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-bundle hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-bundle
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-patch hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-patch
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-refs hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-refs
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-utils hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-utils
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-worktree hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-worktree
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-branch hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-branch
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-process hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-process
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-tag hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-tag
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-libgit-pkg hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-libgit-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-fetch hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-fetch
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-pull hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-pull
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-mode hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-mode
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-files hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-files
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-section-pkg hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-section-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-libgit hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-libgit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-bookmark hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-bookmark
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-apply hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-apply
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-extras hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-extras
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-remote hides /home/monnier/src/emacs/nongnu/packages/git-commit/lisp/magit-remote
/home/monnier/src/emacs/nongnu/packages/arduino-mode/ob-arduino hides /home/monnier/src/emacs/nongnu/packages/org-contrib/lisp/ob-arduino
/home/monnier/src/emacs/nongnu/packages/paredit/test hides /home/monnier/src/emacs/elpa/packages/easy-kill/test
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-util hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-util
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-abbrev hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-abbrev
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-font-lock hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-font-lock
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-layout hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-layout
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-smartparens hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-smartparens
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-stack hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-stack
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-hsinspect hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-hsinspect
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-lexer hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-lexer
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-smie hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-smie
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-syntax hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-syntax
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-company hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-company
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-imenu hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-imenu
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-mode hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-mode
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-compile hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-compile
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-projectile hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-projectile
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-rx hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-rx
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-lsp-hsinspect hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-lsp-hsinspect
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra
/home/monnier/src/emacs/nongnu/packages/haskell-tng-mode/haskell-tng-extra-yasnippet hides /home/monnier/src/emacs/elpa/packages/haskell-tng/haskell-tng-extra-yasnippet
/home/monnier/src/emacs/elpa/packages/realgud-lldb/cask-install hides /home/monnier/src/emacs/elpa/packages/realgud-trepan-ni/cask-install
/home/monnier/src/emacs/elpa/packages/realgud-lldb/cask-install hides /home/monnier/src/emacs/elpa/packages/realgud/cask-install
/home/monnier/src/elisp/sml-mode/sml-mode hides /home/monnier/src/emacs/elpa/packages/sml-mode/sml-mode
/home/monnier/src/emacs/elpa/packages/taxy/taxy-magit-section hides /home/monnier/src/emacs/elpa/packages/taxy-magit-section/taxy-magit-section
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-core hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-core
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-log hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-log
/home/monnier/src/emacs/nongnu/packages/magit/lisp/git-rebase hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/git-rebase
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-pkg hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-blame hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-blame
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-margin hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-margin
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-submodule hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-submodule
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-transient hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-transient
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-wip hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-wip
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-imenu hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-imenu
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-git hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-git
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-ediff hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-ediff
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-push hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-push
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-merge hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-merge
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-sequence hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-sequence
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-diff hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-diff
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-status hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-status
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-bisect hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-bisect
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-clone hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-clone
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-obsolete hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-obsolete
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-stash hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-stash
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-reset hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-reset
/home/monnier/src/emacs/nongnu/packages/magit/lisp/git-commit-pkg hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/git-commit-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-gitignore hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-gitignore
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-section hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-section
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-repos hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-repos
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-subtree hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-subtree
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-reflog hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-reflog
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-commit hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-commit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/git-commit hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/git-commit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-autorevert hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-autorevert
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-notes hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-notes
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-bundle hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-bundle
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-patch hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-patch
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-refs hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-refs
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-utils hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-utils
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-worktree hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-worktree
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-branch hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-branch
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-process hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-process
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-tag hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-tag
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-libgit-pkg hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-libgit-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-fetch hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-fetch
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-pull hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-pull
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-mode hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-mode
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-files hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-files
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-section-pkg hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-section-pkg
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-libgit hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-libgit
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-bookmark hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-bookmark
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-apply hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-apply
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-extras hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-extras
/home/monnier/src/emacs/nongnu/packages/magit/lisp/magit-remote hides /home/monnier/src/emacs/nongnu/packages/magit-section/lisp/magit-remote
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-mode hides /home/monnier/src/elisp/haskell-mode/haskell-mode
/home/monnier/src/emacs/nongnu/packages/haskell-mode/inf-haskell hides /home/monnier/src/elisp/haskell-mode/inf-haskell
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-indent hides /home/monnier/src/elisp/haskell-mode/haskell-indent
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-doc hides /home/monnier/src/elisp/haskell-mode/haskell-doc
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-indentation hides /home/monnier/src/elisp/haskell-mode/haskell-indentation
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-decl-scan hides /home/monnier/src/elisp/haskell-mode/haskell-decl-scan
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-cabal hides /home/monnier/src/elisp/haskell-mode/haskell-cabal
/home/monnier/src/emacs/nongnu/packages/haskell-mode/haskell-font-lock hides /home/monnier/src/elisp/haskell-mode/haskell-font-lock
/home/monnier/src/emacs/elpa/packages/transient/lisp/transient hides /home/monnier/src/emacs/work/lisp/transient
/home/monnier/src/emacs/nongnu/packages/lua-mode/lua-mode hides /home/monnier/src/emacs/work/lisp/progmodes/lua-mode
/home/monnier/src/emacs/nongnu/packages/org-contrib/lisp/ox-koma-letter hides /home/monnier/src/emacs/work/lisp/org/ox-koma-letter
/home/monnier/src/emacs/nongnu/packages/org-contrib/lisp/ob-julia hides /home/monnier/src/emacs/work/lisp/org/ob-julia
/home/monnier/src/emacs/nongnu/packages/org-contrib/lisp/ol-man hides /home/monnier/src/emacs/work/lisp/org/ol-man
/home/monnier/src/elisp/sml-mode/prog-proc hides /home/monnier/src/emacs/work/lisp/emacs-lisp/prog-proc
/home/monnier/src/emacs/elpa/packages/hyperbole/set hides /home/monnier/src/emacs/work/lisp/emacs-lisp/set
/home/monnier/src/emacs/elpa/packages/landmark/landmark hides /home/monnier/src/emacs/work/lisp/obsolete/landmark
/home/monnier/src/emacs/elpa/packages/crisp/crisp hides /home/monnier/src/emacs/work/lisp/obsolete/crisp

Features:
(shadow sort mail-extr emacsbug sendmail face-remap arc-mode
archive-mode ffap epa-file reftex-dcr reftex reftex-loaddefs reftex-vars
tex-mode latexenc pcase whitespace executable copyright ielm
bug-reference smerge-mode org-eldoc org-element avl-tree ol-eww eww xdg
url-queue mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-search eieio-opt speedbar ezimage dframe gnus-art mm-uu mml2015
mm-view mml-smime smime dig gnus-sum shr kinsoku svg dom gnus-group
gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail
mail-source utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range
message rmc puny rfc822 mml mml-sec epa derived epg rfc6068 epg-config
mm-decode mm-bodies mm-encode mailabbrev gmm-utils mailheader gnus-win
gnus nnheader gnus-util rmail rmail-loaddefs mail-utils wid-edit
ol-docview ol-bibtex ol-bbdb ol-w3m ol-doi org-link-doi org ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint
org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp
ob-core ob-eval org-table oc-basic bibtex iso8601 ol org-keys oc
org-compat org-macs org-loaddefs format-spec pp smartparens-haskell
smartparens-markdown smartparens-org smartparens-text smartparens-ruby
smartparens-rust smartparens advice dash cl-extra cl-print debug
backtrace find-func vc-fossil vc-backup log-view pcvs-util vc diff
autorevert filenotify doc-view jka-compr image-mode exif misearch
multi-isearch haskell-doc inf-haskell haskell-decl-scan imenu shell
pcomplete haskell haskell-completions haskell-load haskell-commands
highlight-uses-mode haskell-modules haskell-sandbox
haskell-navigate-imports haskell-repl haskell-svg haskell-collapse
hideshow haskell-debug haskell-interactive-mode
haskell-presentation-mode haskell-compile haskell-hoogle haskell-process
haskell-session haskell-indent haskell-mode haskell-cabal haskell-utils
haskell-font-lock haskell-indentation haskell-string
haskell-sort-imports haskell-lexeme haskell-align-imports
haskell-complete-module haskell-ghc-support etags fileloop generator
xref dabbrev haskell-customize view cal-china lunar solar cal-dst
cal-bahai cal-islam cal-hebrew holidays hol-loaddefs cal-french vc-git
diff-mode vc-dispatcher filecache diary-lib diary-loaddefs mule-util
cal-move cal-menu calendar cal-loaddefs server time-date flymake-proc
flymake project compile text-property-search comint ansi-color warnings
noutline outline easy-mmode flyspell ispell checkdoc lisp-mnt mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr dired
dired-loaddefs thingatpt load-dir elec-pair reveal autoinsert savehist
minibuf-eldef disp-table compact-docstrings ede/auto eieio-base
geiser-impl help-fns radix-tree help-mode geiser-custom geiser-base ring
proof-site proof-autoloads slime-autoloads sly-autoloads cl-seq
engrave-faces gnu-elpa-features rx realgud-recursive-autoloads
finder-inf url-auth info package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
mailcap url-handlers url-parse auth-source eieio eieio-core cl-macs gv
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt
bytecomp byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc
paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame epa-hook jka-cmpr-hook simple minibuffer
cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese composite
emoji-zwj charscript charprop case-table help abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 8 386651 50718)
 (symbols 24 32087 1) (strings 16 131514 7275) (string-bytes 1 4224439)
 (vectors 8 79143)
 (vector-slots 4 2059935 123066) (floats 8 959 215) (intervals 28 10196 0)
 (buffers 564 49))

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> As the title says, I find this command terribly slow, in the sense that
> it takes several seconds for Emacs to give me a prompt asking for the
> mime type to use.  I usually use this command on PDF attachments (and
> the profile below was for a PDF labeled as application/octet-stream).

Hm.

(progn
  (require 'mailcap)
  (benchmark-run 1 (mailcap-mime-info "application/octet-stream")))

takes 0.02s for me.  Do you have very long and involved mailcap files or
something? 

>         3775  30%                - insert-file-contents
>         3695  30%                 - set-auto-coding
>         3647  29%                  - find-auto-coding
>         3308  27%                   - auto-coding-alist-lookup
>         2439  19%                      assoc-default
>         [...]

Looks like a significant amount of time is taken by
`find-auto-encoding'?  Isn't that odd?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-10-31 16:27:16] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> As the title says, I find this command terribly slow, in the sense that
>> it takes several seconds for Emacs to give me a prompt asking for the
>> mime type to use.  I usually use this command on PDF attachments (and
>> the profile below was for a PDF labeled as application/octet-stream).
>
> Hm.
>
> (progn
>   (require 'mailcap)
>   (benchmark-run 1 (mailcap-mime-info "application/octet-stream")))
>
> takes 0.02s for me.  Do you have very long and involved mailcap files or
> something? 

Not sure if it's large or not, it's whatever comes with Debian, AFAIK.
(length (mailcap-mime-types)) says 1181,
and (benchmark-run 1 (mapcar #'mailcap-mime-info (mailcap-mime-types))))
tells me it takes about 4s.

> Looks like a significant amount of time is taken by
> `find-auto-encoding'?  Isn't that odd?

Could be.  To me, the surprise (and what I'd blame as the cause of the
problem) is that we re-read the mailcap file(s) every time (i.e. 1181
times plus 1 time for `mailcap-mime-types`).


        Stefan

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


This is because of commit a5a967b43d.  Patch attached.

[-- Attachment #2: Type: text/x-diff, Size: 1010 bytes --]

From bcc02e8bec781bf843aeb725117964747d44ac88 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 31 Oct 2021 23:39:26 +0000
Subject: [PATCH] Do not read mailcap files again for each invocation of
 mailcap-mime-info.

* lisp/net/mailcap.el (mailcap-mime-info): Remove force argument to
mailcap-parse-mailcaps.  Fixes bug#51523.
---
 lisp/net/mailcap.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index 83d0eeef9f..68cf5ccd38 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -825,7 +825,7 @@ mailcap-mime-info
           (setq passed (list viewer))
         ;; None found, so heuristically select some applicable viewer
         ;; from `mailcap--computed-mime-data'.
-        (mailcap-parse-mailcaps nil t)
+        (mailcap-parse-mailcaps)
         (setq major (split-string (car ctl) "/"))
         (setq minor (cadr major)
               major (car major))
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> * lisp/net/mailcap.el (mailcap-mime-info): Remove force argument to
> mailcap-parse-mailcaps.  Fixes bug#51523.

The mailcaps are being parsed on each invocation on purpose to catch
edits.

The problem here is that it takes a long time to parse the file(s) -- it
should be pretty much instantaneous. 

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Not sure if it's large or not, it's whatever comes with Debian, AFAIK.
> (length (mailcap-mime-types)) says 1181,
> and (benchmark-run 1 (mapcar #'mailcap-mime-info (mailcap-mime-types))))
> tells me it takes about 4s.

(length (mailcap-mime-types))
=> 1636

I'm assuming my machine isn't...   (/ 4.0 0.02) => 200x faster than
yours, so is there something in the files that's making Emacs freak out?

(benchmark-run 1 (find-auto-coding "/etc/mailcap" 4096))
=> (5.4144e-05 0 0.0)

here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

>> * lisp/net/mailcap.el (mailcap-mime-info): Remove force argument to 
>> mailcap-parse-mailcaps.  Fixes bug#51523.
>
> The mailcaps are being parsed on each invocation on purpose to catch 
> edits.
>
> The problem here is that it takes a long time to parse the file(s) -- it 
> should be pretty much instantaneous.
>

Hmm...  On my computer, Stefan's recipe (benchmark-run 1 (mapcar 
#'mailcap-mime-info (mailcap-mime-types))) takes 0.5 s without the force 
argument, and 25 s with it.

FWIW, on my computer (Debian bookworm):

(length (mailcap-mime-types)) => 1478
(benchmark-run 1 (mailcap-mime-info "application/octet-stream")) => (0.073829567 1 0.02996454400000001)
(benchmark-run 1 (find-auto-coding "/etc/mailcap" 4096)) => (0.00030021999999999997 0 0.0)
(benchmark-run 1 (mapcar #'mailcap-mime-info (mailcap-mime-types))) => (25.234923454 1674 12.865286990000001)

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

>>> * lisp/net/mailcap.el (mailcap-mime-info): Remove force argument to
>>> mailcap-parse-mailcaps.  Fixes bug#51523.
>>
>> The mailcaps are being parsed on each invocation on purpose to catch
>> edits.
>>
>> The problem here is that it takes a long time to parse the file(s)
>> -- it should be pretty much instantaneous.
>>
>
> Hmm...  On my computer, Stefan's recipe (benchmark-run 1 (mapcar
> #'mailcap-mime-info (mailcap-mime-types))) takes 0.5 s without the
> force argument, and 25 s with it.

Sorry, I didn't realise that his benchmark was different than mine:

(benchmark-run 1 (mapcar  #'mailcap-mime-info (mailcap-mime-types)))

And that is indeed very slow.  But I don't think any code is doing that?
The code is doing (mailcap-mime-info "application/octet-stream").

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

>
> The mailcaps are being parsed on each invocation on purpose to catch 
> edits.
>

BTW, does it make sense to parse it on each invocation, even when this 
happens in a loop?  Would it not make sense to cache the result during a 
limited amount of time?

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> BTW, does it make sense to parse it on each invocation, even when this
> happens in a loop?  Would it not make sense to cache the result during
> a limited amount of time?

Ideally, it should keep track of the file timestamps and re-parse when
they've changed.

But I didn't think this code was used in a loop anywhere, so it didn't
seem like it was worth the bother.

This reminds me of something I've wanted more than a handful of times
-- it'd be nice to have a function/macro like

 (when-file-has-changed "/etc/mailcap"
   (do-the-parsing))

It'd just have to maintain a hash table.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


>
> This reminds me of something I've wanted more than a handful of times -- 
> it'd be nice to have a function/macro like
>
> (when-file-has-changed "/etc/mailcap"
>   (do-the-parsing))
>

See attached patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Add-a-macro-to-execute-body-only-when-a-file-has-cha.patch, Size: 1565 bytes --]

From 827f6aa9fb616d9467dcccc011a5d3eb4dbac02d Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 1 Nov 2021 00:53:05 +0000
Subject: [PATCH] Add a macro to execute body only when a file has changed.

* lisp/subr.el (when-file-has-changed): New macro.
(when-file-has-changed--hash-table): Internal variable used by the
new macro.
---
 lisp/subr.el | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index f6dbd00532..b29c56f8c6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3967,6 +3967,22 @@ function-get
             nil                         ;Re-try `get' on the same `f'.
           (setq f fundef))))
     val))
+
+(defvar when-file-has-changed--hash-table (make-hash-table)
+  "Internal variable used by `when-file-has-changed'.")
+
+(defmacro when-file-has-changed (file body)
+  "Execute BODY only if file has changed.
+The modification time of the FILE is compared to the modification
+time of FILE during a previous invocation of
+`when-file-has-changed'.  If they differ, BODY is executed."
+  `(let* ((attr (file-attributes ,file 'integer))
+	  (mtime (file-attribute-modification-time attr))
+	  (saved-mtime (gethash (intern ,file)
+				when-file-has-changed--hash-table)))
+     (when (not (equal mtime saved-mtime))
+       (puthash (intern ,file) mtime when-file-has-changed--hash-table)
+       ,body)))
 \f
 ;;;; Support for yanking and text properties.
 ;; Why here in subr.el rather than in simple.el?  --Stef
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

>> This reminds me of something I've wanted more than a handful of times 
>> -- it'd be nice to have a function/macro like
>> 
>> (when-file-has-changed "/etc/mailcap"
>>   (do-the-parsing))
>> 
>
> See attached patch.
>

And here's another attempt to fix the bug, which uses the 
when-file-has-changed macro.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


>>> This reminds me of something I've wanted more than a handful of times 
>>> -- it'd be nice to have a function/macro like
>>> 
>>> (when-file-has-changed "/etc/mailcap"
>>>   (do-the-parsing))
>>> 
>> 
>> See attached patch.
>
> And here's another attempt to fix the bug, which uses the 
> when-file-has-changed macro.
>

ENOPATCH catched.

[-- Attachment #2: Type: text/x-diff, Size: 2342 bytes --]

From e8b7530008feb232ead9c91dac2d23e7392b02e3 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 1 Nov 2021 01:22:51 +0000
Subject: [PATCH] Read mailcaps again only when necessary.

* lisp/net/mailcap.el (mailcap-parse-mailcaps): Read mailcaps again
only when at least one of the mailcap files has changed.  Fixes
bug#51523.
---
 lisp/net/mailcap.el | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index 83d0eeef9f..a147ee0ce3 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -447,19 +447,23 @@ mailcap-parse-mailcaps
               ("/etc/mailcap" system)
               ("/usr/etc/mailcap" system)
 	      ("/usr/local/etc/mailcap" system)))))
-    ;; The ~/.mailcap entries will end up first in the resulting data.
-    (dolist (spec (reverse
-                   (if (stringp path)
-                       (split-string path path-separator t)
-                     path)))
-      (let ((source (and (consp spec) (cadr spec)))
-            (file-name (if (stringp spec)
-                           spec
-                         (car spec))))
-        (when (and (file-readable-p file-name)
-                   (file-regular-p file-name))
-          (mailcap-parse-mailcap file-name source))))
-    (setq mailcap-parsed-p t)))
+    (let ((file-has-changed nil))
+      (dolist (file path)
+        (when-file-has-changed (car file) (setq file-has-changed t)))
+      (when file-has-changed
+        ;; The ~/.mailcap entries will end up first in the resulting data.
+        (dolist (spec (reverse
+                       (if (stringp path)
+                           (split-string path path-separator t)
+                         path)))
+          (let ((source (and (consp spec) (cadr spec)))
+                (file-name (if (stringp spec)
+                               spec
+                             (car spec))))
+            (when (and (file-readable-p file-name)
+                       (file-regular-p file-name))
+              (mailcap-parse-mailcap file-name source)))))
+      (setq mailcap-parsed-p t))))
 
 (defun mailcap-parse-mailcap (fname &optional source)
   "Parse out the mailcap file specified by FNAME.
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> And that is indeed very slow.  But I don't think any code is doing that?
> The code is doing (mailcap-mime-info "application/octet-stream").

The profile gave a pretty good hint:

        9863  80%    - gnus-mime-view-part-externally
        9787  80%     - gnus-mime-view-part-as-type
        9374  76%      - seq-filter
        9374  76%       - seq-map
        9374  76%        - apply
        9374  76%         - #<compiled -0x1c9911d9>
        9374  76%          - mapcar
        9374  76%           - #<compiled 0xd7171c>
        9370  76%            - #<compiled 0x1d5f69f8>
        9330  76%             - mailcap-mime-info

So, yes, we do have code that performs such a loop, in
`gnus-mime-view-part-as-type`.


        Stefan

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

<6abcac838bb842604d22@heytings.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="9Oi2R2STSB"


--9Oi2R2STSB
Content-Type: text/plain; charset=us-ascii; format=flowed


I attach a better patch, it uses a file-has-change-p function instead of a 
when-file-has-changed macro.
--9Oi2R2STSB
Content-Type: text/x-diff; name=Read-mailcaps-again-only-when-necessary.patch
Content-Transfer-Encoding: base64
Content-ID: <6abcac838bae8618f52f@heytings.org>
Content-Description: 
Content-Disposition: attachment; filename=Read-mailcaps-again-only-when-necessary.patch

RnJvbSA0MTA2YjM0MmZlNDBkNGI1ODVmNDNjMTliYTQ1Y2NjNmYzZjZjMWUx
IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0KRnJvbTogR3JlZ29yeSBIZXl0
aW5ncyA8Z3JlZ29yeUBoZXl0aW5ncy5vcmc+DQpEYXRlOiBNb24sIDEgTm92
IDIwMjEgMDk6MjU6MjMgKzAwMDANClN1YmplY3Q6IFtQQVRDSF0gUmVhZCBt
YWlsY2FwcyBhZ2FpbiBvbmx5IHdoZW4gbmVjZXNzYXJ5Lg0KDQoqIGxpc3Av
bmV0L21haWxjYXAuZWwgKG1haWxjYXAtcGFyc2UtbWFpbGNhcHMpOiBSZWFk
IG1haWxjYXBzIGFnYWluDQpvbmx5IHdoZW4gYXQgbGVhc3Qgb25lIG9mIHRo
ZSBtYWlsY2FwIGZpbGVzIGhhcyBjaGFuZ2VkLiAgRml4ZXMNCmJ1ZyM1MTUy
My4NCg0KKiBsaXNwL2ZpbGVzLmVsIChmaWxlLWhhcy1jaGFuZ2VkLXApOiBO
ZXcgZnVuY3Rpb24uDQooZmlsZS1oYXMtY2hhbmdlZC1wLS1oYXNoLXRhYmxl
KTogSW50ZXJuYWwgdmFyaWFibGUgdXNlZCBieSB0aGUNCm5ldyBmdW5jdGlv
bi4NCi0tLQ0KIGxpc3AvZmlsZXMuZWwgICAgICAgfCAxNiArKysrKysrKysr
KysrKysrDQogbGlzcC9uZXQvbWFpbGNhcC5lbCB8IDI1ICsrKysrKysrKysr
KystLS0tLS0tLS0tLS0NCiAyIGZpbGVzIGNoYW5nZWQsIDI5IGluc2VydGlv
bnMoKyksIDEyIGRlbGV0aW9ucygtKQ0KDQpkaWZmIC0tZ2l0IGEvbGlzcC9m
aWxlcy5lbCBiL2xpc3AvZmlsZXMuZWwNCmluZGV4IDFlNjVkMGNlODMuLjVl
N2JlMzg0NGUgMTAwNjQ0DQotLS0gYS9saXNwL2ZpbGVzLmVsDQorKysgYi9s
aXNwL2ZpbGVzLmVsDQpAQCAtNjE4MSw2ICs2MTgxLDIyIEBAIGZpbGUtaW4t
ZGlyZWN0b3J5LXANCiAJICAodW5sZXNzIG1pc21hdGNoDQogCSAgICAoZmls
ZS1lcXVhbC1wIHJvb3QgZGlyKSkpKSkpKQ0KIA0KKyhkZWZ2YXIgZmlsZS1o
YXMtY2hhbmdlZC1wLS1oYXNoLXRhYmxlIChtYWtlLWhhc2gtdGFibGUpDQor
ICAiSW50ZXJuYWwgdmFyaWFibGUgdXNlZCBieSBgZmlsZS1oYXMtY2hhbmdl
ZC1wJy4iKQ0KKw0KKyhkZWZ1biBmaWxlLWhhcy1jaGFuZ2VkLXAgKGZpbGUp
DQorICAiUmV0dXJuIG5vbi1uaWwgaWYgRklMRSBoYXMgY2hhbmdlZC4NCitU
aGUgbW9kaWZpY2F0aW9uIHRpbWUgb2YgRklMRSBpcyBjb21wYXJlZCB0byB0
aGUgbW9kaWZpY2F0aW9uDQordGltZSBvZiBGSUxFIGR1cmluZyBhIHByZXZp
b3VzIGludm9jYXRpb24gb2YgYGZpbGUtaGFzLWNoYW5nZWQtcCcuDQorVGhl
cmVmb3JlIHRoZSBmaXJzdCBpbnZvY2F0aW9uIG9mIGBmaWxlLWhhcy1jaGFu
Z2VkLXAnIGFsd2F5cw0KK3JldHVybnMgbm9uLW5pbC4iDQorICAobGV0KiAo
KGF0dHIgKGZpbGUtYXR0cmlidXRlcyBmaWxlICdpbnRlZ2VyKSkNCisJICAo
bXRpbWUgKGZpbGUtYXR0cmlidXRlLW1vZGlmaWNhdGlvbi10aW1lIGF0dHIp
KQ0KKwkgIChzYXZlZC1tdGltZSAoZ2V0aGFzaCAoaW50ZXJuIGZpbGUpDQor
CQkJCWZpbGUtaGFzLWNoYW5nZWQtcC0taGFzaC10YWJsZSkpKQ0KKyAgICAg
KHdoZW4gKG5vdCAoZXF1YWwgbXRpbWUgc2F2ZWQtbXRpbWUpKQ0KKyAgICAg
ICAocHV0aGFzaCAoaW50ZXJuIGZpbGUpIG10aW1lIGZpbGUtaGFzLWNoYW5n
ZWQtcC0taGFzaC10YWJsZSkpKSkNCisNCiAoZGVmdW4gY29weS1kaXJlY3Rv
cnkgKGRpcmVjdG9yeSBuZXduYW1lICZvcHRpb25hbCBrZWVwLXRpbWUgcGFy
ZW50cyBjb3B5LWNvbnRlbnRzKQ0KICAgIkNvcHkgRElSRUNUT1JZIHRvIE5F
V05BTUUuICBCb3RoIGFyZ3MgbXVzdCBiZSBzdHJpbmdzLg0KIFRoaXMgZnVu
Y3Rpb24gYWx3YXlzIHNldHMgdGhlIGZpbGUgbW9kZXMgb2YgdGhlIG91dHB1
dCBmaWxlcyB0byBtYXRjaA0KZGlmZiAtLWdpdCBhL2xpc3AvbmV0L21haWxj
YXAuZWwgYi9saXNwL25ldC9tYWlsY2FwLmVsDQppbmRleCA4M2QwZWVlZjlm
Li40ZGVkZDM4YzIyIDEwMDY0NA0KLS0tIGEvbGlzcC9uZXQvbWFpbGNhcC5l
bA0KKysrIGIvbGlzcC9uZXQvbWFpbGNhcC5lbA0KQEAgLTQ0NywxOCArNDQ3
LDE5IEBAIG1haWxjYXAtcGFyc2UtbWFpbGNhcHMNCiAgICAgICAgICAgICAg
ICgiL2V0Yy9tYWlsY2FwIiBzeXN0ZW0pDQogICAgICAgICAgICAgICAoIi91
c3IvZXRjL21haWxjYXAiIHN5c3RlbSkNCiAJICAgICAgKCIvdXNyL2xvY2Fs
L2V0Yy9tYWlsY2FwIiBzeXN0ZW0pKSkpKQ0KLSAgICA7OyBUaGUgfi8ubWFp
bGNhcCBlbnRyaWVzIHdpbGwgZW5kIHVwIGZpcnN0IGluIHRoZSByZXN1bHRp
bmcgZGF0YS4NCi0gICAgKGRvbGlzdCAoc3BlYyAocmV2ZXJzZQ0KLSAgICAg
ICAgICAgICAgICAgICAoaWYgKHN0cmluZ3AgcGF0aCkNCi0gICAgICAgICAg
ICAgICAgICAgICAgIChzcGxpdC1zdHJpbmcgcGF0aCBwYXRoLXNlcGFyYXRv
ciB0KQ0KLSAgICAgICAgICAgICAgICAgICAgIHBhdGgpKSkNCi0gICAgICAo
bGV0ICgoc291cmNlIChhbmQgKGNvbnNwIHNwZWMpIChjYWRyIHNwZWMpKSkN
Ci0gICAgICAgICAgICAoZmlsZS1uYW1lIChpZiAoc3RyaW5ncCBzcGVjKQ0K
LSAgICAgICAgICAgICAgICAgICAgICAgICAgIHNwZWMNCi0gICAgICAgICAg
ICAgICAgICAgICAgICAgKGNhciBzcGVjKSkpKQ0KLSAgICAgICAgKHdoZW4g
KGFuZCAoZmlsZS1yZWFkYWJsZS1wIGZpbGUtbmFtZSkNCi0gICAgICAgICAg
ICAgICAgICAgKGZpbGUtcmVndWxhci1wIGZpbGUtbmFtZSkpDQotICAgICAg
ICAgIChtYWlsY2FwLXBhcnNlLW1haWxjYXAgZmlsZS1uYW1lIHNvdXJjZSkp
KSkNCisgICAgKHdoZW4gKHNlcS1zb21lIChsYW1iZGEgKGYpIChmaWxlLWhh
cy1jaGFuZ2VkLXAgKGNhciBmKSkpIHBhdGgpDQorICAgICAgOzsgVGhlIH4v
Lm1haWxjYXAgZW50cmllcyB3aWxsIGVuZCB1cCBmaXJzdCBpbiB0aGUgcmVz
dWx0aW5nIGRhdGEuDQorICAgICAgKGRvbGlzdCAoc3BlYyAocmV2ZXJzZQ0K
KwkJICAgICAoaWYgKHN0cmluZ3AgcGF0aCkNCisJCQkgKHNwbGl0LXN0cmlu
ZyBwYXRoIHBhdGgtc2VwYXJhdG9yIHQpDQorCQkgICAgICAgcGF0aCkpKQ0K
KwkobGV0ICgoc291cmNlIChhbmQgKGNvbnNwIHNwZWMpIChjYWRyIHNwZWMp
KSkNCisJICAgICAgKGZpbGUtbmFtZSAoaWYgKHN0cmluZ3Agc3BlYykNCisJ
CQkgICAgIHNwZWMNCisJCQkgICAoY2FyIHNwZWMpKSkpDQorCSAgKHdoZW4g
KGFuZCAoZmlsZS1yZWFkYWJsZS1wIGZpbGUtbmFtZSkNCisJCSAgICAgKGZp
bGUtcmVndWxhci1wIGZpbGUtbmFtZSkpDQorCSAgICAobWFpbGNhcC1wYXJz
ZS1tYWlsY2FwIGZpbGUtbmFtZSBzb3VyY2UpKSkpKQ0KICAgICAoc2V0cSBt
YWlsY2FwLXBhcnNlZC1wIHQpKSkNCiANCiAoZGVmdW4gbWFpbGNhcC1wYXJz
ZS1tYWlsY2FwIChmbmFtZSAmb3B0aW9uYWwgc291cmNlKQ0KLS0gDQoyLjMz
LjANCg0K

--9Oi2R2STSB--

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


[Stefan just told me that this email got corrupted when I sent it, here it 
is again.]

I attach a better patch, it uses a file-has-change-p function instead of a 
when-file-has-changed macro.

[-- Attachment #2: Type: text/x-diff, Size: 3336 bytes --]

From 4106b342fe40d4b585f43c19ba45ccc6f3f6c1e1 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 1 Nov 2021 09:25:23 +0000
Subject: [PATCH] Read mailcaps again only when necessary.

* lisp/net/mailcap.el (mailcap-parse-mailcaps): Read mailcaps again
only when at least one of the mailcap files has changed.  Fixes
bug#51523.

* lisp/files.el (file-has-changed-p): New function.
(file-has-changed-p--hash-table): Internal variable used by the
new function.
---
 lisp/files.el       | 16 ++++++++++++++++
 lisp/net/mailcap.el | 25 +++++++++++++------------
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 1e65d0ce83..5e7be3844e 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6181,6 +6181,22 @@ file-in-directory-p
 	  (unless mismatch
 	    (file-equal-p root dir)))))))
 
+(defvar file-has-changed-p--hash-table (make-hash-table)
+  "Internal variable used by `file-has-changed-p'.")
+
+(defun file-has-changed-p (file)
+  "Return non-nil if FILE has changed.
+The modification time of FILE is compared to the modification
+time of FILE during a previous invocation of `file-has-changed-p'.
+Therefore the first invocation of `file-has-changed-p' always
+returns non-nil."
+  (let* ((attr (file-attributes file 'integer))
+	  (mtime (file-attribute-modification-time attr))
+	  (saved-mtime (gethash (intern file)
+				file-has-changed-p--hash-table)))
+     (when (not (equal mtime saved-mtime))
+       (puthash (intern file) mtime file-has-changed-p--hash-table))))
+
 (defun copy-directory (directory newname &optional keep-time parents copy-contents)
   "Copy DIRECTORY to NEWNAME.  Both args must be strings.
 This function always sets the file modes of the output files to match
diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index 83d0eeef9f..4dedd38c22 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -447,18 +447,19 @@ mailcap-parse-mailcaps
               ("/etc/mailcap" system)
               ("/usr/etc/mailcap" system)
 	      ("/usr/local/etc/mailcap" system)))))
-    ;; The ~/.mailcap entries will end up first in the resulting data.
-    (dolist (spec (reverse
-                   (if (stringp path)
-                       (split-string path path-separator t)
-                     path)))
-      (let ((source (and (consp spec) (cadr spec)))
-            (file-name (if (stringp spec)
-                           spec
-                         (car spec))))
-        (when (and (file-readable-p file-name)
-                   (file-regular-p file-name))
-          (mailcap-parse-mailcap file-name source))))
+    (when (seq-some (lambda (f) (file-has-changed-p (car f))) path)
+      ;; The ~/.mailcap entries will end up first in the resulting data.
+      (dolist (spec (reverse
+		     (if (stringp path)
+			 (split-string path path-separator t)
+		       path)))
+	(let ((source (and (consp spec) (cadr spec)))
+	      (file-name (if (stringp spec)
+			     spec
+			   (car spec))))
+	  (when (and (file-readable-p file-name)
+		     (file-regular-p file-name))
+	    (mailcap-parse-mailcap file-name source)))))
     (setq mailcap-parsed-p t)))
 
 (defun mailcap-parse-mailcap (fname &optional source)
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The profile gave a pretty good hint:
>
>         9863  80%    - gnus-mime-view-part-externally
>         9787  80%     - gnus-mime-view-part-as-type
>         9374  76%      - seq-filter
>         9374  76%       - seq-map
>         9374  76%        - apply
>         9374  76%         - #<compiled -0x1c9911d9>
>         9374  76%          - mapcar
>         9374  76%           - #<compiled 0xd7171c>
>         9370  76%            - #<compiled 0x1d5f69f8>
>         9330  76%             - mailcap-mime-info
>
> So, yes, we do have code that performs such a loop, in
> `gnus-mime-view-part-as-type`.

D'oh!  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> I attach a better patch, it uses a file-has-change-p function instead
> of a when-file-has-changed macro.

Looks good to me; pushed to Emacs 29 now (with documentation added).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> Date: Mon, 01 Nov 2021 12:26:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: 51523@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> 
> I attach a better patch, it uses a file-has-change-p function instead of a 
> when-file-has-changed macro.

Thanks.

> +(defun file-has-changed-p (file)
> +  "Return non-nil if FILE has changed.
> +The modification time of FILE is compared to the modification
> +time of FILE during a previous invocation of `file-has-changed-p'.
> +Therefore the first invocation of `file-has-changed-p' always
> +returns non-nil."
> +  (let* ((attr (file-attributes file 'integer))
> +	  (mtime (file-attribute-modification-time attr))
> +	  (saved-mtime (gethash (intern file)
> +				file-has-changed-p--hash-table)))
> +     (when (not (equal mtime saved-mtime))
> +       (puthash (intern file) mtime file-has-changed-p--hash-table))))

Bother: I think this implementation might cause both false positives
and false negatives.

  . it uses the literal file name without even expanding it to an
    absolute file name, so the same FILE might mean different files if
    default-directory changes
  . file names are generally not reliable enough for unique
    identifiers of files (think symlinks on all systems, letter-case
    and numerical tails on Windows, etc.), so we should at least use
    file-truename
  . interning the file names could produce many unnecessary symbols in
    the global obarray

Can we make the implementation more robust by fixing these?

The name of the function also doesn't reflect what it does: it only
looks at the file's last modification time, so maybe at least "time"
should be in the name?

I also question the decision of testing modification time for
equality: why not check if the time stamp is newer, and disregard the
changes to an older time stamp?  When looking this way at this
function, I ask myself whether extending file-newer-than-file-p to do
this job would be a better idea?  Or maybe I don't understand the
general use case for this function.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

>
> . it uses the literal file name without even expanding it to an absolute file name, so the same FILE might mean different files if default-directory changes
> . file names are generally not reliable enough for unique identifiers of files (think symlinks on all systems, letter-case and numerical tails on Windows, etc.), so we should at least use file-truename
> . interning the file names could produce many unnecessary symbols in the global obarray
>
> Can we make the implementation more robust by fixing these?
>

Sure, I'll do that.  Am I right that the first two points are fixed by 
using file-truename, and that the third one would be fixed by using 
(intern ... nil)?

>
> The name of the function also doesn't reflect what it does: it only 
> looks at the file's last modification time, so maybe at least "time" 
> should be in the name?
>

Perhaps we could also check the file size?  This is IIRC what rsync does 
for example (by default), it considers that a file has changed if its 
modification time or size has changed.

>
> I also question the decision of testing modification time for equality: 
> why not check if the time stamp is newer, and disregard the changes to 
> an older time stamp?
>

This wouldn't be right I think, because the user might replace a file with 
another one with an older modification time.  In which case the file has 
changed, too.

>
> When looking this way at this function, I ask myself whether extending 
> file-newer-than-file-p to do this job would be a better idea?
>

You mean, using file-newer-than-file-p with two identical arguments? 
That would always return nil, I guess.

>
> Or maybe I don't understand the general use case for this function.
>

The use case is the one of this bug: check whether a file has changed 
since the last invocation of file-has-changed-p.  It's used to solve this 
bug: mailcap-parse-mailcaps parses the mailcap files again only when at 
least one of them has changed.  Without this, when mailcap-parse-mailcaps 
is called in a loop, the mailcap files are parsed again and again, which 
is slow and unnecessary.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> This wouldn't be right I think, because the user might replace a file
> with another one with an older modification time.  In which case the
> file has changed, too.

Yup. 

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> Date: Mon, 01 Nov 2021 15:20:38 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 51523@debbugs.gnu.org, larsi@gnus.org, monnier@iro.umontreal.ca
> 
> > . it uses the literal file name without even expanding it to an absolute file name, so the same FILE might mean different files if default-directory changes
> > . file names are generally not reliable enough for unique identifiers of files (think symlinks on all systems, letter-case and numerical tails on Windows, etc.), so we should at least use file-truename
> > . interning the file names could produce many unnecessary symbols in the global obarray
> >
> > Can we make the implementation more robust by fixing these?
> >
> 
> Sure, I'll do that.  Am I right that the first two points are fixed by 
> using file-truename, and that the third one would be fixed by using 
> (intern ... nil)?

Yes for the first two, but I don't understand the last one: using nil
as the 2nd argument is the same as omitting it, and they both stand
for the global obarray.  You need to specify a different obarray to
avoid "polluting" the global one.

> > The name of the function also doesn't reflect what it does: it only 
> > looks at the file's last modification time, so maybe at least "time" 
> > should be in the name?
> 
> Perhaps we could also check the file size?

If what we really want is to detect changes in contents, yes, I think
we should also check the size.

> > I also question the decision of testing modification time for equality: 
> > why not check if the time stamp is newer, and disregard the changes to 
> > an older time stamp?
> 
> This wouldn't be right I think, because the user might replace a file with 
> another one with an older modification time.

What would be the purpose of replacing with an older file, but keeping
the old time stamp?  And why is Gnus obligated to support such strange
use cases?  We can always tell the users to bump the file's time stamp
by 'touch'ing it, no?

> > When looking this way at this function, I ask myself whether extending 
> > file-newer-than-file-p to do this job would be a better idea?
> 
> You mean, using file-newer-than-file-p with two identical arguments? 

No, that'd be silly.  I mean something like

   (file-newer-than-file-p FILE t)

should have the special meaning of doing what file-has-changed-p does
now.

> > Or maybe I don't understand the general use case for this function.
> 
> The use case is the one of this bug: check whether a file has changed 
> since the last invocation of file-has-changed-p.

But then the time stamp and the size might not be enough.  What if the
inode changed, for example?

> It's used to solve this bug: mailcap-parse-mailcaps parses the
> mailcap files again only when at least one of them has changed.

Yes, but the function is supposed to be more general than just this
one case, and I'm asking about the more general use of it.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> What would be the purpose of replacing with an older file, but keeping
> the old time stamp?

That's the interface I want.  If the file changes, in any noticeable
way, then it...  changes.  Newer or not has nothing to do with it.

(As a practical matter, this is what happens when you revert a file in
some VCs.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Gregory Heytings <gregory@heytings.org>,  51523@debbugs.gnu.org,
>   monnier@iro.umontreal.ca
> Date: Mon, 01 Nov 2021 17:59:32 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What would be the purpose of replacing with an older file, but keeping
> > the old time stamp?
> 
> That's the interface I want.  If the file changes, in any noticeable
> way, then it...  changes.  Newer or not has nothing to do with it.

But the real answer to that question is to compare the contents, not
file's attributes.  Testing attributes is an approximation, and once
we are using an approximation, it is legitimate to ask when it's okay
for the approximation to fail.

Basically, comparing time stamps is what Make does, and Make doesn't
care about the time stamp becoming older.  So why should we?

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Gregory Heytings <gregory@heytings.org>,  51523@debbugs.gnu.org,
>   monnier@iro.umontreal.ca
> Date: Mon, 01 Nov 2021 17:59:32 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What would be the purpose of replacing with an older file, but keeping
> > the old time stamp?
> 
> That's the interface I want.  If the file changes, in any noticeable
> way, then it...  changes.  Newer or not has nothing to do with it.

But the real answer to that question is to compare the contents, not
file's attributes.  Testing attributes is an approximation, and once
we are using an approximation, it is legitimate to ask when it's okay
for the approximation to fail.

Basically, comparing time stamps is what Make does, and Make doesn't
care about the time stamp becoming older.  So why should we?

And if by "I want" you mean Gnus, then maybe we should name this
function gnus-has-mailcap-file-changed-p, and stop pretending that it
has more general use?

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> But the real answer to that question is to compare the contents, not
> file's attributes.  Testing attributes is an approximation, and once
> we are using an approximation, it is legitimate to ask when it's okay
> for the approximation to fail.

The doc string clearly says that it's about the file's modification
time.

> Basically, comparing time stamps is what Make does, and Make doesn't
> care about the time stamp becoming older.  So why should we?

I don't see why we should care what Make does.

> And if by "I want" you mean Gnus, then maybe we should name this
> function gnus-has-mailcap-file-changed-p, and stop pretending that it
> has more general use?

It's not a Gnus function, and the first instance of its use is not in
Gnus, but in mailcap.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: gregory@heytings.org,  51523@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Mon, 01 Nov 2021 18:19:02 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > And if by "I want" you mean Gnus, then maybe we should name this
> > function gnus-has-mailcap-file-changed-p, and stop pretending that it
> > has more general use?
> 
> It's not a Gnus function, and the first instance of its use is not in
> Gnus, but in mailcap.

The Subject of this bug report mentions Gnus.

My point is that saying "this is what I want" is not a good argument
for deciding what a general-purpose function should and shouldn't do.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> My point is that saying "this is what I want" is not a good argument
> for deciding what a general-purpose function should and shouldn't do.

Well, we're the Emacs maintainers, so "what a maintainer wants" is pretty
much how we design general-purpose functions, isn't it?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: gregory@heytings.org,  51523@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Mon, 01 Nov 2021 18:23:16 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > My point is that saying "this is what I want" is not a good argument
> > for deciding what a general-purpose function should and shouldn't do.
> 
> Well, we're the Emacs maintainers, so "what a maintainer wants" is pretty
> much how we design general-purpose functions, isn't it?

Presumably, Emacs maintainers have some general-purpose use case or
set of use cases in their minds when they design this stuff, don't
they?  That's why I asked what would those use cases be in this case;
"this is what I want" doesn't really answer that question, and doesn't
allow to have a discussion about what is and isn't TRT for this
function to do.

It's okay to have special-purpose functions when that's convenient,
but then we shouldn't put this in files.el and shouldn't advertise it
as a general-purpose function.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> It's okay to have special-purpose functions when that's convenient,
> but then we shouldn't put this in files.el and shouldn't advertise it
> as a general-purpose function.

Like I said, it's something I've felt like implementing a dozen times
but never got around to.  And the use cases are explained in the
documentation, and I'm sure you can find plenty of cases in the Emacs
code base where something is caching something, but reloading if the
file has changed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: gregory@heytings.org,  51523@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Mon, 01 Nov 2021 18:34:35 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It's okay to have special-purpose functions when that's convenient,
> > but then we shouldn't put this in files.el and shouldn't advertise it
> > as a general-purpose function.
> 
> Like I said, it's something I've felt like implementing a dozen times
> but never got around to.  And the use cases are explained in the
> documentation, and I'm sure you can find plenty of cases in the Emacs
> code base where something is caching something, but reloading if the
> file has changed.

The changes I suggested will serve this use case as well as what we
have now.  The advantage of what I proposed is that it will also serve
other use cases.  So I don't understand why you are opposed to the
changes I proposed.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


I attach an improved version of file-has-changed-p.  Following Eli's 
suggestion, it records the file size together with its timestamp.

Stefan:
>
> This API doesn't seem safe.
>
> If two packages read&parse the same file and both rely on this function 
> to decide when to reparse, the second package can get a nil value 
> (because the first has caused the mtime to be reset in the hash table) 
> even though the file has changed since it last parsed it.
>

I agree with you, and added a second optional "tag" argument to the 
function, in which one can for example pass the name of the calling 
function, or the name of the library.  It's the responsibility of the 
caller to use the tag they want.

Eli:
>
> What would be the purpose of replacing with an older file, but keeping 
> the old time stamp?  And why is Gnus obligated to support such strange 
> use cases?  We can always tell the users to bump the file's time stamp 
> by 'touch'ing it, no?
>

It's not a strange use case at all.  For example, you make a backup of 
your ~/.mailcap file, make some changes in the original file, decide that 
after all they're not what you want, and do mv ~/.mailcap.bak ~/.mailcap.

>
> But the real answer to that question is to compare the contents, not 
> file's attributes.  Testing attributes is an approximation, and once we 
> are using an approximation, it is legitimate to ask when it's okay for 
> the approximation to fail.
>

It's an approximation indeed, but it's a very safe one.  It's what rsync 
uses, and I haven't seen a single case in which it failed to do TRT in 
twenty years or so.  The likelihood that a file with the exact same 
filename, same size and same timestamp have different contents is, in 
practice, zero.  It is of course possible to cheat that detection 
mechanism, but only if you do it on purpose.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Improve-file-has-changed-p.patch, Size: 4288 bytes --]

From f8b969b1aced58e74e942a09335ba3f3c752eba1 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 1 Nov 2021 21:10:49 +0000
Subject: [PATCH] Improve file-has-changed-p.

* lisp/files (file-has-changed-p): Add a second argument, and use it.
Update the docstring.

* doc/lispref/files.texi: Update the documentation.

* lisp/net/mailcap.el: Add a second argument to the call to
file-has-changed-p.
---
 doc/lispref/files.texi |  8 +++++---
 lisp/files.el          | 28 ++++++++++++++++------------
 lisp/net/mailcap.el    |  4 +++-
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 250f7a3f9f..ce967ee381 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -1314,13 +1314,15 @@ File Attributes
 @end example
 @end defun
 
-@defun file-has-changed-p filename
+@defun file-has-changed-p filename tag
 This convenience function is useful when, for instance, parsing files
 run-time, and you typically want to re-read a file when it has
 changed.  This function returns non-@code{nil} the first time it's
 called on @var{filename} in an Emacs session, but will return
-@code{nil} on subsequent calls in that session (unless the file
-changes its modification time).
+@code{nil} on subsequent calls in that session (unless the file size
+or modification time has changed in the meantime).  With an optional
+argument @var{tag}, the size and modification time comparisons are
+limited to calls with the same tag.
 @end defun
 
 @defun file-attributes filename &optional id-format
diff --git a/lisp/files.el b/lisp/files.el
index 5e7be3844e..d7dfa9399e 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6181,21 +6181,25 @@ file-in-directory-p
 	  (unless mismatch
 	    (file-equal-p root dir)))))))
 
-(defvar file-has-changed-p--hash-table (make-hash-table)
+(defvar file-has-changed-p--hash-table (make-hash-table :test #'equal)
   "Internal variable used by `file-has-changed-p'.")
 
-(defun file-has-changed-p (file)
+(defun file-has-changed-p (file &optional tag)
   "Return non-nil if FILE has changed.
-The modification time of FILE is compared to the modification
-time of FILE during a previous invocation of `file-has-changed-p'.
-Therefore the first invocation of `file-has-changed-p' always
-returns non-nil."
-  (let* ((attr (file-attributes file 'integer))
-	  (mtime (file-attribute-modification-time attr))
-	  (saved-mtime (gethash (intern file)
-				file-has-changed-p--hash-table)))
-     (when (not (equal mtime saved-mtime))
-       (puthash (intern file) mtime file-has-changed-p--hash-table))))
+The size and modification time of FILE is compared to the size
+and modification time of FILE during a previous invocation of
+`file-has-changed-p'.  Therefore the first invocation of
+`file-has-changed-p' always returns non-nil.
+The optional argument TAG can be used to limit the comparison to
+invocations with identical tags; it can for example be the symbol
+of the calling function."
+  (let* ((fileattr (file-attributes file 'integer))
+	 (attr (cons (file-attribute-size fileattr)
+		     (file-attribute-modification-time fileattr)))
+	 (sym (concat (symbol-name tag) "@" file))
+	 (cachedattr (gethash sym file-has-changed-p--hash-table)))
+     (when (not (equal attr cachedattr))
+       (puthash sym attr file-has-changed-p--hash-table))))
 
 (defun copy-directory (directory newname &optional keep-time parents copy-contents)
   "Copy DIRECTORY to NEWNAME.  Both args must be strings.
diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index 4dedd38c22..e40cf2a336 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -447,7 +447,9 @@ mailcap-parse-mailcaps
               ("/etc/mailcap" system)
               ("/usr/etc/mailcap" system)
 	      ("/usr/local/etc/mailcap" system)))))
-    (when (seq-some (lambda (f) (file-has-changed-p (car f))) path)
+    (when (seq-some (lambda (f)
+                      (file-has-changed-p (car f) 'mail-parse-mailcaps))
+                    path)
       ;; The ~/.mailcap entries will end up first in the resulting data.
       (dolist (spec (reverse
 		     (if (stringp path)
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> I attach an improved version of file-has-changed-p.  Following Eli's
> suggestion, it records the file size together with its timestamp.

Thanks; applied to the trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> Date: Mon, 01 Nov 2021 21:14:14 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, 51523@debbugs.gnu.org, 
>     monnier@iro.umontreal.ca
> 
> I attach an improved version of file-has-changed-p.  Following Eli's 
> suggestion, it records the file size together with its timestamp.

What about expand-file-name or file-truename?  That problem still
exists in this new patch, AFAICT.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

>> I attach an improved version of file-has-changed-p.  Following Eli's 
>> suggestion, it records the file size together with its timestamp.
>
> What about expand-file-name or file-truename?  That problem still exists 
> in this new patch, AFAICT.
>

I don't know what I should do here.  Both Stefan and Lars think (IIUC) 
that it's okay to use the filename as is.  I tend to agree with them; 
after all, the filename is only used as a key in the hash table, and 
file-attributes finds the "right" file anyway.

Using expand-file-name or file-truename also makes that function much 
slower: (benchmark-run 100000 (file-has-changed-p "~/.profile")) takes (on 
my computer) 2 seconds in its current version, 3 seconds with 
expand-file-name, and... 10 seconds with file-truename.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I don't know what I should do here.  Both Stefan and Lars think (IIUC) that
> it's okay to use the filename as is.

No: the hash table should be indexed by absolute file names.
Anything else will lead to bugs.
There are various ways to get those names to be absolute, with tradeoffs.

> Using expand-file-name or file-truename also makes that function much
> slower: (benchmark-run 100000 (file-has-changed-p "~/.profile")) takes (on
> my computer) 2 seconds in its current version, 3 seconds with
> expand-file-name, and... 10 seconds with file-truename.

In the sample code I sent around this, I used
(unless (file-name-absolute-p file) (setq file (expand-file-name file)))
in part to avoid the cost of expand-file-name.


        Stefan

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


>> I don't know what I should do here.  Both Stefan and Lars think (IIUC) 
>> that it's okay to use the filename as is.
>
> No: the hash table should be indexed by absolute file names. Anything 
> else will lead to bugs. There are various ways to get those names to be 
> absolute, with tradeoffs.
>

Okay, so there were three opinions, not two ;-)

Here's an again improved version, which I hope is a reasonable compromise.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Further-improvement-of-file-has-changed-p.patch, Size: 1618 bytes --]

From 8cdbbf29ad7f2156e8bba308d83910765ab5b387 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 3 Nov 2021 12:50:52 +0000
Subject: [PATCH] Further improvement of file-has-changed-p

* lisp/files.el (file-has-changed-p): Expand the file name when necessary.
Clarify and fix typo in the docstring.
---
 lisp/files.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 173198a424..758d0edd88 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6187,13 +6187,15 @@ file-has-changed-p--hash-table
 (defun file-has-changed-p (file &optional tag)
   "Return non-nil if FILE has changed.
 The size and modification time of FILE are compared to the size
-and modification time of tghe same FILE during a previous
+and modification time of the same FILE during a previous
 invocation of `file-has-changed-p'.  Thus, the first invocation
-of `file-has-changed-p' always returns non-nil.
+of `file-has-changed-p' on a given FILE always returns non-nil.
 The optional argument TAG, which must be a symbol, can be used to
 limit the comparison to invocations with identical tags; it can be
 the symbol of the calling function, for example."
-  (let* ((fileattr (file-attributes file 'integer))
+  (let* ((fileattr (file-attributes
+                    (if (file-name-absolute-p file) file (expand-file-name file))
+                    'integer))
 	 (attr (cons (file-attribute-size fileattr)
 		     (file-attribute-modification-time fileattr)))
 	 (sym (concat (symbol-name tag) "@" file))
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> Date: Wed, 03 Nov 2021 10:45:35 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, 51523@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> >> I attach an improved version of file-has-changed-p.  Following Eli's 
> >> suggestion, it records the file size together with its timestamp.
> >
> > What about expand-file-name or file-truename?  That problem still exists 
> > in this new patch, AFAICT.
> 
> I don't know what I should do here.  Both Stefan and Lars think (IIUC) 
> that it's okay to use the filename as is.  I tend to agree with them; 
> after all, the filename is only used as a key in the hash table, and 
> file-attributes finds the "right" file anyway.

The function is buggy without that, so much so that I tend to remove
it from Emacs.

> Using expand-file-name or file-truename also makes that function much 
> slower: (benchmark-run 100000 (file-has-changed-p "~/.profile")) takes (on 
> my computer) 2 seconds in its current version, 3 seconds with 
> expand-file-name, and... 10 seconds with file-truename.

This isn't a time critical function, so 0.1 msec per call is nothing
to be afraid of.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> Date: Wed, 03 Nov 2021 12:57:27 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 51523@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org
> 
> -  (let* ((fileattr (file-attributes file 'integer))
> +  (let* ((fileattr (file-attributes
> +                    (if (file-name-absolute-p file) file (expand-file-name file))
> +                    'integer))
>  	 (attr (cons (file-attribute-size fileattr)
>  		     (file-attribute-modification-time fileattr)))
>  	 (sym (concat (symbol-name tag) "@" file))

You don't need to expand the file name you pass to file-attributes: it
does that internally.  What you need is to expand the file name you
pass as the key, i.e. for the symbol you are generating from the file
name.

Thanks.

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


>
> You don't need to expand the file name you pass to file-attributes: it 
> does that internally.  What you need is to expand the file name you pass 
> as the key, i.e. for the symbol you are generating from the file name.
>

Whoops, sorry, that's what I meant to do of course, I went too fast. 
Sorry again for the confusion.

[-- Attachment #2: Type: text/x-diff, Size: 1813 bytes --]

From 56037a349fdefb5929347fb19a5682b627fc476b Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 3 Nov 2021 13:24:51 +0000
Subject: [PATCH] Further improvement of file-has-changed-p

* lisp/files.el (file-has-changed-p): Expand the file name when necessary.
Clarify and fix typo in the docstring.
---
 lisp/files.el | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 173198a424..2cd932adfd 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6187,16 +6187,19 @@ file-has-changed-p--hash-table
 (defun file-has-changed-p (file &optional tag)
   "Return non-nil if FILE has changed.
 The size and modification time of FILE are compared to the size
-and modification time of tghe same FILE during a previous
+and modification time of the same FILE during a previous
 invocation of `file-has-changed-p'.  Thus, the first invocation
-of `file-has-changed-p' always returns non-nil.
+of `file-has-changed-p' on a given FILE always returns non-nil.
 The optional argument TAG, which must be a symbol, can be used to
 limit the comparison to invocations with identical tags; it can be
 the symbol of the calling function, for example."
   (let* ((fileattr (file-attributes file 'integer))
 	 (attr (cons (file-attribute-size fileattr)
 		     (file-attribute-modification-time fileattr)))
-	 (sym (concat (symbol-name tag) "@" file))
+	 (sym (concat (symbol-name tag) "@"
+                      (if (file-name-absolute-p file)
+                          file
+                        (expand-file-name file))))
 	 (cachedattr (gethash sym file-has-changed-p--hash-table)))
      (when (not (equal attr cachedattr))
        (puthash sym attr file-has-changed-p--hash-table))))
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Eli Zaretskii]

> Date: Wed, 03 Nov 2021 13:27:31 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, 51523@debbugs.gnu.org, larsi@gnus.org
> 
> > You don't need to expand the file name you pass to file-attributes: it 
> > does that internally.  What you need is to expand the file name you pass 
> > as the key, i.e. for the symbol you are generating from the file name.
> 
> Whoops, sorry, that's what I meant to do of course, I went too fast. 

Yes, that's better, thanks.

Do we care that (file-name-absolute-p "~/foo") => t ?

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Do we care that (file-name-absolute-p "~/foo") => t ?

It's not a problem.  Just like the fact of not using `file-truename` that
means we will occasionally consider a file as changed even tho
it hasn't.


        Stefan

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> -	 (sym (concat (symbol-name tag) "@" file))
> +	 (sym (concat (symbol-name tag) "@"
> +                      (if (file-name-absolute-p file)
> +                          file
> +                        (expand-file-name file))))

    (cons tag (if (file-name-absolute-p file) file (expand-file-name file)))

is more efficient.


        Stefan

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Gregory Heytings]

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


>> -	 (sym (concat (symbol-name tag) "@" file))
>> +	 (sym (concat (symbol-name tag) "@"
>> +                      (if (file-name-absolute-p file)
>> +                          file
>> +                        (expand-file-name file))))
>
> (cons tag (if (file-name-absolute-p file) file (expand-file-name file)))
>
> is more efficient.
>

I don't see a performance impact in my tests, but I'll trust the master.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Further-improvement-of-file-has-changed-p.patch, Size: 1759 bytes --]

From ea3b76d3a7ae0d72be1239fa1fcf00f7d55710e7 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 3 Nov 2021 15:17:32 +0000
Subject: [PATCH] Further improvement of file-has-changed-p

* lisp/files.el (file-has-changed-p): Expand the file name when necessary.
Use cons instead of concat.  Clarify and fix typo in the docstring.
---
 lisp/files.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 173198a424..ad814ed3d3 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6187,16 +6187,18 @@ file-has-changed-p--hash-table
 (defun file-has-changed-p (file &optional tag)
   "Return non-nil if FILE has changed.
 The size and modification time of FILE are compared to the size
-and modification time of tghe same FILE during a previous
+and modification time of the same FILE during a previous
 invocation of `file-has-changed-p'.  Thus, the first invocation
-of `file-has-changed-p' always returns non-nil.
+of `file-has-changed-p' on a given FILE always returns non-nil.
 The optional argument TAG, which must be a symbol, can be used to
 limit the comparison to invocations with identical tags; it can be
 the symbol of the calling function, for example."
   (let* ((fileattr (file-attributes file 'integer))
 	 (attr (cons (file-attribute-size fileattr)
 		     (file-attribute-modification-time fileattr)))
-	 (sym (concat (symbol-name tag) "@" file))
+	 (sym (cons tag (if (file-name-absolute-p file)
+			    file
+			  (expand-file-name file))))
 	 (cachedattr (gethash sym file-has-changed-p--hash-table)))
      (when (not (equal attr cachedattr))
        (puthash sym attr file-has-changed-p--hash-table))))
-- 
2.33.0

bug#51523: 29.0.50; gnus-mime-view-part-externally very slow

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> -	 (sym (concat (symbol-name tag) "@" file))
>>> +	 (sym (concat (symbol-name tag) "@"
>>> +                      (if (file-name-absolute-p file)
>>> +                          file
>>> +                        (expand-file-name file))))
>> (cons tag (if (file-name-absolute-p file) file (expand-file-name file)))
>> is more efficient.
> I don't see a performance impact in my tests, but I'll trust the master.

It's likely lost in the noise, indeed, but `cons` just allocates
a single 2-word object, whereas your concat will allocate a Lisp_String
object (4-word object) plus the actual byte array (of N+M+1 bytes).
[ And if you're unlucky and one of the strings has text-properties
  applied to it, then you get a fair bit more since the interval tree
  then needs to be copied (and merged if both strings have
  properties).  ]

It also gives you cleaner behavior, since it avoids the possibility of
freak collisions when tag or file contains `@`.  IOW, it should be the
natural choice.


        Stefan