Org-mode mailing list
 help / color / mirror / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Ihor Radchenko <yantar92@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Date: Fri, 05 Jun 2020 15:50:30 +0200
Message-ID: <87ftb9pqop.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87mu5iq618.fsf@localhost> (Ihor Radchenko's message of "Fri, 05 Jun 2020 16:18:59 +0800")

Ihor Radchenko <yantar92@gmail.com> writes:

>> See also `gensym'. Do we really need to use it for something else than
>> `invisible'? If not, the tool doesn't need to be generic.
>
> For now, I also use it for buffer-local 'invisible stack. The stack is
> needed to preserve folding state of drawers/blocks inside folded
> outline. Though I am thinking about replacing the stack with separate
> text properties, like 'invisible-outline-buffer-local +
> 'invisible-drawer-buffer-local + 'invisible-block-buffer-local.
> Maintaining stack takes a noticeable percentage of CPU time in profiler.
>
> org--get-buffer-local-text-property-symbol must take care about
> situation with indirect buffers. When an indirect buffer is created from
> some org buffer, the old value of char-property-alias-alist is carried
> over. We need to detect this case and create new buffer-local symbol,
> which is unique to the newly created buffer (but not create it if the
> buffer-local property is already there). Then, the new symbol must
> replace the old alias in char-property-alias-alist + old folding state
> must be preserved (via copying the old invisibility specs into the new
> buffer-local text property). I do not see how gensym can benefit this
> logic.

`gensym' is just a shorter, and somewhat standard way, to create a new
uninterned symbol with a given prefix. You seem to re-invent it. What
you do with that new symbol is orthogonal to that suggestion, of course.

>> OK, but this may not be sufficient if we want to do slightly better than
>> overlays in that area. This is not mandatory, though.
>
> Could you elaborate on what can be "slightly better"? 

IIRC, I gave examples of finer control of folding state after a change.
Consider this _folded_ drawer:

  :BEGIN:
  Foo
  :END:

Inserting ":END" in it should not unfold it, as it is currently the case
with overlays,

  :BEGIN
  Foo
  :END
  :END:

but a soon as the last ":" is inserted, the initial drawer could be
expanded.

  :BEGIN
  Foo
  :END:
  :END:

The latter case is not currently handled by overlays. This is what
I call "slightly better".

Also, note that this change is not related to opening and closing lines
of the initial drawer, so sticking text properties on them would not
help here.

Another case is modifying those borders, e.g.,


  :BEGIN:               :BEGIN:
  Foo         ------>   Foo  
  :END:                 :ND:

which should expand the drawer. Your implementation catches this, but
I'm pointing out that current implementation with overlays does not.
Even though that's not strictly required for compatibility with
overlays, it is a welcome slight improvement.

>> As discussed before, I don't think you need to use `modification-hooks'
>> or `insert-behind-hooks' if you already use `after-change-functions'.
>>
>> `after-change-functions' are also triggered upon text properties
>> changes. So, what is the use case for the other hooks?
>
> The problem is that `after-change-functions' cannot be a text property.
> Only `modification-hooks' and `insert-in-front/behind-hooks' can be a
> valid text property. If we use `after-change-functions', they will
> always be triggered, regardless if the change was made inside or outside
> folded region.

As discussed, text properties are local to the change, but require extra
care when moving text around. You also observed serious overhead when
using them.

OTOH, even if `a-c-f' is not local, you can quickly determine if the
change altered a folded element, so the overhead is limited, i.e.,
mostly checking for a text property at a given buffer position.

To be clear, I initially thought that text properties were a superior
choice, but I changed my mind a while ago, and I thought you had, too.
IOW, `after-change-functions' is the way to go, since you have no strong
reason to stick to text properties for this kind of function.

>>> :asd:
>>> :drawer:
>>> lksjdfksdfjl
>>> sdfsdfsdf
>>> :end:
>>>
>>> If :asd: was inserted in front of folded :drawer:, changes in :drawer:
>>> line of the new folded :asd: drawer would reveal the text between
>>> :drawer: and :end:.
>>>
>>> Let me know what you think on this.
>
>> I have first to understand the use case for `modification-hook'. But
>> I think unfolding is the right thing to do in this situation, isn't it?
>
> That situation arises because the modification-hooks from ":drawer:"
> (they are set via text properties) only have information about the
> :drawer:...:end: drawer before the modifications (they were set when
> :drawer: was folded last time). So, they will only unfold a part of the
> new :asd: drawer. I do not see a simple way to unfold everything without
> re-parsing the drawer around the changed text.

Oh! I misread your message. I withdraw what I wrote. In this case, we
don't want to unfold anything. The situation is not worse than what we
have now, and trying to fix it would have repercussions down in the
buffer, e.g., expanding drawers screen below.

As a rule of thumb, I think we can pay attention to changes in the
folded text, and its immediate surroundings (e.g., the opening line,
which is not folded), but no further.

As written above, slight changes are welcome, but let's not go overboard
and parse a whole section just to know if we can expand a drawer.

> Actually, I am quite unhappy with the performance of modification-hooks
> set via text properties (I am using this patch on my Emacs during this
> week). It appears that setting the text properties costs a significant
> CPU time in practice, even though running the hooks is pretty fast.
> I will think about a way to handle modifications using global
> after-change-functions.

That's better, IMO.

I gave you a few ideas to quickly check if a change requires expansion,
in an earlier mail. I suggest to start out from that. Let me know if you
have questions about it.


  reply	other threads:[~2020-06-05 13:51 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24  6:55 Ihor Radchenko
2020-04-24  8:02 ` Nicolas Goaziou
2020-04-25  0:29   ` stardiviner
2020-04-26 16:04   ` Ihor Radchenko
2020-05-04 16:56     ` Karl Voit
2020-05-07  7:18       ` Karl Voit
2020-05-09 15:43       ` Ihor Radchenko
2020-05-07 11:04     ` Christian Heinrich
2020-05-09 15:46       ` Ihor Radchenko
2020-05-08 16:38     ` Nicolas Goaziou
2020-05-09 13:58       ` Nicolas Goaziou
2020-05-09 16:22         ` Ihor Radchenko
2020-05-09 17:21           ` Nicolas Goaziou
2020-05-10  5:25             ` Ihor Radchenko
2020-05-10  9:47               ` Nicolas Goaziou
2020-05-10 13:29                 ` Ihor Radchenko
2020-05-10 14:46                   ` Nicolas Goaziou
2020-05-10 16:21                     ` Ihor Radchenko
2020-05-10 16:38                       ` Nicolas Goaziou
2020-05-10 17:08                         ` Ihor Radchenko
2020-05-10 19:38                           ` Nicolas Goaziou
2020-05-09 15:40       ` Ihor Radchenko
2020-05-09 16:30         ` Ihor Radchenko
2020-05-09 17:32           ` Nicolas Goaziou
2020-05-09 18:06             ` Ihor Radchenko
2020-05-10 14:59               ` Nicolas Goaziou
2020-05-10 15:15                 ` Kyle Meyer
2020-05-10 16:30                 ` Ihor Radchenko
2020-05-10 19:32                   ` Nicolas Goaziou
2020-05-12 10:03                     ` Nicolas Goaziou
2020-05-17 15:00                     ` Ihor Radchenko
2020-05-17 15:40                       ` Ihor Radchenko
2020-05-18 14:35                         ` Nicolas Goaziou
2020-05-18 16:52                           ` Ihor Radchenko
2020-05-19 13:07                             ` Nicolas Goaziou
2020-05-23 13:52                               ` Ihor Radchenko
2020-05-23 13:53                                 ` Ihor Radchenko
2020-05-23 15:26                                   ` Ihor Radchenko
2020-05-26  8:33                                 ` Nicolas Goaziou
2020-06-02  9:21                                   ` Ihor Radchenko
2020-06-02  9:23                                     ` Ihor Radchenko
2020-06-02 12:10                                       ` Bastien
2020-06-02 13:12                                         ` Ihor Radchenko
2020-06-02 13:23                                           ` Bastien
2020-06-02 13:30                                             ` Ihor Radchenko
2020-06-02  9:25                                     ` Ihor Radchenko
2020-06-05  7:26                                     ` Nicolas Goaziou
2020-06-05  8:18                                       ` Ihor Radchenko
2020-06-05 13:50                                         ` Nicolas Goaziou [this message]
2020-06-08  5:05                                           ` Ihor Radchenko
2020-06-08  5:06                                             ` Ihor Radchenko
2020-06-08  5:08                                             ` Ihor Radchenko
2020-06-10 17:14                                             ` Nicolas Goaziou
2020-06-21  9:52                                               ` Ihor Radchenko
2020-06-21 15:01                                                 ` Nicolas Goaziou
2020-08-11  6:45                                               ` Ihor Radchenko
2020-08-11 23:07                                                 ` Kyle Meyer
2020-08-12  6:29                                                   ` Ihor Radchenko
2020-09-20  5:53                                                     ` Ihor Radchenko
2020-09-20 11:45                                                       ` Kévin Le Gouguec
2020-09-22  9:05                                                         ` Ihor Radchenko
2020-09-22 10:00                                                           ` Ihor Radchenko
2020-09-23  6:16                                                             ` Kévin Le Gouguec
2020-09-23  6:48                                                               ` Ihor Radchenko
2020-09-23  7:09                                                                 ` Bastien
2020-09-23  7:30                                                                   ` Ihor Radchenko
2020-09-24 18:07                                                                 ` Kévin Le Gouguec
2020-09-25  2:16                                                                   ` Ihor Radchenko
2020-12-15 17:38                                                                     ` [9.4] Fixing logbook visibility during isearch Kévin Le Gouguec
2020-12-16  3:15                                                                       ` Ihor Radchenko
2020-12-16 18:05                                                                         ` Kévin Le Gouguec
2020-12-17  3:18                                                                           ` Ihor Radchenko
2020-12-17 14:50                                                                             ` Kévin Le Gouguec
2020-12-18  2:23                                                                               ` Ihor Radchenko
2020-12-24 23:37                                                                                 ` Kévin Le Gouguec
2020-12-25  2:51                                                                                   ` Ihor Radchenko
2020-12-25 10:59                                                                                     ` Kévin Le Gouguec
2020-12-25 12:32                                                                                       ` Ihor Radchenko
2020-12-25 21:35                                                                                     ` Kévin Le Gouguec
2020-12-26  4:14                                                                                       ` Ihor Radchenko
2020-12-26 11:44                                                                                         ` Kévin Le Gouguec
2020-12-26 12:22                                                                                           ` Ihor Radchenko
2020-12-04  5:58                                                       ` [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers Ihor Radchenko
2021-03-21  9:09                                                         ` Ihor Radchenko
2021-05-03 17:28                                                           ` Bastien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://orgmode.org

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ftb9pqop.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=emacs-orgmode@gnu.org \
    --cc=yantar92@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Org-mode mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://orgmode.org/list/0 list/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 list list/ https://orgmode.org/list \
		emacs-orgmode@gnu.org
	public-inbox-index list

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.orgmode
	nntp://news.gmane.io/gmane.emacs.orgmode


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git