Rendering regression in Gnus with gnus-treat-from-gravatar

Rendering regression in Gnus with gnus-treat-from-gravatar

[Adam Sjøgren via "Emacs development discussions.]

A recent change in Emacs has made rendering of articles in Gnus on my
machine noticably slower: uncolored headers flash before the article is
shown in the final rendering.

I have bisected the problem, and this seems to be the cause:

  421eeff243af683bf0b7c6d9181650a1c6900f9b is the first bad commit
  commit 421eeff243af683bf0b7c6d9181650a1c6900f9b
  Author: Philip K <philip@warpmail.net>
  Date:   Tue Mar 17 15:29:53 2020 +0100

      Add support for multiple Gravatar services

      Now supports Libravatar and Unicornify, next to Gravatar (Bug#39965).

      * lisp/image/gravatar.el (gravatar-base-url): Remove constant.
      (gravatar-service-alist): List supported services.
      (gravatar-service): Add user option to specify service, defaults to
      Libravatar.
      (gravatar--service-libravatar): New function, libravatar image host
      resolver implementation.
      (gravatar-build-url): Use alist gravatar-service-alist instead of
      gravatar-base-url.
      * etc/NEWS: Mention new gravatar service option.

   etc/NEWS               |  6 ++++++
   lisp/image/gravatar.el | 43 +++++++++++++++++++++++++++++++++++++++----
   2 files changed, 45 insertions(+), 4 deletions(-)

This makes the option "gnus-treat-from-gravatar 'head" unpleasant. Here
is a video showing the effect:

 · https://koldfront.dk/misc/gnus/avatar-lag.mp4 (4.1 MB)

And here is the "before" video:

 · https://koldfront.dk/misc/gnus/avatar-normal.mp4 (5.5 MB)


  Best regards,

    Adam

--
 "Hur långt man än har kommit                               Adam Sjøgren
  Är det alltid längre kvar"                           asjo@koldfront.dk

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Sat, 11 Apr 2020 15:18:51 +0200, "Adam Sjøgren via \"Emacs development discussions." <emacs-devel@gnu.org> said:

    Adam> A recent change in Emacs has made rendering of articles in Gnus on my
    Adam> machine noticably slower: uncolored headers flash before the article is
    Adam> shown in the final rendering.

    Adam> I have bisected the problem, and this seems to be the cause:

    Adam>   421eeff243af683bf0b7c6d9181650a1c6900f9b is the first bad commit
    Adam>   commit 421eeff243af683bf0b7c6d9181650a1c6900f9b
    Adam>   Author: Philip K <philip@warpmail.net>
    Adam>   Date:   Tue Mar 17 15:29:53 2020 +0100

    Adam>       Add support for multiple Gravatar services

    Adam>       Now supports Libravatar and Unicornify, next to Gravatar (Bug#39965).

The new version doesnʼt do any more gravatar lookups than the old one,
but it does by default do some DNS lookups. Is it better if you
customize 'gravatar-service' to one of the other options? If so, Iʼll
have to pay some attention to Philip K's caching patch for this
feature.

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Adam Sjøgren]

Robert writes:

> The new version doesnʼt do any more gravatar lookups than the old one,
> but it does by default do some DNS lookups. Is it better if you
> customize 'gravatar-service' to one of the other options?

Of the three possible options for gravatar-service: gravatar,
unicornify, libravatar only the last, and default, libravatar exhibits
the problem.


  Best regards,

    Adam

-- 
 "Most regrets I've gathered came from caving in            Adam Sjøgren
  Darkness has a way of shedding light on me           asjo@koldfront.dk
  Seems the more I'm lost the more I'm feeling free"

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Sat, 11 Apr 2020 17:16:52 +0200, Adam Sjøgren <asjo@koldfront.dk> said:

    Adam> Robert writes:
    >> The new version doesnʼt do any more gravatar lookups than the old one,
    >> but it does by default do some DNS lookups. Is it better if you
    >> customize 'gravatar-service' to one of the other options?

    Adam> Of the three possible options for gravatar-service: gravatar,
    Adam> unicornify, libravatar only the last, and default, libravatar exhibits
    Adam> the problem.

OK, so itʼs the DNS. The libravatar method does a bunch of DNS
queries, and those can be slow, and they're done for every article.

I guess we need to see if we can make those DNS lookups asynchronous.

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Sat, 11 Apr 2020 18:53:38 +0200, Robert Pluim <rpluim@gmail.com> said:

    Robert> OK, so itʼs the DNS. The libravatar method does a bunch of DNS
    Robert> queries, and those can be slow, and they're done for every article.

    Robert> I guess we need to see if we can make those DNS lookups asynchronous.

The following does at least make rendering of the article complete
straight away, although you might end up waiting a little longer for
the gravatar to actually appear. Iʼm assuming your emacs has thread
support.

(the inhibit-redisplay is there because otherwise this crashes on
macOS, where apparently you can only call certain OS window system
functions from the main thread).

diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
index ff59a72ac8..b9ad16bb28 100644
--- a/lisp/image/gravatar.el
+++ b/lisp/image/gravatar.el
@@ -149,7 +149,7 @@ gravatar--service-libravatar
           (dolist (record '(("_avatars-sec" . "https")
                             ("_avatars" . "http")))
             (let* ((query (concat (car record) "._tcp." domain))
-                   (result (dns-query query 'SRV)))
+                   (result (dns-query-cached query 'SRV)))
               (when result
                 (throw 'found (format "%s://%s/avatar"
                                       (cdr record)
@@ -189,17 +189,41 @@ gravatar-get-data
          (search-forward "\n\n" nil t)
          (buffer-substring (point) (point-max)))))
 
-;;;###autoload
-(defun gravatar-retrieve (mail-address callback &optional cbargs)
+(defvar gravatar-retrieve-params nil)
+(defvar gravatar-thread nil)
+(defvar gravatar-exit-thread nil)
+(defvar gravatar-mutex nil)
+(defvar gravatar-cond-var nil)
+
+(defun gravatar-retrieve-thread ()
   "Asynchronously retrieve a gravatar for MAIL-ADDRESS.
 When finished, call CALLBACK as (apply CALLBACK GRAVATAR CBARGS),
 where GRAVATAR is either an image descriptor, or the symbol
 `error' if the retrieval failed."
-  (let ((url (gravatar-build-url mail-address)))
-    (if (url-cache-expired url gravatar-cache-ttl)
-        (url-retrieve url #'gravatar-retrieved (list callback cbargs) t)
-      (with-current-buffer (url-fetch-from-cache url)
-        (gravatar-retrieved () callback cbargs)))))
+  (while (not gravatar-exit-thread)
+    (with-mutex gravatar-mutex
+      (condition-wait gravatar-cond-var)
+      (let ((inhibit-redisplay t)
+            (mail-address (nth 0 gravatar-retrieve-params))
+            (callback (nth 1 gravatar-retrieve-params))
+            (cbargs (nth 2 gravatar-retrieve-params)))
+        (let ((url (gravatar-build-url mail-address)))
+          (if (url-cache-expired url gravatar-cache-ttl)
+              (url-retrieve url #'gravatar-retrieved (list callback cbargs) t)
+            (with-current-buffer (url-fetch-from-cache url)
+              (gravatar-retrieved () callback cbargs)))))
+      (thread-yield))))
+
+;;;###autoload
+(defun gravatar-retrieve (mail-address callback &optional cbargs)
+  (unless gravatar-thread
+    (setq gravatar-mutex (make-mutex)
+          gravatar-cond-var (make-condition-variable gravatar-mutex)
+          gravatar-thread (make-thread #'gravatar-retrieve-thread)))
+  (with-mutex gravatar-mutex
+    (setq gravatar-retrieve-params
+          (list mail-address callback cbargs))
+    (condition-notify gravatar-cond-var)))
 
 ;;;###autoload
 (defun gravatar-retrieve-synchronously (mail-address)

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Tue, 14 Apr 2020 15:14:11 +0200
> Cc: ding@gnus.org, emacs-devel@gnu.org
> 
> (the inhibit-redisplay is there because otherwise this crashes on
> macOS, where apparently you can only call certain OS window system
> functions from the main thread).

Please be sure to have a comment about this in the final version of
the code.

Thanks.

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Adam Sjøgren]

Robert writes:

> The following does at least make rendering of the article complete
> straight away, although you might end up waiting a little longer for
> the gravatar to actually appear. Iʼm assuming your emacs has thread
> support.

Works nicely for me - thanks!

(The patch didn't apply to my source tree - git master - so I put the
changes in by hand to test.)


  Best regards,

    Adam

-- 
 "Darkness has a way of shedding light on me                Adam Sjøgren
  Seems the more I'm lost the more I think I'm free"   asjo@koldfront.dk

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Tue, 14 Apr 2020 19:32:39 +0200, Adam Sjøgren <asjo@koldfront.dk> said:

    Adam> Robert writes:
    >> The following does at least make rendering of the article complete
    >> straight away, although you might end up waiting a little longer for
    >> the gravatar to actually appear. Iʼm assuming your emacs has thread
    >> support.

    Adam> Works nicely for me - thanks!

    Adam> (The patch didn't apply to my source tree - git master - so I put the
    Adam> changes in by hand to test.)

Thanks for testing.

Eli, I guess I need to conditionalize this on (featurep 'threads) in
master?

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Wed, 15 Apr 2020 10:15:04 +0200
> Cc: ding@gnus.org, emacs-devel@gnu.org
> 
> Eli, I guess I need to conditionalize this on (featurep 'threads) in
> master?

Yes.

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 15 Apr 2020 12:41:39 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Wed, 15 Apr 2020 10:15:04 +0200
    >> Cc: ding@gnus.org, emacs-devel@gnu.org
    >> 
    >> Eli, I guess I need to conditionalize this on (featurep 'threads) in
    >> master?

    Eli> Yes.

OK, I can do that. As an aside, emacs threading seems very
unresponsive, in that it appears to take a very long time after the
thread has been started before it reacts to the signaling of the
condition variable. I was assuming I didnʼt need to explicitly yield
from the main thread after that, but perhaps I do?

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: asjo@koldfront.dk,  ding@gnus.org,  emacs-devel@gnu.org
> Date: Wed, 15 Apr 2020 12:13:24 +0200
> 
> As an aside, emacs threading seems very unresponsive, in that it
> appears to take a very long time after the thread has been started
> before it reacts to the signaling of the condition variable. I was
> assuming I didnʼt need to explicitly yield from the main thread
> after that, but perhaps I do?

The thread will not run until the main thread yields in some way.
What does the main thread do after calling make-thread?

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 15 Apr 2020 14:38:15 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: asjo@koldfront.dk,  ding@gnus.org,  emacs-devel@gnu.org
    >> Date: Wed, 15 Apr 2020 12:13:24 +0200
    >> 
    >> As an aside, emacs threading seems very unresponsive, in that it
    >> appears to take a very long time after the thread has been started
    >> before it reacts to the signaling of the condition variable. I was
    >> assuming I didnʼt need to explicitly yield from the main thread
    >> after that, but perhaps I do?

    Eli> The thread will not run until the main thread yields in some way.
    Eli> What does the main thread do after calling make-thread?

    (with-mutex gravatar-mutex
      (setq gravatar-retrieve-params
            (list mail-address callback cbargs))
      (condition-notify gravatar-cond-var))
    (thread-yield)

where the gravatar thread is doing 'condition-wait' on that same
condition var. Except that it then fires off an asynchronous url
request, so perhaps thatʼs the slow bit. Or itʼs the interaction with
redisplay thatʼs slow, I should perhaps test on a platform where
redisplay is not limited to the main thread.

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: asjo@koldfront.dk,  ding@gnus.org,  emacs-devel@gnu.org
> Date: Wed, 15 Apr 2020 13:55:59 +0200
> 
>     Eli> The thread will not run until the main thread yields in some way.
>     Eli> What does the main thread do after calling make-thread?
> 
>     (with-mutex gravatar-mutex
>       (setq gravatar-retrieve-params
>             (list mail-address callback cbargs))
>       (condition-notify gravatar-cond-var))
>     (thread-yield)
> 
> where the gravatar thread is doing 'condition-wait' on that same
> condition var. Except that it then fires off an asynchronous url
> request, so perhaps thatʼs the slow bit.

You could put a breakpoint before and after the URL request, and then
see which part takes time.

> Or itʼs the interaction with redisplay thatʼs slow

What interaction is that?

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 15 Apr 2020 15:01:49 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> where the gravatar thread is doing 'condition-wait' on that same
    >> condition var. Except that it then fires off an asynchronous url
    >> request, so perhaps thatʼs the slow bit.

    Eli> You could put a breakpoint before and after the URL request, and then
    Eli> see which part takes time.

Iʼll try that.

    >> Or itʼs the interaction with redisplay thatʼs slow

    Eli> What interaction is that?

The url request fetches some image data, and inserts that in the Gnus
article buffer. It won't get shown until redisplay runs, which means
the main thread has to run. Itʼs not yet clear to me from which thread
that insertion eventually gets done, maybe some more calls to
thread-yield are necessary.

(this would all be easier if emacs threading weren't cooperative, but
baby steps)

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: asjo@koldfront.dk,  ding@gnus.org,  emacs-devel@gnu.org
> Date: Wed, 15 Apr 2020 15:54:09 +0200
> 
>     >> Or itʼs the interaction with redisplay thatʼs slow
> 
>     Eli> What interaction is that?
> 
> The url request fetches some image data, and inserts that in the Gnus
> article buffer. It won't get shown until redisplay runs, which means
> the main thread has to run.

Assuming that the non-main thread exits once it's done fetching,
that's not different from a single-threaded fetching, is it?  IOW,
redisplay won't kick in until the code which fetches the images
finishes.

> Itʼs not yet clear to me from which thread that insertion eventually
> gets done

The one that fetches the stuff, I suppose.  If not, how will the
fetched stuff get passed to the main thread for insertion?

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 15 Apr 2020 17:12:00 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: asjo@koldfront.dk,  ding@gnus.org,  emacs-devel@gnu.org
    >> Date: Wed, 15 Apr 2020 15:54:09 +0200
    >> 
    >> >> Or itʼs the interaction with redisplay thatʼs slow
    >> 
    Eli> What interaction is that?
    >> 
    >> The url request fetches some image data, and inserts that in the Gnus
    >> article buffer. It won't get shown until redisplay runs, which means
    >> the main thread has to run.

    Eli> Assuming that the non-main thread exits once it's done fetching,
    Eli> that's not different from a single-threaded fetching, is it?  IOW,
    Eli> redisplay won't kick in until the code which fetches the images
    Eli> finishes.

True.

    >> Itʼs not yet clear to me from which thread that insertion eventually
    >> gets done

    Eli> The one that fetches the stuff, I suppose.  If not, how will the
    Eli> fetched stuff get passed to the main thread for insertion?

url-retrieve doesnʼt actually fetch anything, it adds a fetching
request to a work-queue, which then somehow actually causes a
fetch. I think I have a lot more code to read before I understand
everything thatʼs going on :-)

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Tassilo Horn]

Robert Pluim <rpluim@gmail.com> writes:

Hi Robert and Adam,

>     Adam> Robert writes:
>     >> The new version doesnʼt do any more gravatar lookups than the
>     >> old one, but it does by default do some DNS lookups. Is it
>     >> better if you customize 'gravatar-service' to one of the other
>     >> options?
>
>     Adam> Of the three possible options for gravatar-service:
>     Adam> gravatar, unicornify, libravatar only the last, and default,
>     Adam> libravatar exhibits the problem.
>
> OK, so itʼs the DNS. The libravatar method does a bunch of DNS
> queries, and those can be slow, and they're done for every article.

I have the same problem as Adam.  And it happened two or three times
that upon clicking a message, Emacs started consuming 100% CPU and stuck
at least 30 seconds before I hit C-g.  At one occurrence, I attached GDB
after waiting at least 30 seconds for the full message to appear and on
the bottom of the call stack was

  dns-query
  accept-process-output

Hm, that must be that while loop in dns-query but given the values of
step and times, I cannot see how that could block for so long.  And
usually, it doesn't.

And in the ususal case, it is just slow in the sense of maybe 2-5
seconds.  And in those normal cases, it's not so much the dns query but
the TLS negotiation with seccdn.libravatar.org (in case the DNS lookup
found no custom gravatar url).

Here's a profiler report gathered with:

(progn
  (profiler-start 'cpu)
  (gravatar-retrieve-synchronously "tsdh@gnu.org")
  (profiler-report)
  (profiler-stop))

--8<---------------cut here---------------start------------->8---
- command-execute                                                  33 100%
 - call-interactively                                              33 100%
  - funcall-interactively                                          33 100%
   - eval-last-sexp                                                33 100%
    - elisp--eval-last-sexp                                        33 100%
     - eval                                                        33 100%
      - progn                                                      33 100%
       - progn                                                     33 100%
        - gravatar-retrieve-synchronously                          32  96%
         - let                                                     32  96%
          - save-current-buffer                                    25  75%
           - set-buffer                                            25  75%
            - if                                                   25  75%
             - url-retrieve-synchronously                          25  75%
              - url-retrieve                                       24  72%
               - url-retrieve-internal                             24  72%
                - url-https                                        24  72%
                 - url-http                                        24  72%
                  - url-http-find-free-connection                  24  72%
                   - url-open-stream                               24  72%
                    - open-network-stream                          24  72%
                     - network-stream-open-tls                     24  72%
                      - open-gnutls-stream                         24  72%
                       - gnutls-negotiate                          24  72%
                        - gnutls-boot-parameters                    1   3%
                         - gnutls-trustfiles                        1   3%
                          - gnutls--get-files                       1   3%
                             file-expand-wildcards                  1   3%
              + accept-process-output                               1   3%
          - gravatar-build-url                                      7  21%
           - format                                                 7  21%
            - funcall                                               7  21%
             - gravatar--service-libravatar                         7  21%
              - let                                                 7  21%
               - unwind-protect                                     7  21%
                - progn                                             7  21%
                 - if                                               7  21%
                  - let                                             7  21%
                   - catch                                          7  21%
                    - let                                           7  21%
                     - while                                        7  21%
                      - let                                         7  21%
                       - let*                                       7  21%
                        - dns-query                                 7  21%
                         - sit-for                                  6  18%
                            redisplay                               3   9%
                            read-event                              3   9%
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Tassilo Horn]

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

Tassilo Horn <tsdh@gnu.org> writes:

>> OK, so itʼs the DNS. The libravatar method does a bunch of DNS
>> queries, and those can be slow, and they're done for every article.
>
> I have the same problem as Adam.  And it happened two or three times
> that upon clicking a message, Emacs started consuming 100% CPU and
> stuck at least 30 seconds before I hit C-g.  At one occurrence, I
> attached GDB after waiting at least 30 seconds for the full message to
> appear and on the bottom of the call stack was
>
>   dns-query
>   accept-process-output

It just happened again, and this time I made a full C and Lisp
backtrace (attached).

[-- Attachment #2: backtrace.txt.gz --]
[-- Type: application/gzip, Size: 9322 bytes --]

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


I guess I should make a new bug report for that, right?

Bye,
Tassilo

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 22 Apr 2020 16:39:02 +0200, Tassilo Horn <tsdh@gnu.org> said:

    Tassilo> Tassilo Horn <tsdh@gnu.org> writes:
    >>> OK, so itʼs the DNS. The libravatar method does a bunch of DNS
    >>> queries, and those can be slow, and they're done for every article.
    >> 
    >> I have the same problem as Adam.  And it happened two or three times
    >> that upon clicking a message, Emacs started consuming 100% CPU and
    >> stuck at least 30 seconds before I hit C-g.  At one occurrence, I
    >> attached GDB after waiting at least 30 seconds for the full message to
    >> appear and on the bottom of the call stack was
    >> 
    >> dns-query
    >> accept-process-output

    Tassilo> It just happened again, and this time I made a full C and Lisp
    Tassilo> backtrace (attached).



    Tassilo> I guess I should make a new bug report for that, right?

Yes please.

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 22 Apr 2020 16:25:34 +0200, Tassilo Horn <tsdh@gnu.org> said:

    Tassilo> I have the same problem as Adam.  And it happened two or three times
    Tassilo> that upon clicking a message, Emacs started consuming 100% CPU and stuck
    Tassilo> at least 30 seconds before I hit C-g.  At one occurrence, I attached GDB
    Tassilo> after waiting at least 30 seconds for the full message to appear and on
    Tassilo> the bottom of the call stack was

    Tassilo>   dns-query
    Tassilo>   accept-process-output

    Tassilo> Hm, that must be that while loop in dns-query but given the values of
    Tassilo> step and times, I cannot see how that could block for so long.  And
    Tassilo> usually, it doesn't.

Iʼve seen it take 5 seconds with slow DNS just to do the DNS
lookup. There might be multiple issues here.

    Tassilo> And in the ususal case, it is just slow in the sense of maybe 2-5
    Tassilo> seconds.  And in those normal cases, it's not so much the dns query but
    Tassilo> the TLS negotiation with seccdn.libravatar.org (in case the DNS lookup
    Tassilo> found no custom gravatar url).

    Tassilo> Here's a profiler report gathered with:

    Tassilo> (progn
    Tassilo>   (profiler-start 'cpu)
    Tassilo>   (gravatar-retrieve-synchronously "tsdh@gnu.org")
    Tassilo>   (profiler-report)
    Tassilo>   (profiler-stop))

gravatar does those retrieves asynchronously, but that won't change
the call graph, I donʼt think.

Would it make sense to

1. Use dns-query-cached
2. Switch gravatar to use http rather than https by default, with a
user option to use https (Iʼm assuming this speeds things up)
3. Apply the gravater caching patch from Philip K

I suspect [2] there would give the biggest improvement, given your
profile report.

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Tassilo Horn]

Robert Pluim <rpluim@gmail.com> writes:

Hi Robert,

>     Tassilo> And in the ususal case, it is just slow in the sense of
>     Tassilo> maybe 2-5 seconds.  And in those normal cases, it's not
>     Tassilo> so much the dns query but the TLS negotiation with
>     Tassilo> seccdn.libravatar.org (in case the DNS lookup found no
>     Tassilo> custom gravatar url).
>
>     Tassilo> Here's a profiler report gathered with:
>
>     Tassilo> (progn
>     Tassilo>   (profiler-start 'cpu)
>     Tassilo>   (gravatar-retrieve-synchronously "tsdh@gnu.org")
>     Tassilo>   (profiler-report)
>     Tassilo>   (profiler-stop))
>
> gravatar does those retrieves asynchronously, but that won't change
> the call graph, I donʼt think.
>
> Would it make sense to
>
> 1. Use dns-query-cached

I guess so as it does the lookups on every message and every mail
address where it would suffice to do it once per unique domain part of
mail addresses.

> 2. Switch gravatar to use http rather than https by default, with a
> user option to use https (Iʼm assuming this speeds things up)

Indeed, I've tried using "http://cdn.libravatar.org/avatar" which was
way faster.  Do you know what might slow down TLS negotiation so much?
Now when I think about it, I'd also say that accessing GNU ELPA for
package upgrades became slower, too.  But browsing the web seems to work
normally but I think Firefox or Epiphany might not use gnutls...

> 3. Apply the gravater caching patch from Philip K
>
> I suspect [2] there would give the biggest improvement, given your
> profile report.

Maybe.  But if I understood your reply to Adam on ding, only the DNS
lookups block seeing the complete Gnus article, right?  The retrieval is
asynchronous anyway which just means the gravatar images might take some
time to pop up in the article buffer.  If that is true, then I'd only
care about the DNS part.

Bye,
Tassilo

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Robert Pluim]

>>>>> On Wed, 22 Apr 2020 19:01:42 +0200, Tassilo Horn <tsdh@gnu.org> said:

    >> Would it make sense to
    >> 
    >> 1. Use dns-query-cached

    Tassilo> I guess so as it does the lookups on every message and every mail
    Tassilo> address where it would suffice to do it once per unique domain part of
    Tassilo> mail addresses.

Right. And dns-query-cached remembers 'nil' returns from the DNS
lookup as well. I guess I should push that change soon.

diff --git a/lisp/image/gravatar.el b/lisp/image/gravatar.el
index ff59a72ac8..8a73959996 100644
--- a/lisp/image/gravatar.el
+++ b/lisp/image/gravatar.el
@@ -149,7 +149,7 @@ gravatar--service-libravatar
           (dolist (record '(("_avatars-sec" . "https")
                             ("_avatars" . "http")))
             (let* ((query (concat (car record) "._tcp." domain))
-                   (result (dns-query query 'SRV)))
+                   (result (dns-query-cached query 'SRV)))
               (when result
                 (throw 'found (format "%s://%s/avatar"
                                       (cdr record)

    >> 2. Switch gravatar to use http rather than https by default, with a
    >> user option to use https (Iʼm assuming this speeds things up)

    Tassilo> Indeed, I've tried using "http://cdn.libravatar.org/avatar" which was
    Tassilo> way faster.  Do you know what might slow down TLS negotiation so much?
    Tassilo> Now when I think about it, I'd also say that accessing GNU ELPA for
    Tassilo> package upgrades became slower, too.  But browsing the web seems to work
    Tassilo> normally but I think Firefox or Epiphany might not use gnutls...

Nothing has changed in that area recently, although Bug#40665 has
uncovered some interesting corner cases that weʼre working
on. Covid-19 effect maybe.

    >> 3. Apply the gravater caching patch from Philip K
    >> 
    >> I suspect [2] there would give the biggest improvement, given your
    >> profile report.

    Tassilo> Maybe.  But if I understood your reply to Adam on ding, only the DNS
    Tassilo> lookups block seeing the complete Gnus article, right?  The retrieval is
    Tassilo> asynchronous anyway which just means the gravatar images might take some
    Tassilo> time to pop up in the article buffer.  If that is true, then I'd only
    Tassilo> care about the DNS part.

Yes, the retrieval is asynchronous, DNS isn't. Iʼve made some attempts
to get the DNS lookups be done in a separate thread, but they've not
been very successful. And now that I look closely, url-retrieve does
caching as well, so hopefully with extended use the slowdowns will go
away.

Robert

Re: Rendering regression in Gnus with gnus-treat-from-gravatar

[Tassilo Horn]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Wed, 22 Apr 2020 16:39:02 +0200, Tassilo Horn <tsdh@gnu.org> said:
>     >> And it happened two or three times that upon clicking a
>     >> message, Emacs started consuming 100% CPU and stuck at least 30
>     >> seconds before I hit C-g.  At one occurrence, I attached GDB
>     >> after waiting at least 30 seconds for the full message to
>     >> appear and on the bottom of the call stack was
>     >> 
>     >> dns-query
>     >> accept-process-output
>
>     Tassilo> It just happened again, and this time I made a full C and Lisp
>     Tassilo> backtrace (attached).
>     Tassilo> I guess I should make a new bug report for that, right?
>
> Yes please.

It's bug#40775.

Bye,
Tassilo