~protesilaos/denote

8 2

Slow down when Editing buffers with links

Details
Message ID
<87k05umyyo.fsf@prevos.net>
DKIM signature
pass
Download raw message
I am noticing a delay in typing when editing a buffer in which a 
paragraph has multiple Denote links. I suspect that is because 
Denote checks the validity of the link for fontification.

Is there a way to disable the link checking on the fly?

Regards

P:)

-- 
Dr Peter Prevos
---------------
peterprevos.com
Details
Message ID
<87tu4yab6g.fsf@protesilaos.com>
In-Reply-To
<87k05umyyo.fsf@prevos.net> (view parent)
DKIM signature
pass
Download raw message
> From: Peter Prevos <peter@prevos.net>
> Date: Fri, 23 Sep 2022 15:36:39 +1000

Hello Peter,

> I am noticing a delay in typing when editing a buffer in which a 
> paragraph has multiple Denote links. I suspect that is because 
> Denote checks the validity of the link for fontification.
>
> Is there a way to disable the link checking on the fly?

I don't think we have such fine-grained control over this feature.  It
is Org which decides to fontify links this way.  All we can do, in terms
of what Org exposes for custom links, is to register the link type:

    (org-link-set-parameters
      "denote"
      :follow #'denote-link-ol-follow
      :face #'denote-link-ol-face
      :complete #'denote-link-ol-complete
      :store #'denote-link-ol-store
      :export #'denote-link-ol-export)

Since this is problematic, my idea is to either (i) figure out what Org
can do about delays in fontification more generally, or (ii) remove the
parts concerning the 'denote-link-ol-face'.  Regarding the latter, it is
nice to have fontification that shows broken links, but it is not worth
a performance penalty.

The doc string of 'org-link-parameters' gives us this:

    ‘:activate-func’

      Function to run at the end of Font Lock activation.  It must
      accept four arguments:
      - the buffer position at the start of the link,
      - the buffer position at its end,
      - the path, as a string,
      - a boolean, non-nil when the link has brackets.

In theory, we could have a function with a timer, but I feel this is
just asking for more bugs down the line.  I am reluctant to touch that,
unless for expert in Org internals shares their insights on the matter.

What do you or others think?

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<87wn9ul91w.fsf@prevos.net>
In-Reply-To
<87tu4yab6g.fsf@protesilaos.com> (view parent)
DKIM signature
pass
Download raw message
Hi Prot,

Org Mode naively does not check for the status of a link, it just 
fontifies them.

Link checking is usually a function that is run as part of regular 
maintenance. Perhaps a variable can decide whether link-checking 
is enabled, as such:

(defcustom denote-check-broken-link t
  "Whether to check for broken links."
  :group 'denote
  :type 'boolean)

(defun denote-link-ol-face (link)
  (if (not (denote--current-file-is-note-p))
      'link
    (if (not denote-check-broken-link)
	'denote-faces-link
      (if (denote-link--ol-resolve-link-to-target link)
          'denote-faces-link
	'denote-faces-broken-link))))



regards

P:)


Protesilaos Stavrou <info@protesilaos.com> writes:

>> From: Peter Prevos <peter@prevos.net>
>> Date: Fri, 23 Sep 2022 15:36:39 +1000
>
> Hello Peter,
>
>> I am noticing a delay in typing when editing a buffer in which 
>> a 
>> paragraph has multiple Denote links. I suspect that is because 
>> Denote checks the validity of the link for fontification.
>>
>> Is there a way to disable the link checking on the fly?
>
> I don't think we have such fine-grained control over this 
> feature.  It
> is Org which decides to fontify links this way.  All we can do, 
> in terms
> of what Org exposes for custom links, is to register the link 
> type:
>
>     (org-link-set-parameters
>       "denote"
>       :follow #'denote-link-ol-follow
>       :face #'denote-link-ol-face
>       :complete #'denote-link-ol-complete
>       :store #'denote-link-ol-store
>       :export #'denote-link-ol-export)
>
> Since this is problematic, my idea is to either (i) figure out 
> what Org
> can do about delays in fontification more generally, or (ii) 
> remove the
> parts concerning the 'denote-link-ol-face'.  Regarding the 
> latter, it is
> nice to have fontification that shows broken links, but it is 
> not worth
> a performance penalty.
>
> The doc string of 'org-link-parameters' gives us this:
>
>     ‘:activate-func’
>
>       Function to run at the end of Font Lock activation.  It 
>       must
>       accept four arguments:
>       - the buffer position at the start of the link,
>       - the buffer position at its end,
>       - the path, as a string,
>       - a boolean, non-nil when the link has brackets.
>
> In theory, we could have a function with a timer, but I feel 
> this is
> just asking for more bugs down the line.  I am reluctant to 
> touch that,
> unless for expert in Org internals shares their insights on the 
> matter.
>
> What do you or others think?
>
> All the best,
> Prot


-- 
Dr Peter Prevos
---------------
peterprevos.com
Details
Message ID
<87zgeqs71t.fsf@protesilaos.com>
In-Reply-To
<87wn9ul91w.fsf@prevos.net> (view parent)
DKIM signature
pass
Download raw message
> From: Peter Prevos <peter@prevos.net>
> Date: Fri, 23 Sep 2022 19:22:58 +1000
>
>
> Hi Prot,

Hello Peter,

> Org Mode naively does not check for the status of a link, it just 
> fontifies them.
>
> Link checking is usually a function that is run as part of regular 
> maintenance. Perhaps a variable can decide whether link-checking 
> is enabled, as such:
>
> (defcustom denote-check-broken-link t
>   "Whether to check for broken links."
>   :group 'denote
>   :type 'boolean)
>
> (defun denote-link-ol-face (link)
>   (if (not (denote--current-file-is-note-p))
>       'link
>     (if (not denote-check-broken-link)
> 	'denote-faces-link
>       (if (denote-link--ol-resolve-link-to-target link)
>           'denote-faces-link
> 	'denote-faces-broken-link))))

I prefer not to add a user option prematurely.  This is a niche
functionality and it is better not to overwhelm the user with
customisations.

At this stage, I say we try to find other packages that have such a
check for the link's face.  We can then see how they are doing things
and try to learn from it.  Do you know of such a package?  I am running
a grep for 'org-link-set-parameters' in org.git but none of the results
use the :face parameter.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<87r10272lh.fsf@prevos.net>
In-Reply-To
<87zgeqs71t.fsf@protesilaos.com> (view parent)
DKIM signature
pass
Download raw message
Hi Prot,

org-ref uses fortification for dead links and that never caused me 
performance issues: https://github.com/jkitchin/org-ref. Citar has 
similar functionality.: 
https://github.com/emacs-citar/citar/blob/main/citar.el, but 
something misjudges link status.

The main difference would be that in citation software, the BibTeX 
files are cached).



Regards

P:)

Protesilaos Stavrou <info@protesilaos.com> writes:

>> From: Peter Prevos <peter@prevos.net>
>> Date: Fri, 23 Sep 2022 19:22:58 +1000
>>
>>
>> Hi Prot,
>
> Hello Peter,
>
>> Org Mode naively does not check for the status of a link, it 
>> just 
>> fontifies them.
>>
>> Link checking is usually a function that is run as part of 
>> regular 
>> maintenance. Perhaps a variable can decide whether 
>> link-checking 
>> is enabled, as such:
>>
>> (defcustom denote-check-broken-link t
>>   "Whether to check for broken links."
>>   :group 'denote
>>   :type 'boolean)
>>
>> (defun denote-link-ol-face (link)
>>   (if (not (denote--current-file-is-note-p))
>>       'link
>>     (if (not denote-check-broken-link)
>> 	'denote-faces-link
>>       (if (denote-link--ol-resolve-link-to-target link)
>>           'denote-faces-link
>> 	'denote-faces-broken-link))))
>
> I prefer not to add a user option prematurely.  This is a niche
> functionality and it is better not to overwhelm the user with
> customisations.
>
> At this stage, I say we try to find other packages that have 
> such a
> check for the link's face.  We can then see how they are doing 
> things
> and try to learn from it.  Do you know of such a package?  I am 
> running
> a grep for 'org-link-set-parameters' in org.git but none of the 
> results
> use the :face parameter.
>
> All the best,
> Prot


-- 
Dr Peter Prevos
---------------
peterprevos.com
Details
Message ID
<87tu4ys3pm.fsf@protesilaos.com>
In-Reply-To
<87r10272lh.fsf@prevos.net> (view parent)
DKIM signature
pass
Download raw message
> From: Peter Prevos <peter@prevos.net>
> Date: Fri, 23 Sep 2022 21:18:46 +1000
>
>
> Hi Prot,

Hello Peter,

> org-ref uses fortification for dead links and that never caused me
> performance issues: https://github.com/jkitchin/org-ref. Citar has
> similar functionality.:
> https://github.com/emacs-citar/citar/blob/main/citar.el, but something
> misjudges link status.

Thank you!  I searched org-ref and found this:

    (defcustom org-ref-activate-glossary-links t
      "If non-nil activate acronym and glossary links.
    Checks in `org-ref-glossary-face-fn' and `org-ref-acronym-face-fn'.
    This is not always fast, so we provide a way to disable it."
      :type 'boolean
      :group 'org-ref-glossary)

It is like the customisation option you suggested.  Notice the "not
always fast" part, which suggests the rationale for this variable's
introduction was to address the underlying problem we are now facing.

This is unfortunate.  It shows that package authors and users are left
to work around a fundamental problem with Org fontification and/or with
how the ':face' parameter of custom links behaves.  Before accepting the
status quo, I must ask:

- Do we really need this special fontification?
- Is it necessary for 'denote:' links to be aware of when they are broken?

> The main difference would be that in citation software, the BibTeX 
> files are cached).

This is an important difference.  We have no cache.  Perhaps we should,
but that is a design we need to think carefully about.  How would we do
it?  What it would be used for?  This sort of consideration.  If done
properly, it is a nice extra.  Though it cannot be a dependency for core
functionality.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<87fsgh7ms1.fsf@prevos.net>
In-Reply-To
<87tu4ys3pm.fsf@protesilaos.com> (view parent)
DKIM signature
pass
Download raw message
Hi Prot,

In my view, fontification for broken links as a default is 
unnecessary. Org mode itself has no such functionality.

Could it be a minor mode? Or perhaps an auditing function? In the 
same vain, I have been playing around with a function that audits 
all denotes and synchronises frontmatter with the filename and 
functions to audit tag names (correct typos, improve consistency).

P.S. The Emacs copyright assignment is finally sorted.

P:)





Protesilaos Stavrou <info@protesilaos.com> writes:

>> From: Peter Prevos <peter@prevos.net>
>> Date: Fri, 23 Sep 2022 21:18:46 +1000
>>
>>
>> Hi Prot,
>
> Hello Peter,
>
>> org-ref uses fortification for dead links and that never caused 
>> me
>> performance issues: https://github.com/jkitchin/org-ref. Citar 
>> has
>> similar functionality.:
>> https://github.com/emacs-citar/citar/blob/main/citar.el, but 
>> something
>> misjudges link status.
>
> Thank you!  I searched org-ref and found this:
>
>     (defcustom org-ref-activate-glossary-links t
>       "If non-nil activate acronym and glossary links.
>     Checks in `org-ref-glossary-face-fn' and 
>     `org-ref-acronym-face-fn'.
>     This is not always fast, so we provide a way to disable it."
>       :type 'boolean
>       :group 'org-ref-glossary)
>
> It is like the customisation option you suggested.  Notice the 
> "not
> always fast" part, which suggests the rationale for this 
> variable's
> introduction was to address the underlying problem we are now 
> facing.
>
> This is unfortunate.  It shows that package authors and users 
> are left
> to work around a fundamental problem with Org fontification 
> and/or with
> how the ':face' parameter of custom links behaves.  Before 
> accepting the
> status quo, I must ask:
>
> - Do we really need this special fontification?
> - Is it necessary for 'denote:' links to be aware of when they 
> are broken?
>
>> The main difference would be that in citation software, the 
>> BibTeX 
>> files are cached).
>
> This is an important difference.  We have no cache.  Perhaps we 
> should,
> but that is a design we need to think carefully about.  How 
> would we do
> it?  What it would be used for?  This sort of consideration.  If 
> done
> properly, it is a nice extra.  Though it cannot be a dependency 
> for core
> functionality.
>
> All the best,
> Prot


-- 
Dr Peter Prevos
---------------
peterprevos.com
Details
Message ID
<874jwx31sz.fsf@protesilaos.com>
In-Reply-To
<87fsgh7ms1.fsf@prevos.net> (view parent)
DKIM signature
pass
Download raw message
> From: Peter Prevos <peter@prevos.net>
> Date: Sat, 24 Sep 2022 08:18:41 +1000
>
>
> Hi Prot,

Hello Peter,

> In my view, fontification for broken links as a default is 
> unnecessary. Org mode itself has no such functionality.

Indeed.  Same for most custom link types I searched for.  The idea of a
custom face that is applied conditionally is not bad.  The problem is
that the function we use to determine the face is called too often.

> Could it be a minor mode? Or perhaps an auditing function?

If I understand your questions correctly, such functionality requires
that we interfere with Org's fontification methods.  This is outside the
scope of Denote, but even if we were to try, we would likely find more
complications down the line.  Those would not be worth the trouble of
adding a different face.

I think the only way to salvage this feature is to introduce the
variable, as you suggested.  Users who want broken links to be fontified
differently can then opt in.  We shall warn them that this can lead to
a performance penalty.

Given that we already have the feature, that is the most conservative
step we can take.  Though I am currently leaning towards removing it
altogether and using the 'denote-faces-link' unconditionally.  By
default, this looks the same as all other links.

The reason why I think this way is that the variable we would introduce
would not be useful to most users.  Those who care about editing
performance will not opt in.  Those who are unsure will be hesitant to
opt in after they read our warning.  Those who do opt in anyway are
likely to never see a broken link, given that it is fairly difficult to
break them.

> In the same vain, I have been playing around with a function that
> audits all denotes and synchronises frontmatter with the filename and
> functions to audit tag names (correct typos, improve consistency).

Sounds promising!  Looking forward to learn more about it.

> P.S. The Emacs copyright assignment is finally sorted.

Good news!

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<87sfke7qz2.fsf@protesilaos.com>
In-Reply-To
<874jwx31sz.fsf@protesilaos.com> (view parent)
DKIM signature
pass
Download raw message
> From: Protesilaos Stavrou <info@protesilaos.com>
> Date: Sat, 24 Sep 2022 06:10:20 +0300

> [... 31 lines elided]

> Though I am currently leaning towards removing it altogether and using
> the 'denote-faces-link' unconditionally.  By default, this looks the
> same as all other links.
>
> The reason why I think this way is that the variable we would introduce
> would not be useful to most users.  Those who care about editing
> performance will not opt in.  Those who are unsure will be hesitant to
> opt in after they read our warning.  Those who do opt in anyway are
> likely to never see a broken link, given that it is fairly difficult to
> break them.

Hello Peter,

I have removed the 'denote-faces-broken-link' in commit 6374751.  I
think it is better this way than trying to salvage it in a way that
wouldn't fix the underlying problem.

    commit 6374751ee20be95637dcf5eefd52c0da321158a6
    Author: Protesilaos Stavrou <info@protesilaos.com>
    Date:   Mon Sep 26 06:24:05 2022 +0300

        REMOVE 'denote-faces-broken-link'

        It leads to performance issues in Org under certain circumstances.
        Fundamentally, this has to do with the fontification mechanism, which
        would call the 'denote-link-ol-face' function too often.

        Thanks to Peter Prevos for reporting the issue and discussing it with
        me: <https://lists.sr.ht/~protesilaos/denote/%3C87k05umyyo.fsf%40prevos.net%3E>.

     README.org |  7 -------
     denote.el  | 21 ++-------------------
     2 files changed, 2 insertions(+), 26 deletions(-)

As I wrote before, I am not against the idea of a conditional face.  I
liked it from the start.  If we find a better way, I am ready to
reinstate this feature.

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Reply to thread Export thread (mbox)