bug#58302: 29.0.50; browse-url-emacs is extremely slow (and I think always has been?)

[Phil Sainty]

On 2022-10-07 15:47, Phil Sainty wrote:
>  (or not-serious (sit-for 1 t))

With that commented out, I tried to do some profiling like this:

(progn
  (profiler-start 'cpu)
  (browse-url-emacs "http://www.example.com";)
  (profiler-report)
  (profiler-stop)
  (profiler-reset)
  (kill-buffer "www.example.com"))

The results were perplexing in their variability -- all I can
suggest is that you run that code multiple times, and C-u RET
to expand the full profile after each run, and see whether you
also observe a variety of fairly different outcomes.

Here's one example where we can see `url-retrieve-synchronously'
being called 4 times; but other times it was called 2-3 times,
and the profile looked rather different.

          23  69%         - browse-url-emacs
          23  69%          - find-file-other-window
          23  69%           - find-file-noselect
          17  51%            - find-file-noselect-1
           8  24%             - after-find-file
           8  24%              - if
           4  12%               - let*
           4  12%                - cond
           4  12%                 - and
           4  12%                  - file-exists-p
           4  12%                   - url-file-handler
           4  12%                    - apply
           4  12%                     - url-file-exists-p
           4  12%                      - url-http-file-exists-p
           4  12%                       - url-http-head
           4  12%                        - url-retrieve-synchronously
           4  12%                         - accept-process-output
           4  12%                          - url-http-generic-filter
           4  12%                           - 
url-http-wait-for-headers-change-function
           4  12%                              mail-fetch-field
           4  12%               - run-hooks
           4  12%                - vc-refresh-state
           4  12%                 - vc-backend
           4  12%                  - vc-file-getprop
           4  12%                   - expand-file-name
           4  12%                      url-file-handler
           6  18%             - insert-file-contents
           6  18%              - url-file-handler
           6  18%               - apply
           6  18%                - url-insert-file-contents
           4  12%                   url-retrieve-synchronously
           2   6%                 - url-insert-buffer-contents
           2   6%                  - url-insert
           2   6%                   - mm-dissect-buffer
           2   6%                    - mm-dissect-singlepart
           2   6%                     - mm-copy-to-buffer
           2   6%                        generate-new-buffer
           3   9%             - file-readable-p
           3   9%              - url-file-handler
           3   9%               - apply
           3   9%                - url-file-exists-p
           3   9%                 - url-http-file-exists-p
           3   9%                  - url-http-head
           3   9%                   - url-retrieve-synchronously
           3   9%                    - url-retrieve
           3   9%                     - url-retrieve-internal
           3   9%                        url-http
           6  18%            - file-attributes
           6  18%             - url-file-handler
           6  18%              - apply
           6  18%               - url-file-attributes
           6  18%                - url-http-file-attributes
           6  18%                 - url-http-head-file-attributes
           6  18%                  - url-http-head
           6  18%                   - url-retrieve-synchronously
           6  18%                    - url-retrieve
           6  18%                     - url-retrieve-internal
           6  18%                      - url-http
           6  18%                         generate-new-buffer
          10  30%    Automatic GC



I'm not very familiar with the ins and outs of these code paths,
but my first impression is that we've initiated an operation which
needs to deal with a particular URL and if we were to make a high-
level binding to indicate that we were doing this, we could then
cache and re-use the results of those network requests for the
extent of that binding.

3 of the 4 `url-retrieve-synchronously' calls above are from
`url-http-head'; twice on account of `url-file-exists-p', and another
from `url-file-attributes'.

I see the following in the code:

(defun url-http-head (url)
  (let ((url-request-method "HEAD")
        (url-request-data nil))
    (url-retrieve-synchronously url)))

(defun url-http-file-exists-p (url)
  (let ((buffer (url-http-head url)))
    ...))

(defalias 'url-http-file-readable-p 'url-http-file-exists-p)

(defun url-http-head-file-attributes (url &optional _id-format)
  (let ((buffer (url-http-head url)))
    ...))

(defun url-http-file-attributes (url &optional id-format)
  (if (url-dav-supported-p url)
      (url-dav-file-attributes url id-format)
    (url-http-head-file-attributes url id-format)))


In principle, I don't see why we couldn't be re-using the buffer
returned by the first call `url-http-head' in each of the
subsequent calls.

Furthermore, we could *probably* flag the fact that we are 100%
intending to request the entire file later on in the command,
and use that information to just do a GET request instead of a
HEAD request in the first place -- the resulting buffer for which
can then *also* be re-used by the eventual `url-insert-file-contents'
call.

I think `url-http-head' itself should only ever do a HEAD request,
but `url-http-head-file-attributes' and `url-http-file-exists-p'
could conditionally use the full GET buffer.


-Phil