Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Matt Price writes: > Is this code in main now, and do I have to do anything special to test it > out? Not on main yet. I need maintainers to agree about the merge. It is a major change. I plan to prepare a proper patchset and bump the thread again a few weeks later, when the dust settles on the recent org-persist/org-element merge. If you want to test the code, it is available at https://github.com/yantar92/org You can simply clone the github repo and load Org from there. Best, Ihor
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
On Tue, Sep 21, 2021 at 9:36 AM Timothy wrote: > I’m suspect it too short notice for such a large change to make its way > into Org > 9.5, but Bastien’s release email is certainly a good prompt to bump this. > > Bastien writes: > > > Thank you *very much* for this work and sorry for the slow reply. > > > > I urge everyone to test this change, as I’d like to include it in > > Org 9.5 if it’s ready. > > > > I will test this myself this week and report. > > All the best, > Timothy > Is this code in main now, and do I have to do anything special to test it out?
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
I’m suspect it too short notice for such a large change to make its way into Org 9.5, but Bastien’s release email is certainly a good prompt to bump this. Bastien writes: > Thank you *very much* for this work and sorry for the slow reply. > > I urge everyone to test this change, as I’d like to include it in > Org 9.5 if it’s ready. > > I will test this myself this week and report. All the best, Timothy
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi Ihor, Ihor Radchenko writes: > This is another update about the status of the patch. Thank you *very much* for this work and sorry for the slow reply. I urge everyone to test this change, as I'd like to include it in Org 9.5 if it's ready. I will test this myself this week and report. Thanks! -- Bastien
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, This is another update about the status of the patch. I am mostly happy with the current state of the code, got rid of most of the bugs, and did not get any new bug reports in github for a while. I would like to start the process of applying the patch on master. As a first step, I would like to submit the core folding library (org-fold-core) for review. org-fold-core is pretty much independent from org-mode code base and does not affect anything if applied as is. It will be used by org-specific org-fold.el I will finalise and send later. For now, I would like to hear any suggestions about API and implementation of org-fold-core.el. I tried to document all the details in the code. Looking forward for the feedback. Best, Ihor org-fold-core.el Description: application/emacs-lisp
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, This is an update about the current status of the patch. Since there was not much feedback, I decided to share the up-to-date branch on github, so that people can directly download/clone the whole thing and load it to Emacs without a need to install the patch manually. The github repo is https://github.com/yantar92/org On the progress with the code, I have found many more bugs, which are not critical for me, but should be fixed anyway. I will keep working on them and keep the github repo up to date. One more important thing I wanted to mention is about the way org-fold should be merged on master. I plan to support using overlays within org-fold depending on custom variable. If the variable is set to 'overlay, org fold will use overlays without all the complexity of text property approach. The 'overlay value will be set by default. If a user wants to use text properties, the variable can be customised. The described approach will allow all the users test the text property-based folding as experimental feature (similar to org-element-use-cache). Once we are confident enough that the code is stable, we can just change the default. What do you think? Best, Ihor Ihor Radchenko writes: > Hello, > >> There are still known problems though. The patch currently breaks many >> org-mode tests when running =make test=. It is partially because some >> tests assume overlays to be used for folding and partially because the >> patch appears to break certain folding conventions. I am still >> investigating this (and learning =ert=). > > All the tests are passing now. > The current version of the patch (against master) is in > https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef > > The patch is stable on my system for last several months. There are > still some minor issues here and there, but it is getting harder for me > to find any problems by myself. I need help from interested users to > review and/or test the patch. > > Best, > Ihor
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> I understand from your answer to Bastien's query that this fix is > specific to your branch; would it be hard to backport it to Org's maint > branch? Otherwise IIUC Org 9.4 will keep this regression, and users > will have to wait until Org 9.5 for a fix. The problem is that fix in my branch has nothing to do with main branch. The bugs were inherently different even though looked same from user point of view. If one wants to make the fix work on master, the whole branch must be applied. However, I can try to suggest a way to fix the issue on master. The way isearch handles folded text in org is set from org-flag-region (org-macs.el): (overlay-put o 'isearch-open-invisible (lambda ( _) (org-show-context 'isearch))) It means that isearch calls org-show-context (org.el) to reveal hidden text. Then, it calls org-show-set-visibility with argument defined in org-show-context-detail (now, it is 'lineage). With current defaults, the searched text is revealed using org-flag-heading, which reveals both heading body and drawers. The easiest way to write the fix would be changing org-flag-heading directly, but there might be unforeseen consequences on other folding commands. Another way would be changing the way org-show-set-visibility handles 'lineage argument. Again, it may affect other things. Finally, one can add an extra possible argument to org-show-set-visibility and alter default value of org-show-context-detail accordingly. The last way will have least risk to break something else. I guess, patches welcome ;) > Bastien asked for the /gist/ as a patch against master, whereas your > answer explained why you couldn't share the /fix/ as a patch against > master. If Bastien did mean the whole gist, here is the corresponding > patch against master: Well. The gist is a patch applying the whole feature/org-fold branch to master. That's not yet something we can do. The plan is to apply the org-fold feature in several steps, as discussed in earlier messages. So, I thought that it would just create confusion if I share the gist as is. Sorry if I was not clear. Best, Ihor Kévin Le Gouguec writes: > Ihor Radchenko writes: > >> Thanks for reporting! I accidentally reintroduced the bug because of >> mistake when converting org-hide-drawers to new folding library. >> (:facepalm:). >> >> Should be fixed in the gist now. > > Can confirm, thanks! > > I understand from your answer to Bastien's query that this fix is > specific to your branch; would it be hard to backport it to Org's maint > branch? Otherwise IIUC Org 9.4 will keep this regression, and users > will have to wait until Org 9.5 for a fix. > > Also, just in case there's been a misunderstanding: > > Bastien writes: > >> Can you share this gist as a patch against Org's current master? > > Bastien asked for the /gist/ as a patch against master, whereas your > answer explained why you couldn't share the /fix/ as a patch against > master. If Bastien did mean the whole gist, here is the corresponding > patch against master: > > https://gist.githubusercontent.com/yantar92/6447754415457927293acda43a7fcaef/raw/7e43948e6c21220661534b79770bc1a6784b7893/featuredrawertextprop.patch > > Apologies if I'm the one misunderstanding, and thank you for all your > efforts!
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > Thanks for reporting! I accidentally reintroduced the bug because of > mistake when converting org-hide-drawers to new folding library. > (:facepalm:). > > Should be fixed in the gist now. Can confirm, thanks! I understand from your answer to Bastien's query that this fix is specific to your branch; would it be hard to backport it to Org's maint branch? Otherwise IIUC Org 9.4 will keep this regression, and users will have to wait until Org 9.5 for a fix. Also, just in case there's been a misunderstanding: Bastien writes: > Can you share this gist as a patch against Org's current master? Bastien asked for the /gist/ as a patch against master, whereas your answer explained why you couldn't share the /fix/ as a patch against master. If Bastien did mean the whole gist, here is the corresponding patch against master: https://gist.githubusercontent.com/yantar92/6447754415457927293acda43a7fcaef/raw/7e43948e6c21220661534b79770bc1a6784b7893/featuredrawertextprop.patch Apologies if I'm the one misunderstanding, and thank you for all your efforts!
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Can you share this gist as a patch against Org's current master? That is not possible. The underlying reason of the bug in the patch is different from master. On master, the overlays for folded drawers and headlines are merged together - when folded headline is opened by isearch, everything is revealed. The fix would involve special logic re-hiding drawers when necessary. On the org-fold feature branch, the drawers and headlines are folded independently. The reason why the bug persisted was my mistake in org-hide-drawers - I skipped drawers inside folded headlines, even when the drawers themselves were not folded. In my case the fix was trivial - I replaced condition when to skip drawer at point: [any fold is present at point] -> [drawer fold is present at point] (org-fold-get-folding-spec) -> (org-fold-get-folding-spec (org-fold-get-folding-spec-for-element 'drawer)) So, the fix is only relevant to the whole org-fold branch. Best, Ihor Bastien writes: > Hi Ihor, > > Ihor Radchenko writes: > >> Thanks for reporting! I accidentally reintroduced the bug because of >> mistake when converting org-hide-drawers to new folding library. >> (:facepalm:). >> >> Should be fixed in the gist now. > > Can you share this gist as a patch against Org's current master? > > -- > Bastien
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi Ihor, Ihor Radchenko writes: > Thanks for reporting! I accidentally reintroduced the bug because of > mistake when converting org-hide-drawers to new folding library. > (:facepalm:). > > Should be fixed in the gist now. Can you share this gist as a patch against Org's current master? -- Bastien
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Apologies for maybe changing the subject, but earlier this summer you > mentioned[1] you were working on a patch to the folding system that > would fix an issue I have[2] with LOGBOOKs since 9.4. AFAICT the patch > you are sharing now does not fix that; is this issue still on your > radar? Thanks for reporting! I accidentally reintroduced the bug because of mistake when converting org-hide-drawers to new folding library. (:facepalm:). Should be fixed in the gist now. Best, Ihor Kévin Le Gouguec writes: > Ihor Radchenko writes: > then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't get a stacktrace. I'm guessing this is because this error (which IIUC originates from org-back-to-heading in org.el) is a user-error; however, if I change the function to raise a "regular error", then everything compiles fine… >>> >>> I suspect that you forgot to run =make clean= (to remove old untracked >>> .elc files). >> >> I was wrong. It was actually a problem with org-back-to-heading. Should >> be fixed now. > > Thanks! The new patch applies cleanly (to aea1109ef), and "make" runs > to completion. > > I have seen no obvious breakage so far; I'll make sure to report if > anything funny shows up. > > > Apologies for maybe changing the subject, but earlier this summer you > mentioned[1] you were working on a patch to the folding system that > would fix an issue I have[2] with LOGBOOKs since 9.4. AFAICT the patch > you are sharing now does not fix that; is this issue still on your > radar? > > > At any rate, thank you for your work! > > > [1] https://orgmode.org/list/87r1ts3s8r.fsf@localhost/ > [2] https://orgmode.org/list/87eepuz0bj@gmail.com/ > > tl;dr even with #+STARTUP: overview, isearching opens all logbooks > near search results, even though there are no matches inside > logbooks themselves.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: >>> then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't >>> get a stacktrace. I'm guessing this is because this error (which IIUC >>> originates from org-back-to-heading in org.el) is a user-error; however, >>> if I change the function to raise a "regular error", then everything >>> compiles fine… >> >> I suspect that you forgot to run =make clean= (to remove old untracked >> .elc files). > > I was wrong. It was actually a problem with org-back-to-heading. Should > be fixed now. Thanks! The new patch applies cleanly (to aea1109ef), and "make" runs to completion. I have seen no obvious breakage so far; I'll make sure to report if anything funny shows up. Apologies for maybe changing the subject, but earlier this summer you mentioned[1] you were working on a patch to the folding system that would fix an issue I have[2] with LOGBOOKs since 9.4. AFAICT the patch you are sharing now does not fix that; is this issue still on your radar? At any rate, thank you for your work! [1] https://orgmode.org/list/87r1ts3s8r.fsf@localhost/ [2] https://orgmode.org/list/87eepuz0bj@gmail.com/ tl;dr even with #+STARTUP: overview, isearching opens all logbooks near search results, even though there are no matches inside logbooks themselves.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
>> then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't >> get a stacktrace. I'm guessing this is because this error (which IIUC >> originates from org-back-to-heading in org.el) is a user-error; however, >> if I change the function to raise a "regular error", then everything >> compiles fine… > > I suspect that you forgot to run =make clean= (to remove old untracked > .elc files). I was wrong. It was actually a problem with org-back-to-heading. Should be fixed now. Best, Ihor Ihor Radchenko writes: >> I get a conflict in org.el, on the hunk where org-reveal-location >> and org-show-context-detail are defined; since your patch just >> deletes them, I resolve this with: > > That's because the patch was against 0afef17e1. The new version of the > patch (same URL) is against aea1109ef now. > >> I've tried going to doc/, running >> >> emacs -Q --eval '(setq vc-handled-backends nil org-startup-folded nil)' \ >> --eval '(add-to-list '"'"'load-path "../lisp")' \ >> --eval '(load "../mk/org-fixup.el")' >> >> then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't >> get a stacktrace. I'm guessing this is because this error (which IIUC >> originates from org-back-to-heading in org.el) is a user-error; however, >> if I change the function to raise a "regular error", then everything >> compiles fine… > > I suspect that you forgot to run =make clean= (to remove old untracked > .elc files). > > Best, > Ihor > > Kévin Le Gouguec writes: > >> Hi! >> >> Ihor Radchenko writes: >> >>> The current version of the patch (against master) is in >>> https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef >> >> I'm probably missing something obvious, but when applying your patch on >> top of master[1], make fails when generating manuals: >> >>> emacs -Q -batch --eval '(setq vc-handled-backends nil org-startup-folded >>> nil)' \ >>> --eval '(add-to-list '"'"'load-path "../lisp")' \ >>> --eval '(load "../mk/org-fixup.el")' \ >>> --eval '(org-make-manuals)' >>> Loading >>> /home/peniblec/Downloads/sources/emacs-meta/org-mode/mk/org-fixup.el >>> (source)... >>> Before first headline at position 760959 in buffer org-manual.org<2> >>> make[1]: *** [Makefile:31: org.texi] Error 255 >> >> I've tried going to doc/, running >> >> emacs -Q --eval '(setq vc-handled-backends nil org-startup-folded nil)' \ >> --eval '(add-to-list '"'"'load-path "../lisp")' \ >> --eval '(load "../mk/org-fixup.el")' >> >> then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't >> get a stacktrace. I'm guessing this is because this error (which IIUC >> originates from org-back-to-heading in org.el) is a user-error; however, >> if I change the function to raise a "regular error", then everything >> compiles fine… >> >> >> [1] git apply --3way, on top of commit b64ba64fe. >> >> I get a conflict in org.el, on the hunk where org-reveal-location >> and org-show-context-detail are defined; since your patch just >> deletes them, I resolve this with: >> >> git checkout --theirs -- lisp/org.el
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> I get a conflict in org.el, on the hunk where org-reveal-location > and org-show-context-detail are defined; since your patch just > deletes them, I resolve this with: That's because the patch was against 0afef17e1. The new version of the patch (same URL) is against aea1109ef now. > I've tried going to doc/, running > > emacs -Q --eval '(setq vc-handled-backends nil org-startup-folded nil)' \ > --eval '(add-to-list '"'"'load-path "../lisp")' \ > --eval '(load "../mk/org-fixup.el")' > > then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't > get a stacktrace. I'm guessing this is because this error (which IIUC > originates from org-back-to-heading in org.el) is a user-error; however, > if I change the function to raise a "regular error", then everything > compiles fine… I suspect that you forgot to run =make clean= (to remove old untracked .elc files). Best, Ihor Kévin Le Gouguec writes: > Hi! > > Ihor Radchenko writes: > >> The current version of the patch (against master) is in >> https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef > > I'm probably missing something obvious, but when applying your patch on > top of master[1], make fails when generating manuals: > >> emacs -Q -batch --eval '(setq vc-handled-backends nil org-startup-folded >> nil)' \ >> --eval '(add-to-list '"'"'load-path "../lisp")' \ >> --eval '(load "../mk/org-fixup.el")' \ >> --eval '(org-make-manuals)' >> Loading /home/peniblec/Downloads/sources/emacs-meta/org-mode/mk/org-fixup.el >> (source)... >> Before first headline at position 760959 in buffer org-manual.org<2> >> make[1]: *** [Makefile:31: org.texi] Error 255 > > I've tried going to doc/, running > > emacs -Q --eval '(setq vc-handled-backends nil org-startup-folded nil)' \ > --eval '(add-to-list '"'"'load-path "../lisp")' \ > --eval '(load "../mk/org-fixup.el")' > > then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't > get a stacktrace. I'm guessing this is because this error (which IIUC > originates from org-back-to-heading in org.el) is a user-error; however, > if I change the function to raise a "regular error", then everything > compiles fine… > > > [1] git apply --3way, on top of commit b64ba64fe. > > I get a conflict in org.el, on the hunk where org-reveal-location > and org-show-context-detail are defined; since your patch just > deletes them, I resolve this with: > > git checkout --theirs -- lisp/org.el
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi! Ihor Radchenko writes: > The current version of the patch (against master) is in > https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef I'm probably missing something obvious, but when applying your patch on top of master[1], make fails when generating manuals: > emacs -Q -batch --eval '(setq vc-handled-backends nil org-startup-folded > nil)' \ > --eval '(add-to-list '"'"'load-path "../lisp")' \ > --eval '(load "../mk/org-fixup.el")' \ > --eval '(org-make-manuals)' > Loading /home/peniblec/Downloads/sources/emacs-meta/org-mode/mk/org-fixup.el > (source)... > Before first headline at position 760959 in buffer org-manual.org<2> > make[1]: *** [Makefile:31: org.texi] Error 255 I've tried going to doc/, running emacs -Q --eval '(setq vc-handled-backends nil org-startup-folded nil)' \ --eval '(add-to-list '"'"'load-path "../lisp")' \ --eval '(load "../mk/org-fixup.el")' then M-x toggle-debug-on-error and M-: (org-make-manuals), but I can't get a stacktrace. I'm guessing this is because this error (which IIUC originates from org-back-to-heading in org.el) is a user-error; however, if I change the function to raise a "regular error", then everything compiles fine… [1] git apply --3way, on top of commit b64ba64fe. I get a conflict in org.el, on the hunk where org-reveal-location and org-show-context-detail are defined; since your patch just deletes them, I resolve this with: git checkout --theirs -- lisp/org.el
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, > There are still known problems though. The patch currently breaks many > org-mode tests when running =make test=. It is partially because some > tests assume overlays to be used for folding and partially because the > patch appears to break certain folding conventions. I am still > investigating this (and learning =ert=). All the tests are passing now. The current version of the patch (against master) is in https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef The patch is stable on my system for last several months. There are still some minor issues here and there, but it is getting harder for me to find any problems by myself. I need help from interested users to review and/or test the patch. Best, Ihor Ihor Radchenko writes: 'outline --> `outline >>> >>> Could you explain why? >> >> Compatibility. pcase learned that in Emacs 25, IIRC. > > Thanks for the explanation. Fixed now in my local branch. > > I will send the updated version of the patch after more edits unless > someone specifically need to fix this change to make patch work on their > system. > > Best, > Ihor > > > Kyle Meyer writes: > >> Ihor Radchenko writes: >> 'outline --> `outline >>> >>> Could you explain why? >> >> Compatibility. pcase learned that in Emacs 25, IIRC.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
>>> 'outline --> `outline >> >> Could you explain why? > > Compatibility. pcase learned that in Emacs 25, IIRC. Thanks for the explanation. Fixed now in my local branch. I will send the updated version of the patch after more edits unless someone specifically need to fix this change to make patch work on their system. Best, Ihor Kyle Meyer writes: > Ihor Radchenko writes: > >>> 'outline --> `outline >> >> Could you explain why? > > Compatibility. pcase learned that in Emacs 25, IIRC.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: >> 'outline --> `outline > > Could you explain why? Compatibility. pcase learned that in Emacs 25, IIRC.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, [The patch itself will be provided in the following email or can be accessed via Github [1]] I have finally finished the suggested edits. Most importantly: - All the folding-related code lives in =org-fold.el= and =org-cycle.el= now. - =org-fold.el= have commentary section explaining how folding works and exposing API for external code using folding. - I wrote a patch for =isearch.el= adding support searching inside text hidden via text properties [2] and the relevant support of the patch in the =org-fold.el=. The current =isearch= behaviour is also supported. Hope the patch will go through eventually. The patch is fairly stable on my system. Any feedback or bug reports are welcome. There are still known problems though. The patch currently breaks many org-mode tests when running =make test=. It is partially because some tests assume overlays to be used for folding and partially because the patch appears to break certain folding conventions. I am still investigating this (and learning =ert=). More details: >> 2. The text property stack is rewritten using char-property-alias-alist. >>This is faster in comparison with previous approach, which involved >>modifying all the text properties every timer org-flag-region was >>called. > I'll need information about this, as I'm not sure to fully understand > all the consequences of this. But more importantly, this needs to be > copiously documented somewhere for future hackers. See commentary section in =org-fold.el= and comments in =org-fold--property-symbol-get-create=. > As a reminder, Org 9.4 is about to be released, but Org 9.5 will take > months to go out. So, even though I hope your changes will land into > Org, there is no reason for us to refrain from improving (actually > fixing a regression in) 9.4 release. Hopefully, once 9.4 is out, such > changes are not expected to happen anymore. > > I hope you understand. Probably my message sounded harsher than it should. I totally understand why such changes are needed, but wanted to make people aware that old folding implementation will be likely changed. > First, it includes a few unrelated changes that should be removed (e.g., > white space fixes in unrelated parts of the code). Also, as written > above, the changes about `org-custom-properties-hide-emptied-drawers' > should be removed for the time being. Let's leave this until the patch is ready to be pushed. I want to focus on handling bugs first without a need to check for the whitespace changes. > Once done, I think we should move (or copy, first) _all_ folding-related > functions into a new "org-fold.el" library. Functions and variables > included there should have a proper "org-fold-" prefix. More on this in > the detailed report. I decided to create =org-fold.el= and =org-cycle.el= and move all the relevant functions there. The org-cycle code seems to be so frequently used that I did not want to break the org-fold prefix to org-fold-cycle and decided to separate the cycle code into standalone file. > Then, another patch can integrate "org-fold.el" into Org folding. I also > suggest to move the Outline -> Org transition to yet another patch. > I think there's more work to do on this part. Agree. For the time being, I will still provide the full patch if anyone wants to test the whole thing on their system. > 1. we still support Emacs 24.4 (and probably Emacs 24.3, but I'm not >sure), so some functions cannot be used. I tried my best to cleanup the functions as you suggested, but I do not know a good way to check which functions are not supported by old Emacs versions. > 2. we don't use "subr-x.el" in the code base. In particular, it would be >nice to replace `when-let' with `when' + `let'. This change costs >only one loc. Done. > 3. Some docstrings need more work. In particular, Emacs documentation >expects all arguments to be explained in the docstring, if possible >in the order in which they appear. There are exceptions, though. For >example, in a function like `org-remove-text-properties', you can >mention arguments are simply the same as in `remove-text-properties'. Done. > 5. I didn't dive much into the Isearch code so far. I tested it a bit >and seems to work nicely. I noticed one bug though. In the following >document: > >#+begin: foo >:FOO: >bar >:END: >#+end >bar > >when both the drawer and the block are folded (i.e., you fold the >drawer first, then the block), searching for "bar" first find the >last one, then overwraps and find the first one. Fixed now. > 6. Since we're rewriting folding code, we might as well rename folding >properties: org-hide-drawer -> org-fold-drawer, outline -> >org-fold-headline… Done. See =org-fold-get-folding-spec-for-element=. >> +(defun org-remove-text-properties (start end properties object) > > IMO, this generic name doesn't match the specialized nature
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > I am currently working on org-fold.el. However, I am not sure if it is > acceptable to move some of the existing functions from org.el to > org-fold.el. > > Specifically, functions from the following sections of org.el might be > moved to org-fold.el: >> ;;; Visibility (headlines, blocks, drawers) >> Reveal point location >> Visibility cycling > > Should I do it? That makes sense, yes. Note that you can first copy and rename most functions to make the transition easier. As a second step, we can plug new functions into the main system. Regards, -- Nicolas Goaziou
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Once done, I think we should move (or copy, first) _all_ folding-related > functions into a new "org-fold.el" library. Functions and variables > included there should have a proper "org-fold-" prefix. More on this in > the detailed report. I am currently working on org-fold.el. However, I am not sure if it is acceptable to move some of the existing functions from org.el to org-fold.el. Specifically, functions from the following sections of org.el might be moved to org-fold.el: > ;;; Visibility (headlines, blocks, drawers) > Reveal point location > Visibility cycling Should I do it? Best, Ihor Nicolas Goaziou writes: > Hello, > > Ihor Radchenko writes: > >> [The patch itself will be provided in the following email] > > Thank you! I'll first make some generic remarks, then comment the diff > in more details. > >> I have four more updates from the previous version of the patch: >> >> 1. All the code handling modifications in folded drawers/blocks is moved >>to after-change-function. It works as follows: >>- if any text is inserted in the middle of hidden region, that text >> is also hidden; >>- if BEGIN/END line of a folded drawer do not match org-drawer-regexp >> and org-property-end-re, unfold it; >>- if org-property-end-re or new org-outline-regexp-bol is inserted in >> the middle of the drawer, unfold it; >>- the same logic for blocks. > > This sounds good, barring a minor error in the regexp for blocks, and > missing optimizations. More on this in the detailed comments. > >> 2. The text property stack is rewritten using char-property-alias-alist. >>This is faster in comparison with previous approach, which involved >>modifying all the text properties every timer org-flag-region was >>called. > > I'll need information about this, as I'm not sure to fully understand > all the consequences of this. But more importantly, this needs to be > copiously documented somewhere for future hackers. > >> 3. org-toggle-custom-properties-visibility is rewritten using text >>properties. I also took a freedom to implement a new feature here. >>Now, setting new `org-custom-properties-hide-emptied-drawers' to >>non-nil will result in hiding the whole property drawer if it >>contains only org-custom-properties. > > I don't think this is a good idea. AFAIR, we always refused to hide > completely anything, including empty drawers. The reason is that if the > drawer is completely hidden, you cannot expand it easily, or even know > there is one. > > In any case, this change shouldn't belong to this patch set, and should > be discussed separately. > >> 4. This patch should work against 1aa095ccf. However, the merge was not >>trivial here. Recent commits actively used the fact that drawers and >>outlines are hidden via 'outline invisibility spec, which is not the >>case in this branch. I am not confident that I did not break anything >>during the merge, especially 1aa095ccf. > > [...] > >> Also, I have seen some optimisations making use of the fact that drawers >> and headlines both use 'outline invisibility spec. This change in the >> implementation details supposed to improve performance and should not be >> necessary if this patch is going to be merged. Would it be possible to >> refrain from abusing this particular implementation detail in the >> nearest commits on master (unless really necessary)? > > To be clear, I didn't intend to make your life miserable. > > However, I had to fix regression on drawers visibility before Org 9.4 > release. Also, merging invisibility properties for drawers and outline > was easier for me. So, I had the opportunity to kill two birds with one > stone. > > As a reminder, Org 9.4 is about to be released, but Org 9.5 will take > months to go out. So, even though I hope your changes will land into > Org, there is no reason for us to refrain from improving (actually > fixing a regression in) 9.4 release. Hopefully, once 9.4 is out, such > changes are not expected to happen anymore. > > I hope you understand. > >> I would like to finalise the current patch and work on other code using >> overlays separately. This patch is already quite complicated as is. I do >> not want to introduce even more potential bugs by working on things not >> directly affected by this version of the patch. > > The patch is technically mostly good, but needs more work for > integration into Org. > > First, it includes a few unrelated changes that should be removed (e.g., > white space fixes in unrelated parts of the code). Also, as written > above, the changes about `org-custom-properties-hide-emptied-drawers' > should be removed for the time being. > > Once done, I think we should move (or copy, first) _all_ folding-related > functions into a new "org-fold.el" library. Functions and variables > included there should have a proper "org-fold-" prefix. More on this in > the detailed report. > > The functions
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > [The patch itself will be provided in the following email] Thank you! I'll first make some generic remarks, then comment the diff in more details. > I have four more updates from the previous version of the patch: > > 1. All the code handling modifications in folded drawers/blocks is moved >to after-change-function. It works as follows: >- if any text is inserted in the middle of hidden region, that text > is also hidden; >- if BEGIN/END line of a folded drawer do not match org-drawer-regexp > and org-property-end-re, unfold it; >- if org-property-end-re or new org-outline-regexp-bol is inserted in > the middle of the drawer, unfold it; >- the same logic for blocks. This sounds good, barring a minor error in the regexp for blocks, and missing optimizations. More on this in the detailed comments. > 2. The text property stack is rewritten using char-property-alias-alist. >This is faster in comparison with previous approach, which involved >modifying all the text properties every timer org-flag-region was >called. I'll need information about this, as I'm not sure to fully understand all the consequences of this. But more importantly, this needs to be copiously documented somewhere for future hackers. > 3. org-toggle-custom-properties-visibility is rewritten using text >properties. I also took a freedom to implement a new feature here. >Now, setting new `org-custom-properties-hide-emptied-drawers' to >non-nil will result in hiding the whole property drawer if it >contains only org-custom-properties. I don't think this is a good idea. AFAIR, we always refused to hide completely anything, including empty drawers. The reason is that if the drawer is completely hidden, you cannot expand it easily, or even know there is one. In any case, this change shouldn't belong to this patch set, and should be discussed separately. > 4. This patch should work against 1aa095ccf. However, the merge was not >trivial here. Recent commits actively used the fact that drawers and >outlines are hidden via 'outline invisibility spec, which is not the >case in this branch. I am not confident that I did not break anything >during the merge, especially 1aa095ccf. [...] > Also, I have seen some optimisations making use of the fact that drawers > and headlines both use 'outline invisibility spec. This change in the > implementation details supposed to improve performance and should not be > necessary if this patch is going to be merged. Would it be possible to > refrain from abusing this particular implementation detail in the > nearest commits on master (unless really necessary)? To be clear, I didn't intend to make your life miserable. However, I had to fix regression on drawers visibility before Org 9.4 release. Also, merging invisibility properties for drawers and outline was easier for me. So, I had the opportunity to kill two birds with one stone. As a reminder, Org 9.4 is about to be released, but Org 9.5 will take months to go out. So, even though I hope your changes will land into Org, there is no reason for us to refrain from improving (actually fixing a regression in) 9.4 release. Hopefully, once 9.4 is out, such changes are not expected to happen anymore. I hope you understand. > I would like to finalise the current patch and work on other code using > overlays separately. This patch is already quite complicated as is. I do > not want to introduce even more potential bugs by working on things not > directly affected by this version of the patch. The patch is technically mostly good, but needs more work for integration into Org. First, it includes a few unrelated changes that should be removed (e.g., white space fixes in unrelated parts of the code). Also, as written above, the changes about `org-custom-properties-hide-emptied-drawers' should be removed for the time being. Once done, I think we should move (or copy, first) _all_ folding-related functions into a new "org-fold.el" library. Functions and variables included there should have a proper "org-fold-" prefix. More on this in the detailed report. The functions `org-find-text-property-region', `org-add-to-list-text-property', and `org-remove-from-list-text-property' can be left in "org-macs.el", since they do not directly depend on the `invisible' property. Note the last two functions I mentioned are not used throughout your patch. They might be removed. This first patch can coexist with overlay folding since functions in both mechanisms are named differently. Then, another patch can integrate "org-fold.el" into Org folding. I also suggest to move the Outline -> Org transition to yet another patch. I think there's more work to do on this part. Now, into the details of your patch. The first remarks are: 1. we still support Emacs 24.4 (and probably Emacs 24.3, but I'm not sure), so some functions cannot be used. 2. we don't use
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Github link to the patch: https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef Ihor Radchenko writes: > Hello, > > [The patch itself will be provided in the following email] > > I have four more updates from the previous version of the patch: > > 1. All the code handling modifications in folded drawers/blocks is moved >to after-change-function. It works as follows: >- if any text is inserted in the middle of hidden region, that text > is also hidden; >- if BEGIN/END line of a folded drawer do not match org-drawer-regexp > and org-property-end-re, unfold it; >- if org-property-end-re or new org-outline-regexp-bol is inserted in > the middle of the drawer, unfold it; >- the same logic for blocks. > > 2. The text property stack is rewritten using char-property-alias-alist. >This is faster in comparison with previous approach, which involved >modifying all the text properties every timer org-flag-region was >called. > > 3. org-toggle-custom-properties-visibility is rewritten using text >properties. I also took a freedom to implement a new feature here. >Now, setting new `org-custom-properties-hide-emptied-drawers' to >non-nil will result in hiding the whole property drawer if it >contains only org-custom-properties. > > 4. This patch should work against 1aa095ccf. However, the merge was not >trivial here. Recent commits actively used the fact that drawers and >outlines are hidden via 'outline invisibility spec, which is not the >case in this branch. I am not confident that I did not break anything >during the merge, especially 1aa095ccf. > > --- > --- > > More details on the new implementation for tracking changes: > >> 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. > > All the code lives in org-after-change-function. I tried to incorporate > the earlier Nicholas' suggestions, except the parts related to > intersecting blocks and drawers. I am not sure if I understand the > parsing priority of blocks vs. drawers. > > --- > --- > > More details on the text property stack: > > The earlier version of the code literally used stack to save > pre-existing 'invisibility specs in org-flag-region. This was done on > every invocation of org-flag-region, which made org-flag-region > significantly slower. I re-implemented the same feature using > char-property-alias-alist. Now, different invisibility specs live in > separate text properties and can be safely modified independently. The > specs are applied according to org--invisible-spec-priority-list. A side > effect of current implementation is that char-property-alias-alist is > fully controlled by org. All the pre-existing settings for 'invisible > text property will be overwritten by org. > >> `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. > > I do not think that `gensym' is suitable here. We don't want a new > symbol every time org--get-buffer-local-invisible-property-symbol is > called. It should return the same symbol if it is called from the same > buffer multiple times. > > --- > --- > > More details on the org-toggle-custom-properties-visibility: > > The implementation showcases how to introduce new invisibility specs to > org. Apart from expected (add-to-invisibility-spec 'org-hide-custom-property) > one also needs to add the spec into org--invisible-spec-priority-list: > > (add-to-list 'org--invisible-spec-priority-list 'org-hide-custom-property) > > Searching for text with the given invisibility spec is done as > follows: > > (text-property-search-forward > (org--get-buffer-local-invisible-property-symbol 'org-hide-custom-property) > 'org-hide-custom-property t) > > This last piece of code is probably not the most elegant. I am thinking > if creating some higher-level interface would be more reasonable here. > What do you think? > > > The new customisation `org-custom-properties-hide-emptied-drawers' > sounds logical for me since empty property drawers left after invoking > org-toggle-custom-properties-visibility are rather useless according to > my experience. If one already wants to hide parts of property drawers, I > do not see a reason to show leftover > > :PROPERTIES: > :END: > > --- >
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
The patch (against 1aa095ccf) is attached.
diff --git a/contrib/lisp/org-notify.el b/contrib/lisp/org-notify.el
index 9f8677871..ab470ea9b 100644
--- a/contrib/lisp/org-notify.el
+++ b/contrib/lisp/org-notify.el
@@ -246,7 +246,7 @@ seconds. The default value for SECS is 20."
(switch-to-buffer (find-file-noselect file))
(org-with-wide-buffer
(goto-char begin)
- (outline-show-entry))
+ (org-show-entry))
(goto-char begin)
(search-forward "DEADLINE: <")
(search-forward ":")
diff --git a/contrib/lisp/org-velocity.el b/contrib/lisp/org-velocity.el
index bfc4d6c3e..2312b235c 100644
--- a/contrib/lisp/org-velocity.el
+++ b/contrib/lisp/org-velocity.el
@@ -325,7 +325,7 @@ use it."
(save-excursion
(when narrow
(org-narrow-to-subtree))
-(outline-show-all)))
+(org-show-all)))
(defun org-velocity-edit-entry/inline (heading)
"Edit entry at HEADING in the original buffer."
diff --git a/doc/org-manual.org b/doc/org-manual.org
index efad195e1..c6f167eac 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -509,11 +509,11 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
Switch back to the startup visibility of the buffer (see [[*Initial
visibility]]).
-- {{{kbd(C-u C-u C-u TAB)}}} (~outline-show-all~) ::
+- {{{kbd(C-u C-u C-u TAB)}}} (~org-show-all~) ::
#+cindex: show all, command
#+kindex: C-u C-u C-u TAB
- #+findex: outline-show-all
+ #+findex: org-show-all
Show all, including drawers.
- {{{kbd(C-c C-r)}}} (~org-reveal~) ::
@@ -529,18 +529,18 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
headings. With a double prefix argument, also show the entire
subtree of the parent.
-- {{{kbd(C-c C-k)}}} (~outline-show-branches~) ::
+- {{{kbd(C-c C-k)}}} (~org-show-branches~) ::
#+cindex: show branches, command
#+kindex: C-c C-k
- #+findex: outline-show-branches
+ #+findex: org-show-branches
Expose all the headings of the subtree, but not their bodies.
-- {{{kbd(C-c TAB)}}} (~outline-show-children~) ::
+- {{{kbd(C-c TAB)}}} (~org-show-children~) ::
#+cindex: show children, command
#+kindex: C-c TAB
- #+findex: outline-show-children
+ #+findex: org-show-children
Expose all direct children of the subtree. With a numeric prefix
argument {{{var(N)}}}, expose all children down to level
{{{var(N)}}}.
@@ -7294,7 +7294,7 @@ its location in the outline tree, but behaves in the following way:
command (see [[*Visibility Cycling]]). You can force cycling archived
subtrees with {{{kbd(C-TAB)}}}, or by setting the option
~org-cycle-open-archived-trees~. Also normal outline commands, like
- ~outline-show-all~, open archived subtrees.
+ ~org-show-all~, open archived subtrees.
-
#+vindex: org-sparse-tree-open-archived-trees
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 9fbeb2a1e..2f121f743 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6824,7 +6824,7 @@ and stored in the variable `org-prefix-format-compiled'."
(t " %-12:c%?-12t% s")))
(start 0)
varform vars var e c f opt)
-(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([ .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+)\\)"
+(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([ .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+?)\\)"
s start)
(setq var (or (cdr (assoc (match-string 4 s)
'(("c" . category) ("t" . time) ("l" . level) ("s" . extra)
@@ -9136,20 +9136,20 @@ if it was hidden in the outline."
((and (called-interactively-p 'any) (= more 1))
(message "Remote: show with default settings"))
((= more 2)
- (outline-show-entry)
+ (org-show-entry)
(org-show-children)
(save-excursion
(org-back-to-heading)
(run-hook-with-args 'org-cycle-hook 'children))
(message "Remote: CHILDREN"))
((= more 3)
- (outline-show-subtree)
+ (org-show-subtree)
(save-excursion
(org-back-to-heading)
(run-hook-with-args 'org-cycle-hook 'subtree))
(message "Remote: SUBTREE"))
((> more 3)
- (outline-show-subtree)
+ (org-show-subtree)
(message "Remote: SUBTREE AND ALL DRAWERS")))
(select-window win)))
diff --git a/lisp/org-archive.el b/lisp/org-archive.el
index d3e12d17b..d864dad8a 100644
--- a/lisp/org-archive.el
+++ b/lisp/org-archive.el
@@ -330,7 +330,7 @@ direct children of this heading."
(insert (if datetree-date "" "\n") heading "\n")
(end-of-line 0))
;; Make the subtree visible
- (outline-show-subtree)
+ (org-show-subtree)
(if org-archive-reversed-order
(progn
(org-back-to-heading t)
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index e50a4d7c8..e656df555 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -699,7 +699,7 @@ FUN is a function called with no argument."
(move-beginning-of-line 2)
(org-at-heading-p t)
(unwind-protect (funcall
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, [The patch itself will be provided in the following email] I have four more updates from the previous version of the patch: 1. All the code handling modifications in folded drawers/blocks is moved to after-change-function. It works as follows: - if any text is inserted in the middle of hidden region, that text is also hidden; - if BEGIN/END line of a folded drawer do not match org-drawer-regexp and org-property-end-re, unfold it; - if org-property-end-re or new org-outline-regexp-bol is inserted in the middle of the drawer, unfold it; - the same logic for blocks. 2. The text property stack is rewritten using char-property-alias-alist. This is faster in comparison with previous approach, which involved modifying all the text properties every timer org-flag-region was called. 3. org-toggle-custom-properties-visibility is rewritten using text properties. I also took a freedom to implement a new feature here. Now, setting new `org-custom-properties-hide-emptied-drawers' to non-nil will result in hiding the whole property drawer if it contains only org-custom-properties. 4. This patch should work against 1aa095ccf. However, the merge was not trivial here. Recent commits actively used the fact that drawers and outlines are hidden via 'outline invisibility spec, which is not the case in this branch. I am not confident that I did not break anything during the merge, especially 1aa095ccf. --- --- More details on the new implementation for tracking changes: > 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. All the code lives in org-after-change-function. I tried to incorporate the earlier Nicholas' suggestions, except the parts related to intersecting blocks and drawers. I am not sure if I understand the parsing priority of blocks vs. drawers. --- --- More details on the text property stack: The earlier version of the code literally used stack to save pre-existing 'invisibility specs in org-flag-region. This was done on every invocation of org-flag-region, which made org-flag-region significantly slower. I re-implemented the same feature using char-property-alias-alist. Now, different invisibility specs live in separate text properties and can be safely modified independently. The specs are applied according to org--invisible-spec-priority-list. A side effect of current implementation is that char-property-alias-alist is fully controlled by org. All the pre-existing settings for 'invisible text property will be overwritten by org. > `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. I do not think that `gensym' is suitable here. We don't want a new symbol every time org--get-buffer-local-invisible-property-symbol is called. It should return the same symbol if it is called from the same buffer multiple times. --- --- More details on the org-toggle-custom-properties-visibility: The implementation showcases how to introduce new invisibility specs to org. Apart from expected (add-to-invisibility-spec 'org-hide-custom-property) one also needs to add the spec into org--invisible-spec-priority-list: (add-to-list 'org--invisible-spec-priority-list 'org-hide-custom-property) Searching for text with the given invisibility spec is done as follows: (text-property-search-forward (org--get-buffer-local-invisible-property-symbol 'org-hide-custom-property) 'org-hide-custom-property t) This last piece of code is probably not the most elegant. I am thinking if creating some higher-level interface would be more reasonable here. What do you think? The new customisation `org-custom-properties-hide-emptied-drawers' sounds logical for me since empty property drawers left after invoking org-toggle-custom-properties-visibility are rather useless according to my experience. If one already wants to hide parts of property drawers, I do not see a reason to show leftover :PROPERTIES: :END: --- --- More details on the merge with the latest master: I tried my best to not break anything. However, I am not sure if I understand all the recent commits. Could someone take a look if there is anything suspicious in
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko 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
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> 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. > 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"? > 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. >> :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. 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. > `org--get-element-region-at-point' is certainly faster, but it is also > wrong, unfortunately. > > Org syntax is not context-free grammar. If you try to parse it locally, > starting from anywhere, it will fail at some point. For example, your > function would choke in the following case: > > [fn:1] Def1 > #+begin_something > > [fn:2] Def2 > #+end_something I see. > AFAIK, the only proper way to parse it is to start from a known position > in the buffer. If you have no information about the buffer, the headline > above is the position you want. With cache could help to start below. > Anyway, in this particular case, you should not use > `org--get-element-region-at-point'. OK Best, Ihor Nicolas Goaziou writes: > Hello, > > Ihor Radchenko writes: > >> [The patch itself will be provided in the following email] > > Thank you. > >> I have found char-property-alias-alist variable that controls how Emacs >> calculates text property value if the property is not set. This variable >> can be buffer-local, which allows independent 'invisible states in >> different buffers. > > Great. I didn't know about this variable! > >> All the implementation stays in >> org--get-buffer-local-text-property-symbol, which takes care about >> generating unique property name and mapping it to 'invisible (or any >> other) text property. > > 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. > >> I simplified the code as suggested, without using pairs of before- and >> after-change-functions. > > Great! > >> Handling text inserted into folded/invisible region is handled by a >> simple after-change function. After testing, it turned out that simple >> re-hiding text based on 'invisible property of the text before/after the >> inserted region works pretty well. > > OK, but this may
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > [The patch itself will be provided in the following email] Thank you. > I have found char-property-alias-alist variable that controls how Emacs > calculates text property value if the property is not set. This variable > can be buffer-local, which allows independent 'invisible states in > different buffers. Great. I didn't know about this variable! > All the implementation stays in > org--get-buffer-local-text-property-symbol, which takes care about > generating unique property name and mapping it to 'invisible (or any > other) text property. 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. > I simplified the code as suggested, without using pairs of before- and > after-change-functions. Great! > Handling text inserted into folded/invisible region is handled by a > simple after-change function. After testing, it turned out that simple > re-hiding text based on 'invisible property of the text before/after the > inserted region works pretty well. OK, but this may not be sufficient if we want to do slightly better than overlays in that area. This is not mandatory, though. > Modifications to BEGIN/END line of the drawers and blocks is handled via > 'modification-hooks + 'insert-behind-hooks text properties (there is no > after-change-functions analogue for text properties in Emacs). The > property is applied during folding and the modification-hook function is > made aware about the drawer/block boundaries (via apply-partially > passing element containing :begin :end markers for the current > drawer/block). Passing the element boundary is important because the > 'modification-hook will not directly know where it belongs to. Only the > modified region (which can be larger than the drawer) is passed to the > function. In the worst case, the region can be the whole buffer (if one > runs revert-buffer). 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? > It turned out that adding 'modification-hook text property takes a > significant cpu time (partially, because we need to take care about > possible existing 'modification-hook value, see > org--add-to-list-text-property). For now, I decided to not clear the > modification hooks during unfolding because of poor performance. > However, this approach would lead to partial unfolding in the following > case: > > :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? > My simplified implementation of element boundary parser > (org--get-element-region-at-point) appears to be much faster and also > uses much less memory in comparison with org-element-at-point. > Moreover, not all the places where org-element-at-point is called > actually need the full parsed element. For example, org-hide-drawer-all, > org-hide-drawer-toggle, org-hide-block-toggle, and > org--hide-wrapper-toggle only need element type and some information > about the element boundaries - the information we can get from > org--get-element-region-at-point. [...] > What do you think about the idea of making use of > org--get-element-region-at-point in org code base? `org--get-element-region-at-point' is certainly faster, but it is also wrong, unfortunately. Org syntax is not context-free grammar. If you try to parse it locally, starting from anywhere, it will fail at some point. For example, your function would choke in the following case: [fn:1] Def1 #+begin_something [fn:2] Def2 #+end_something AFAIK, the only proper way to parse it is to start from a known position in the buffer. If you have no information about the buffer, the headline above is the position you want. With cache could help to start below. Anyway, in this particular case, you should not use `org--get-element-region-at-point'. Hopefully, we don't need to parse anything. In an earlier message, I suggested a few checks to make on the modified text in order to decide if something should be unfolded, or not. I suggest to start from there, and fix any shortcomings we might encounter. We're replacing overlays: low-level is good in this area. WDYT? Regards, -- Nicolas Goaziou
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Oh, I thought this was already done. Do you need to submit the form > or do you wait for the FSF confirmation? Need to submit. Bastien writes: > Hi Ihor, > > Ihor Radchenko writes: > >>> Thanks -- just a quick note, in case you missed the message: we are in >>> feature free for core functionalities, so we have time to work on this >>> welcome enhancement for Org 9.5, which will give us time to properly >>> test it too. >> >> I do not expect it to be merged any time soon. The patch is modifying >> low-level internals. It certainly needs a careful testing under various >> user configs. > > Indeed, thanks for your patience. > >> Not to mention that so big patch will require FSF >> paperwork, unless I miss something. > > Oh, I thought this was already done. Do you need to submit the form > or do you wait for the FSF confirmation? > > -- > Bastien -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi Ihor, Ihor Radchenko writes: >> Thanks -- just a quick note, in case you missed the message: we are in >> feature free for core functionalities, so we have time to work on this >> welcome enhancement for Org 9.5, which will give us time to properly >> test it too. > > I do not expect it to be merged any time soon. The patch is modifying > low-level internals. It certainly needs a careful testing under various > user configs. Indeed, thanks for your patience. > Not to mention that so big patch will require FSF > paperwork, unless I miss something. Oh, I thought this was already done. Do you need to submit the form or do you wait for the FSF confirmation? -- Bastien
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Thanks -- just a quick note, in case you missed the message: we are in > feature free for core functionalities, so we have time to work on this > welcome enhancement for Org 9.5, which will give us time to properly > test it too. I do not expect it to be merged any time soon. The patch is modifying low-level internals. It certainly needs a careful testing under various user configs. Not to mention that so big patch will require FSF paperwork, unless I miss something. Best, Ihor Bastien writes: > Hi Ihor, > > Ihor Radchenko writes: > >> The patch (against 758b039c0) is attached. > > Thanks -- just a quick note, in case you missed the message: we are in > feature free for core functionalities, so we have time to work on this > welcome enhancement for Org 9.5, which will give us time to properly > test it too. > > -- > Bastien -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi Ihor, Ihor Radchenko writes: > The patch (against 758b039c0) is attached. Thanks -- just a quick note, in case you missed the message: we are in feature free for core functionalities, so we have time to work on this welcome enhancement for Org 9.5, which will give us time to properly test it too. -- Bastien
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Github link to the patch: https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef Ihor Radchenko writes: > Hello, > > [The patch itself will be provided in the following email] > > I have three updates from the previous version of the patch: > > 1. I managed to implement buffer-local text properties. >Now, outline folding also uses text properties without a need to give >up independent folding in indirect buffers. > > 2. The code handling modifications in folded drawers/blocks was >rewritten. The new code uses after-change-functions to re-hide text >inserted in the middle of folded regions; and text properties to >unfold folded drawers/blocks if one changes BEGIN/END line. > > 3. [experimental] Started working on improving memory and cpu footprint >of the old code related to folding/unfolding. org-hide-drawer-all now >works significantly faster because I can utilise simplified drawer >parser, which require a lot less memory. Overall, I managed to reduce >Emacs memory footprint after loading all my agenda_files twice. The >loading is also noticeably faster. > > --- > --- > > More details on the buffer-local text properties: > > I have found char-property-alias-alist variable that controls how Emacs > calculates text property value if the property is not set. This variable > can be buffer-local, which allows independent 'invisible states in > different buffers. > > All the implementation stays in > org--get-buffer-local-text-property-symbol, which takes care about > generating unique property name and mapping it to 'invisible (or any > other) text property. > > --- > --- > > More details on the new implementation for tracking changes: > > I simplified the code as suggested, without using pairs of before- and > after-change-functions. > > Handling text inserted into folded/invisible region is handled by a > simple after-change function. After testing, it turned out that simple > re-hiding text based on 'invisible property of the text before/after the > inserted region works pretty well. > > Modifications to BEGIN/END line of the drawers and blocks is handled via > 'modification-hooks + 'insert-behind-hooks text properties (there is no > after-change-functions analogue for text properties in Emacs). The > property is applied during folding and the modification-hook function is > made aware about the drawer/block boundaries (via apply-partially > passing element containing :begin :end markers for the current > drawer/block). Passing the element boundary is important because the > 'modification-hook will not directly know where it belongs to. Only the > modified region (which can be larger than the drawer) is passed to the > function. In the worst case, the region can be the whole buffer (if one > runs revert-buffer). > > It turned out that adding 'modification-hook text property takes a > significant cpu time (partially, because we need to take care about > possible existing 'modification-hook value, see > org--add-to-list-text-property). For now, I decided to not clear the > modification hooks during unfolding because of poor performance. > However, this approach would lead to partial unfolding in the following > case: > > :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. > >> You shouldn't be bothered by the case you're describing here, for >> multiple reasons. >> >> First, this issue already arises in the current implementation. No one >> bothered so far: this change is very unlikely to happen. If it becomes >> an issue, we could make sure that `org-reveal' handles this. >> >> But, more importantly, we actually /want it/ as a feature. Indeed, if >> DRAWER is expanded every time ":BLAH:" is inserted above, then inserting >> a drawer manually would unfold /all/ drawers in the section. The user is >> more likely to write first ":BLAH:" (everything is unfolded) then >> ":END:" than ":END:", then ":BLAH:". > > Agree. This allowed me to simplify the code significantly. > >> It seems you're getting it backwards. `before-change-functions' are the >> functions being called with a possibly wide, imprecise, region to >> handle: >> >> When that happens, the arguments to ‘before-change-functions’ will >> enclose a region in which the individual changes are made, but won’t >> necessarily be the minimal such region >> >> however, after-change-functions calls are always minimal: >> >> and the arguments to each successive call of >> ‘after-change-functions’ will then delimit
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
The patch (against 758b039c0) is attached.
diff --git a/contrib/lisp/org-notify.el b/contrib/lisp/org-notify.el
index 9f8677871..ab470ea9b 100644
--- a/contrib/lisp/org-notify.el
+++ b/contrib/lisp/org-notify.el
@@ -246,7 +246,7 @@ seconds. The default value for SECS is 20."
(switch-to-buffer (find-file-noselect file))
(org-with-wide-buffer
(goto-char begin)
- (outline-show-entry))
+ (org-show-entry))
(goto-char begin)
(search-forward "DEADLINE: <")
(search-forward ":")
diff --git a/contrib/lisp/org-velocity.el b/contrib/lisp/org-velocity.el
index bfc4d6c3e..2312b235c 100644
--- a/contrib/lisp/org-velocity.el
+++ b/contrib/lisp/org-velocity.el
@@ -325,7 +325,7 @@ use it."
(save-excursion
(when narrow
(org-narrow-to-subtree))
-(outline-show-all)))
+(org-show-all)))
(defun org-velocity-edit-entry/inline (heading)
"Edit entry at HEADING in the original buffer."
diff --git a/doc/org-manual.org b/doc/org-manual.org
index 92252179b..ff3e31abe 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -509,11 +509,11 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
Switch back to the startup visibility of the buffer (see [[*Initial
visibility]]).
-- {{{kbd(C-u C-u C-u TAB)}}} (~outline-show-all~) ::
+- {{{kbd(C-u C-u C-u TAB)}}} (~org-show-all~) ::
#+cindex: show all, command
#+kindex: C-u C-u C-u TAB
- #+findex: outline-show-all
+ #+findex: org-show-all
Show all, including drawers.
- {{{kbd(C-c C-r)}}} (~org-reveal~) ::
@@ -529,18 +529,18 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
headings. With a double prefix argument, also show the entire
subtree of the parent.
-- {{{kbd(C-c C-k)}}} (~outline-show-branches~) ::
+- {{{kbd(C-c C-k)}}} (~org-show-branches~) ::
#+cindex: show branches, command
#+kindex: C-c C-k
- #+findex: outline-show-branches
+ #+findex: org-show-branches
Expose all the headings of the subtree, but not their bodies.
-- {{{kbd(C-c TAB)}}} (~outline-show-children~) ::
+- {{{kbd(C-c TAB)}}} (~org-show-children~) ::
#+cindex: show children, command
#+kindex: C-c TAB
- #+findex: outline-show-children
+ #+findex: org-show-children
Expose all direct children of the subtree. With a numeric prefix
argument {{{var(N)}}}, expose all children down to level
{{{var(N)}}}.
@@ -7294,7 +7294,7 @@ its location in the outline tree, but behaves in the following way:
command (see [[*Visibility Cycling]]). You can force cycling archived
subtrees with {{{kbd(C-TAB)}}}, or by setting the option
~org-cycle-open-archived-trees~. Also normal outline commands, like
- ~outline-show-all~, open archived subtrees.
+ ~org-show-all~, open archived subtrees.
-
#+vindex: org-sparse-tree-open-archived-trees
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index f07c3b801..a9c4d9eb2 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6824,7 +6824,7 @@ and stored in the variable `org-prefix-format-compiled'."
(t " %-12:c%?-12t% s")))
(start 0)
varform vars var e c f opt)
-(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([ .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+)\\)"
+(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([ .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+?)\\)"
s start)
(setq var (or (cdr (assoc (match-string 4 s)
'(("c" . category) ("t" . time) ("l" . level) ("s" . extra)
@@ -9136,20 +9136,20 @@ if it was hidden in the outline."
((and (called-interactively-p 'any) (= more 1))
(message "Remote: show with default settings"))
((= more 2)
- (outline-show-entry)
+ (org-show-entry)
(org-show-children)
(save-excursion
(org-back-to-heading)
(run-hook-with-args 'org-cycle-hook 'children))
(message "Remote: CHILDREN"))
((= more 3)
- (outline-show-subtree)
+ (org-show-subtree)
(save-excursion
(org-back-to-heading)
(run-hook-with-args 'org-cycle-hook 'subtree))
(message "Remote: SUBTREE"))
((> more 3)
- (outline-show-subtree)
+ (org-show-subtree)
(message "Remote: SUBTREE AND ALL DRAWERS")))
(select-window win)))
diff --git a/lisp/org-archive.el b/lisp/org-archive.el
index d3e12d17b..d864dad8a 100644
--- a/lisp/org-archive.el
+++ b/lisp/org-archive.el
@@ -330,7 +330,7 @@ direct children of this heading."
(insert (if datetree-date "" "\n") heading "\n")
(end-of-line 0))
;; Make the subtree visible
- (outline-show-subtree)
+ (org-show-subtree)
(if org-archive-reversed-order
(progn
(org-back-to-heading t)
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index e50a4d7c8..e656df555 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -699,7 +699,7 @@ FUN is a function called with no argument."
(move-beginning-of-line 2)
(org-at-heading-p t)
(unwind-protect (funcall
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, [The patch itself will be provided in the following email] I have three updates from the previous version of the patch: 1. I managed to implement buffer-local text properties. Now, outline folding also uses text properties without a need to give up independent folding in indirect buffers. 2. The code handling modifications in folded drawers/blocks was rewritten. The new code uses after-change-functions to re-hide text inserted in the middle of folded regions; and text properties to unfold folded drawers/blocks if one changes BEGIN/END line. 3. [experimental] Started working on improving memory and cpu footprint of the old code related to folding/unfolding. org-hide-drawer-all now works significantly faster because I can utilise simplified drawer parser, which require a lot less memory. Overall, I managed to reduce Emacs memory footprint after loading all my agenda_files twice. The loading is also noticeably faster. --- --- More details on the buffer-local text properties: I have found char-property-alias-alist variable that controls how Emacs calculates text property value if the property is not set. This variable can be buffer-local, which allows independent 'invisible states in different buffers. All the implementation stays in org--get-buffer-local-text-property-symbol, which takes care about generating unique property name and mapping it to 'invisible (or any other) text property. --- --- More details on the new implementation for tracking changes: I simplified the code as suggested, without using pairs of before- and after-change-functions. Handling text inserted into folded/invisible region is handled by a simple after-change function. After testing, it turned out that simple re-hiding text based on 'invisible property of the text before/after the inserted region works pretty well. Modifications to BEGIN/END line of the drawers and blocks is handled via 'modification-hooks + 'insert-behind-hooks text properties (there is no after-change-functions analogue for text properties in Emacs). The property is applied during folding and the modification-hook function is made aware about the drawer/block boundaries (via apply-partially passing element containing :begin :end markers for the current drawer/block). Passing the element boundary is important because the 'modification-hook will not directly know where it belongs to. Only the modified region (which can be larger than the drawer) is passed to the function. In the worst case, the region can be the whole buffer (if one runs revert-buffer). It turned out that adding 'modification-hook text property takes a significant cpu time (partially, because we need to take care about possible existing 'modification-hook value, see org--add-to-list-text-property). For now, I decided to not clear the modification hooks during unfolding because of poor performance. However, this approach would lead to partial unfolding in the following case: :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. > You shouldn't be bothered by the case you're describing here, for > multiple reasons. > > First, this issue already arises in the current implementation. No one > bothered so far: this change is very unlikely to happen. If it becomes > an issue, we could make sure that `org-reveal' handles this. > > But, more importantly, we actually /want it/ as a feature. Indeed, if > DRAWER is expanded every time ":BLAH:" is inserted above, then inserting > a drawer manually would unfold /all/ drawers in the section. The user is > more likely to write first ":BLAH:" (everything is unfolded) then > ":END:" than ":END:", then ":BLAH:". Agree. This allowed me to simplify the code significantly. > It seems you're getting it backwards. `before-change-functions' are the > functions being called with a possibly wide, imprecise, region to > handle: > > When that happens, the arguments to ‘before-change-functions’ will > enclose a region in which the individual changes are made, but won’t > necessarily be the minimal such region > > however, after-change-functions calls are always minimal: > > and the arguments to each successive call of > ‘after-change-functions’ will then delimit the part of text being > changed exactly. > > If you stick to `after-change-functions', there will be no such thing as > you describe. You are right here, I missed that before-change-functions are likely to be called on large regions. I thought that the regions are same for
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > I have five updates from the previous version of the patch: Thank you. > 1. I implemented a simplified version of element parsing to detect > changes in folded drawers or blocks. No computationally expensive calls > of org-element-at-point or org-element-parse-buffer are needed now. > > 2. The patch is now compatible with master (commit 2e96dc639). I > reverted the earlier change in folding drawers and blocks. Now, they are > back to using 'org-hide-block and 'org-hide-drawer. Using 'outline would > achieve nothing when we use text properties. > > 3. 'invisible text property can now be nested. This is important, for > example, when text inside drawers contains fontified links (which also > use 'invisible text property to hide parts of the link). Now, the old > 'invisible spec is recovered after unfolding. Interesting. I'm running out of time, so I cannot properly inspect the code right now. I'll try to do that before the end of the week. > 4. Some outline-* function calls in org referred to outline-flag-region > implementation, which is not in sync with org-flag-region in this patch. > I have implemented their org-* versions and replaced the calls > throughout .el files. Actually, some org-* versions were already > implemented in org, but not used for some reason (or not mentioned in > the manual). I have updated the relevant sections of manual. These > changes might be relevant to org independently of this feature branch. Yes, we certainly want to move to org-specific versions in all cases. > 5. I have managed to get a working version of outline folding via text > properties. However, that approach has a big downside - folding state > cannot be different in indirect buffer when we use text properties. I > have seen packages relying on this feature of org and I do not see any > obvious way to achieve different folding state in indirect buffer while > using text properties for outline folding. Hmm. Good point. This is a serious issue to consider. Even if we don't use text properties for outline, this also affects drawers and blocks. > For now, I still used before/after-change-functions combination. You shouldn't. > I see the following problems with using only after-change-functions: > > 1. They are not guaranteed to be called after every single change: Of course they are! See below. > From (elisp) Change Hooks: > "... some complex primitives call ‘before-change-functions’ once before > making changes, and then call ‘after-change-functions’ zero or more > times" "zero" means there are no changes at all, so, `after-change-functions' are not called, which is expected. > The consequence of it is a possibility that region passed to the > after-change-functions is quite big (including all the singular changes, > even if they are distant). This region may contain changed drawers as > well and unchanged drawers and needs to be parsed to determine which > drawers need to be re-folded. It seems you're getting it backwards. `before-change-functions' are the functions being called with a possibly wide, imprecise, region to handle: When that happens, the arguments to ‘before-change-functions’ will enclose a region in which the individual changes are made, but won’t necessarily be the minimal such region however, after-change-functions calls are always minimal: and the arguments to each successive call of ‘after-change-functions’ will then delimit the part of text being changed exactly. If you stick to `after-change-functions', there will be no such thing as you describe. >> And, more importantly, they are not meant to be used together, i.e., you >> cannot assume that a single call to `before-change-functions' always >> happens before calling `after-change-functions'. This can be tricky if >> you want to use the former to pass information to the latter. > > The fact that before-change-functions can be called multiple times > before after-change-functions, is trivially solved by using buffer-local > changes register (see org--modified-elements). Famous last words. Been there, done that, and it failed. Let me quote the manual: In general, we advise to use either before- or the after-change hooks, but not both. So, let me insist: don't do that. If you don't agree with me, let's at least agree with Emacs developers. > The register is populated by before-change-functions and cleared by > after-change-functions. You cannot expect `after-change-functions' to clear what `before-change-functions' did. This is likely to introduce pernicious bugs. Sorry if it sounds like FUD, but bugs in those areas are just horrible to squash. >> Well, `before-change-fuctions' and `after-change-functions' are not >> clean at all: you modify an unrelated part of the buffer, but still call >> those to check if a drawer needs to be unfolded somewhere. > > 2. As you pointed, instead of global before-change-functions, we can use > modification-hooks text
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Github link to the patch:
https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef
Ihor Radchenko writes:
> The patch is attached
>
> diff --git a/contrib/lisp/org-notify.el b/contrib/lisp/org-notify.el
> index 9f8677871..ab470ea9b 100644
> --- a/contrib/lisp/org-notify.el
> +++ b/contrib/lisp/org-notify.el
> @@ -246,7 +246,7 @@ seconds. The default value for SECS is 20."
>(switch-to-buffer (find-file-noselect file))
>(org-with-wide-buffer
> (goto-char begin)
> - (outline-show-entry))
> + (org-show-entry))
>(goto-char begin)
>(search-forward "DEADLINE: <")
>(search-forward ":")
> diff --git a/contrib/lisp/org-velocity.el b/contrib/lisp/org-velocity.el
> index bfc4d6c3e..2312b235c 100644
> --- a/contrib/lisp/org-velocity.el
> +++ b/contrib/lisp/org-velocity.el
> @@ -325,7 +325,7 @@ use it."
>(save-excursion
> (when narrow
>(org-narrow-to-subtree))
> -(outline-show-all)))
> +(org-show-all)))
>
> (defun org-velocity-edit-entry/inline (heading)
>"Edit entry at HEADING in the original buffer."
> diff --git a/doc/org-manual.org b/doc/org-manual.org
> index 96b160175..2ebe94538 100644
> --- a/doc/org-manual.org
> +++ b/doc/org-manual.org
> @@ -509,11 +509,11 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
>Switch back to the startup visibility of the buffer (see [[*Initial
>visibility]]).
>
> -- {{{kbd(C-u C-u C-u TAB)}}} (~outline-show-all~) ::
> +- {{{kbd(C-u C-u C-u TAB)}}} (~org-show-all~) ::
>
>#+cindex: show all, command
>#+kindex: C-u C-u C-u TAB
> - #+findex: outline-show-all
> + #+findex: org-show-all
>Show all, including drawers.
>
> - {{{kbd(C-c C-r)}}} (~org-reveal~) ::
> @@ -529,18 +529,18 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
>headings. With a double prefix argument, also show the entire
>subtree of the parent.
>
> -- {{{kbd(C-c C-k)}}} (~outline-show-branches~) ::
> +- {{{kbd(C-c C-k)}}} (~org-show-branches~) ::
>
>#+cindex: show branches, command
>#+kindex: C-c C-k
> - #+findex: outline-show-branches
> + #+findex: org-show-branches
>Expose all the headings of the subtree, but not their bodies.
>
> -- {{{kbd(C-c TAB)}}} (~outline-show-children~) ::
> +- {{{kbd(C-c TAB)}}} (~org-show-children~) ::
>
>#+cindex: show children, command
>#+kindex: C-c TAB
> - #+findex: outline-show-children
> + #+findex: org-show-children
>Expose all direct children of the subtree. With a numeric prefix
>argument {{{var(N)}}}, expose all children down to level
>{{{var(N)}}}.
> @@ -7294,7 +7294,7 @@ its location in the outline tree, but behaves in the
> following way:
>command (see [[*Visibility Cycling]]). You can force cycling archived
>subtrees with {{{kbd(C-TAB)}}}, or by setting the option
>~org-cycle-open-archived-trees~. Also normal outline commands, like
> - ~outline-show-all~, open archived subtrees.
> + ~org-show-all~, open archived subtrees.
>
> -
>#+vindex: org-sparse-tree-open-archived-trees
> diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
> index ab13f926c..ad9244940 100644
> --- a/lisp/org-agenda.el
> +++ b/lisp/org-agenda.el
> @@ -6826,7 +6826,7 @@ and stored in the variable
> `org-prefix-format-compiled'."
> (t " %-12:c%?-12t% s")))
> (start 0)
> varform vars var e c f opt)
> -(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([
> .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+)\\)"
> +(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([
> .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+?)\\)"
>s start)
>(setq var (or (cdr (assoc (match-string 4 s)
> '(("c" . category) ("t" . time) ("l" . level)
> ("s" . extra)
> @@ -9138,20 +9138,20 @@ if it was hidden in the outline."
> ((and (called-interactively-p 'any) (= more 1))
>(message "Remote: show with default settings"))
> ((= more 2)
> - (outline-show-entry)
> + (org-show-entry)
>(org-show-children)
>(save-excursion
> (org-back-to-heading)
> (run-hook-with-args 'org-cycle-hook 'children))
>(message "Remote: CHILDREN"))
> ((= more 3)
> - (outline-show-subtree)
> + (org-show-subtree)
>(save-excursion
> (org-back-to-heading)
> (run-hook-with-args 'org-cycle-hook 'subtree))
>(message "Remote: SUBTREE"))
> ((> more 3)
> - (outline-show-subtree)
> + (org-show-subtree)
>(message "Remote: SUBTREE AND ALL DRAWERS")))
> (select-window win)))
>
> diff --git a/lisp/org-archive.el b/lisp/org-archive.el
> index d3e12d17b..d864dad8a 100644
> --- a/lisp/org-archive.el
> +++ b/lisp/org-archive.el
> @@ -330,7 +330,7 @@ direct children of this heading."
> (insert (if datetree-date "" "\n") heading "\n")
> (end-of-line 0))
> ;;
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
The patch is attached
diff --git a/contrib/lisp/org-notify.el b/contrib/lisp/org-notify.el
index 9f8677871..ab470ea9b 100644
--- a/contrib/lisp/org-notify.el
+++ b/contrib/lisp/org-notify.el
@@ -246,7 +246,7 @@ seconds. The default value for SECS is 20."
(switch-to-buffer (find-file-noselect file))
(org-with-wide-buffer
(goto-char begin)
- (outline-show-entry))
+ (org-show-entry))
(goto-char begin)
(search-forward "DEADLINE: <")
(search-forward ":")
diff --git a/contrib/lisp/org-velocity.el b/contrib/lisp/org-velocity.el
index bfc4d6c3e..2312b235c 100644
--- a/contrib/lisp/org-velocity.el
+++ b/contrib/lisp/org-velocity.el
@@ -325,7 +325,7 @@ use it."
(save-excursion
(when narrow
(org-narrow-to-subtree))
-(outline-show-all)))
+(org-show-all)))
(defun org-velocity-edit-entry/inline (heading)
"Edit entry at HEADING in the original buffer."
diff --git a/doc/org-manual.org b/doc/org-manual.org
index 96b160175..2ebe94538 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -509,11 +509,11 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
Switch back to the startup visibility of the buffer (see [[*Initial
visibility]]).
-- {{{kbd(C-u C-u C-u TAB)}}} (~outline-show-all~) ::
+- {{{kbd(C-u C-u C-u TAB)}}} (~org-show-all~) ::
#+cindex: show all, command
#+kindex: C-u C-u C-u TAB
- #+findex: outline-show-all
+ #+findex: org-show-all
Show all, including drawers.
- {{{kbd(C-c C-r)}}} (~org-reveal~) ::
@@ -529,18 +529,18 @@ Org uses just two commands, bound to {{{kbd(TAB)}}} and
headings. With a double prefix argument, also show the entire
subtree of the parent.
-- {{{kbd(C-c C-k)}}} (~outline-show-branches~) ::
+- {{{kbd(C-c C-k)}}} (~org-show-branches~) ::
#+cindex: show branches, command
#+kindex: C-c C-k
- #+findex: outline-show-branches
+ #+findex: org-show-branches
Expose all the headings of the subtree, but not their bodies.
-- {{{kbd(C-c TAB)}}} (~outline-show-children~) ::
+- {{{kbd(C-c TAB)}}} (~org-show-children~) ::
#+cindex: show children, command
#+kindex: C-c TAB
- #+findex: outline-show-children
+ #+findex: org-show-children
Expose all direct children of the subtree. With a numeric prefix
argument {{{var(N)}}}, expose all children down to level
{{{var(N)}}}.
@@ -7294,7 +7294,7 @@ its location in the outline tree, but behaves in the following way:
command (see [[*Visibility Cycling]]). You can force cycling archived
subtrees with {{{kbd(C-TAB)}}}, or by setting the option
~org-cycle-open-archived-trees~. Also normal outline commands, like
- ~outline-show-all~, open archived subtrees.
+ ~org-show-all~, open archived subtrees.
-
#+vindex: org-sparse-tree-open-archived-trees
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index ab13f926c..ad9244940 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6826,7 +6826,7 @@ and stored in the variable `org-prefix-format-compiled'."
(t " %-12:c%?-12t% s")))
(start 0)
varform vars var e c f opt)
-(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([ .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+)\\)"
+(while (string-match "%\\(\\?\\)?\\([-+]?[0-9.]*\\)\\([ .;,:!?=|/<>]?\\)\\([cltseib]\\|(.+?)\\)"
s start)
(setq var (or (cdr (assoc (match-string 4 s)
'(("c" . category) ("t" . time) ("l" . level) ("s" . extra)
@@ -9138,20 +9138,20 @@ if it was hidden in the outline."
((and (called-interactively-p 'any) (= more 1))
(message "Remote: show with default settings"))
((= more 2)
- (outline-show-entry)
+ (org-show-entry)
(org-show-children)
(save-excursion
(org-back-to-heading)
(run-hook-with-args 'org-cycle-hook 'children))
(message "Remote: CHILDREN"))
((= more 3)
- (outline-show-subtree)
+ (org-show-subtree)
(save-excursion
(org-back-to-heading)
(run-hook-with-args 'org-cycle-hook 'subtree))
(message "Remote: SUBTREE"))
((> more 3)
- (outline-show-subtree)
+ (org-show-subtree)
(message "Remote: SUBTREE AND ALL DRAWERS")))
(select-window win)))
diff --git a/lisp/org-archive.el b/lisp/org-archive.el
index d3e12d17b..d864dad8a 100644
--- a/lisp/org-archive.el
+++ b/lisp/org-archive.el
@@ -330,7 +330,7 @@ direct children of this heading."
(insert (if datetree-date "" "\n") heading "\n")
(end-of-line 0))
;; Make the subtree visible
- (outline-show-subtree)
+ (org-show-subtree)
(if org-archive-reversed-order
(progn
(org-back-to-heading t)
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index e50a4d7c8..e656df555 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -699,7 +699,7 @@ FUN is a function called with no argument."
(move-beginning-of-line 2)
(org-at-heading-p t)
(unwind-protect (funcall fun)
- (when
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, [The patch itself will be provided in the following email] I have five updates from the previous version of the patch: 1. I implemented a simplified version of element parsing to detect changes in folded drawers or blocks. No computationally expensive calls of org-element-at-point or org-element-parse-buffer are needed now. 2. The patch is now compatible with master (commit 2e96dc639). I reverted the earlier change in folding drawers and blocks. Now, they are back to using 'org-hide-block and 'org-hide-drawer. Using 'outline would achieve nothing when we use text properties. 3. 'invisible text property can now be nested. This is important, for example, when text inside drawers contains fontified links (which also use 'invisible text property to hide parts of the link). Now, the old 'invisible spec is recovered after unfolding. 4. Some outline-* function calls in org referred to outline-flag-region implementation, which is not in sync with org-flag-region in this patch. I have implemented their org-* versions and replaced the calls throughout .el files. Actually, some org-* versions were already implemented in org, but not used for some reason (or not mentioned in the manual). I have updated the relevant sections of manual. These changes might be relevant to org independently of this feature branch. 5. I have managed to get a working version of outline folding via text properties. However, that approach has a big downside - folding state cannot be different in indirect buffer when we use text properties. I have seen packages relying on this feature of org and I do not see any obvious way to achieve different folding state in indirect buffer while using text properties for outline folding. --- --- More details on the new implementation for tracking changes: > Of course we can. It is only necessary to focus on changes that would > break the structure of the element. This does not entail a full parsing. I have limited parsing to matching beginning and end of a drawer/block. The basic functions are org--get-element-region-at-point, org--get-next-element-region-at-point, and org--find-elements-in-region. They are simplified versions of org-element-* parsers and do not require parsing everything from the beginning of the section. For now, I keep everything in org.el, but those simplified parsers probably belong to org-element.el. > If we can stick with `after-change-functions' (or local equivalent), > that's better. It is more predictable than `before-change-functions' and > alike. For now, I still used before/after-change-functions combination. I see the following problems with using only after-change-functions: 1. They are not guaranteed to be called after every single change: >From (elisp) Change Hooks: "... some complex primitives call ‘before-change-functions’ once before making changes, and then call ‘after-change-functions’ zero or more times" The consequence of it is a possibility that region passed to the after-change-functions is quite big (including all the singular changes, even if they are distant). This region may contain changed drawers as well and unchanged drawers and needs to be parsed to determine which drawers need to be re-folded. > And, more importantly, they are not meant to be used together, i.e., you > cannot assume that a single call to `before-change-functions' always > happens before calling `after-change-functions'. This can be tricky if > you want to use the former to pass information to the latter. The fact that before-change-functions can be called multiple times before after-change-functions, is trivially solved by using buffer-local changes register (see org--modified-elements). The register is populated by before-change-functions and cleared by after-change-functions. > Well, `before-change-fuctions' and `after-change-functions' are not > clean at all: you modify an unrelated part of the buffer, but still call > those to check if a drawer needs to be unfolded somewhere. 2. As you pointed, instead of global before-change-functions, we can use modification-hooks text property on sensitive parts of the drawers/blocks. This would work, but I am concerned about one annoying special case: - :BLAH: :DRAWER: Donec at pede. :END: - In this example, the user would not be able to unfold the folder DRAWER because it will technically become a part of a new giant BLAH drawer. This may be especially annoying if is more than one screen long and there is no easy way to identify why unfolding does not work (with point at :DRAWER:). Because of this scenario, limiting before-change-functions to folded drawers is not sufficient. Any change in text may need to trigger unfolding. In the
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: >> As you noticed, using Org Element is a no-go, unfortunately. Parsing an >> element is a O(N) operation by the number of elements before it in >> a section. In particular, it is not bounded, and not mitigated by >> a cache. For large documents, it is going to be unbearably slow, too. > > Ouch. I thought it is faster. > What do you mean by "not mitigated by a cache"? Parsing starts from the closest headline, every time. So, if Org parses the Nth element in the entry two times, it really parses 2N elements. With a cache, assuming the buffer wasn't modified, Org would parse N elements only. With a smarter cache, with fine grained cache invalidation, it could also reduce the number of subsequent parsed elements. > The reason I would like to utilise org-element parser to make tracking > modifications more robust. Using details of the syntax would make the > code fragile if any modifications are made to syntax in future. I don't think the code would be more fragile. Also, the syntax we're talking about is not going to be modified anytime soon. Moreover, if folding breaks, it is usually visible, so the bug will not be unnoticed. This code is going to be as low-level as it can be. > Debugging bugs in modification functions is not easy, according to my > experience. No, it's not. But this is not really related to whether you use Element or not. > One possible way to avoid performance issues during modification is > running parser in advance. For example, folding an element may > as well add information about the element to its text properties. > This will not degrade performance of folding since we are already > parsing the element during folding (at least, in > org-hide-drawer-toggle). We can use this information stored at fold time. But I'm not even sure we need it. > The problem with parsing an element during folding is that we cannot > easily detect changes like below without re-parsing. Of course we can. It is only necessary to focus on changes that would break the structure of the element. This does not entail a full parsing. > :PROPERTIES: > :CREATED: [2020-05-18 Mon] > :END: <- added line > :ID: test > :END: > > or even > > :PROPERTIES: > :CREATED: [2020-05-18 Mon] > :ID: test > :END: <- delete this line > > :DRAWER: > test > :END: Please have a look at the "sensitive parts" I wrote about. This takes care of this kind of breakage. > The re-parsing can be done via regexp, as you suggested, but I don't > like this idea, because it will end up re-implementing > org-element-*-parser. You may have misunderstood my suggestion. See below. > Would it be acceptable to run org-element-*-parser > in after-change-functions? I'd rather not do that. This is unnecessary consing, and matching, etc. > If I understand correctly, it is not as easy. > Consider the following example: > > :PROPERTIES: > :CREATED: [2020-05-18 Mon] > > :ID: example > :END: > > <... a lot of text, maybe containing other drawers ...> > > Nullam rutrum. > Pellentesque dapibus suscipit ligula. > > Proin quam nisl, tincidunt et, mattis eget, convallis nec, purus. > > If the region gets deleted, the modification hooks from chars inside > drawer will be called as (hook-function > ). So, there is still a need to find the drawer somehow to > mark it as about to be modified (modification hooks are ran before > actual modification). If we can stick with `after-change-functions' (or local equivalent), that's better. It is more predictable than `before-change-functions' and alike. If it is a deletion, here is the kind of checks we could do, depending on when they are performed. Before actual changes : 1. The deletion is happening within a folded drawer (unnecessary step in local functions). 2. The change deleted the sensitive line ":END:". 3. Conclusion : unfold. Or, after actual changes : 1. The deletion involves a drawer. 2. Text properties indicate that the beginning of the propertized part of the buffer start with org-drawer-regexp, but doesn't end with `org-property-end-re'. A "sensitive part" disappeared! 3. Conclusion : unfold This is far away from parsing. IMO, a few checks cover all cases. Let me know if you have questions about it. Also, note that the kind of change you describe will happen perhaps 0.01% of the time. Most change are about one character, or a single line, long. > The only difference between using modification hooks and > before-change-functions is that modification hooks will trigger less > frequently. Exactly. Much less frequently. But extra care is required, as you noted already. > Considering the performance of org-element-at-point, it is > probably worth doing. Initially, I wanted to avoid it because setting a > single before-change-functions hook sounded cleaner than setting > modification-hooks, insert-behind-hooks, and insert-in-front-hooks. Well, `before-change-fuctions' and `after-change-functions' are not clean at all: you
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> As you noticed, using Org Element is a no-go, unfortunately. Parsing an > element is a O(N) operation by the number of elements before it in > a section. In particular, it is not bounded, and not mitigated by > a cache. For large documents, it is going to be unbearably slow, too. Ouch. I thought it is faster. What do you mean by "not mitigated by a cache"? The reason I would like to utilise org-element parser to make tracking modifications more robust. Using details of the syntax would make the code fragile if any modifications are made to syntax in future. Debugging bugs in modification functions is not easy, according to my experience. One possible way to avoid performance issues during modification is running parser in advance. For example, folding an element may as well add information about the element to its text properties. This will not degrade performance of folding since we are already parsing the element during folding (at least, in org-hide-drawer-toggle). The problem with parsing an element during folding is that we cannot easily detect changes like below without re-parsing. :PROPERTIES: :CREATED: [2020-05-18 Mon] :END: <- added line :ID: test :END: or even :PROPERTIES: :CREATED: [2020-05-18 Mon] :ID: test :END: <- delete this line :DRAWER: test :END: The re-parsing can be done via regexp, as you suggested, but I don't like this idea, because it will end up re-implementing org-element-*-parser. Would it be acceptable to run org-element-*-parser in after-change-functions? > If you use modification-hooks and al., you don't need to parse anything, > because you can store information as text properties. Therefore, once > the modification happens, you already know where you are (or, at least > where you were before the change). > The ideas I suggested about sensitive parts of elements are worth > exploring, IMO. Do you have any issue with them? If I understand correctly, it is not as easy. Consider the following example: :PROPERTIES: :CREATED: [2020-05-18 Mon] :ID: example :END: <... a lot of text, maybe containing other drawers ...> Nullam rutrum. Pellentesque dapibus suscipit ligula. Proin quam nisl, tincidunt et, mattis eget, convallis nec, purus. If the region gets deleted, the modification hooks from chars inside drawer will be called as (hook-function ). So, there is still a need to find the drawer somehow to mark it as about to be modified (modification hooks are ran before actual modification). The only difference between using modification hooks and before-change-functions is that modification hooks will trigger less frequently. Considering the performance of org-element-at-point, it is probably worth doing. Initially, I wanted to avoid it because setting a single before-change-functions hook sounded cleaner than setting modification-hooks, insert-behind-hooks, and insert-in-front-hooks. Moreover, these text properties would be copied by default if one uses buffer-substring. Then, the hooks will also trigger later in the yanked text, which may cause all kinds of bugs. > `org-element-at-point' is local, `org-element-parse-buffer' is global. > They are not equivalent, but is it an issue? It was mostly an annoyance, because they returned different results on the same element. Specifically, they returned different :post-blank and :end properties, which does not sound right. Best, Ihor Nicolas Goaziou writes: > Hello, > > Ihor Radchenko writes: > >> Apparently my previous email was again refused by your mail server (I >> tried to add patch as attachment this time). > > Ah. This is annoying, for you and for me. > >> The patch is in >> https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef > > Thank you. > >>> I have finished a seemingly stable implementation of handling changes >>> inside drawer and block elements. For now, I did not bother with >>> 'modification-hooks and 'insert-in-font/behind-hooks, but simply used >>> before/after-change-functions. >>> >>> The basic idea is saving parsed org-elements before the modification >>> (with :begin and :end replaced by markers) and comparing them with the >>> versions of the same elements after the modification. >>> Any valid org element can be examined in such way by an arbitrary >>> function (see org-track-modification-elements) [1]. > > As you noticed, using Org Element is a no-go, unfortunately. Parsing an > element is a O(N) operation by the number of elements before it in > a section. In particular, it is not bounded, and not mitigated by > a cache. For large documents, it is going to be unbearably slow, too. > > I don't think the solution is to use combine-after-change-calls either, > because even a single call to `org-element-at-point' can be noticeable > in a very large section. Such low-level code should avoid using the > Element library altogether, except for the initial folding part, which > is interactive. > > If you use modification-hooks and al., you don't need to parse anything, >
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > Apparently my previous email was again refused by your mail server (I > tried to add patch as attachment this time). Ah. This is annoying, for you and for me. > The patch is in > https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef Thank you. >> I have finished a seemingly stable implementation of handling changes >> inside drawer and block elements. For now, I did not bother with >> 'modification-hooks and 'insert-in-font/behind-hooks, but simply used >> before/after-change-functions. >> >> The basic idea is saving parsed org-elements before the modification >> (with :begin and :end replaced by markers) and comparing them with the >> versions of the same elements after the modification. >> Any valid org element can be examined in such way by an arbitrary >> function (see org-track-modification-elements) [1]. As you noticed, using Org Element is a no-go, unfortunately. Parsing an element is a O(N) operation by the number of elements before it in a section. In particular, it is not bounded, and not mitigated by a cache. For large documents, it is going to be unbearably slow, too. I don't think the solution is to use combine-after-change-calls either, because even a single call to `org-element-at-point' can be noticeable in a very large section. Such low-level code should avoid using the Element library altogether, except for the initial folding part, which is interactive. If you use modification-hooks and al., you don't need to parse anything, because you can store information as text properties. Therefore, once the modification happens, you already know where you are (or, at least where you were before the change). The ideas I suggested about sensitive parts of elements are worth exploring, IMO. Do you have any issue with them? >> For (2), I have introduced org--property-drawer-modified-re to override >> org-property-drawer-re in relevant *-change-function. This seems to work >> for property drawers. However, I am not sure if similar problem may >> happen in some border cases with ordinary drawers or blocks. I already specified what parts were "sensitive" in a previous message. >> 2. I have noticed that results of org-element-at-point and >> org-element-parse-buffer are not always consistent. `org-element-at-point' is local, `org-element-parse-buffer' is global. They are not equivalent, but is it an issue? Regards, -- Nicolas Goaziou
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Dear Nicolas Goaziou, Apparently my previous email was again refused by your mail server (I tried to add patch as attachment this time). The patch is in https://gist.github.com/yantar92/6447754415457927293acda43a7fcaef This patch is actually one commit ahead of the patch in the email, fixing an issue when change function throws an error. I wrapped the call into with-demoted-errors to avoid potential data loss on error in future. Best, Ihor Ihor Radchenko writes: > Hi, > > [All the changes below are relative to commit ed0e75d24. Later commits > make it hard to distinguish between hidden headlines and drawers. I will > need to figure out a way to merge this branch with master. It does not > seem to be trivial.] > > I have finished a seemingly stable implementation of handling changes > inside drawer and block elements. For now, I did not bother with > 'modification-hooks and 'insert-in-font/behind-hooks, but simply used > before/after-change-functions. > > The basic idea is saving parsed org-elements before the modification > (with :begin and :end replaced by markers) and comparing them with the > versions of the same elements after the modification. > Any valid org element can be examined in such way by an arbitrary > function (see org-track-modification-elements) [1]. > > For now, I have implemented tracking changes in all the drawer and block > elements. If the contents of an element is changed and the element is > hidden, the contents remains hidden unless the change was done with > self-insert-command. If the begin/end line of the element was changed in > the way that the element changes the type or extends/shrinks, the > element contents is revealed. To illustrate: > > Case #1 (the element content is hidden): > > :PROPERTIES: > :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 > :END: > > is changed to > > :ROPERTIES: > :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 > :END: > > Text is revealed, because we have drawer in place of property-drawer > > Case #2 (the element content is hidden): > > :ROPERTIES: > :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 > :END: > > is changed to > > :OPERTIES: > :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 > :END: > > The text remains hidden since it is still a drawer. > > Case #3: (the element content is hidden): > > :FOO: > bar > tmp > :END: > > is changed to > > :FOO: > bar > :END: > tmp > :END: > > The text is revealed because the drawer contents shrank. > > Case #4: (the element content is hidden in both the drawers): > > :FOO: > bar > tmp > :END: > :BAR: > jjd > :END: > > is changed to > > :FOO: > bar > tmp > :BAR: > jjd > :END: > > The text is revealed in both the drawers because the drawers are merged > into a new drawer. > >> However, I think we can do better than that, and also fix the glitches >> from overlays. Here are two of them. Write the following drawer: >> >> :FOO: >> bar >> :END: >> >> fold it and delete the ":f". The overlay is still there, and you cannot >> remove it with TAB any more. Or, with the same initial drawer, from >> beginning of buffer, evaluate: >> >> (progn (re-search-forward ":END:") (replace-match "")) >> >> This is no longer a drawer: you just removed its closing line. Yet, the >> overlay is still there, and TAB is ineffective. > > I think the above examples cover what you described. > > Case #5 (the element content is hidden, point at ): > > :FOO: > bar > tmp > :END: > > is changed (via self-insert-command) to > > :FOO:a > bar > tmp > :END: > > The text is revealed. > > This last case sounds logical and might potentially replace > org-catch-invisible-edits. > > > > Some potential issues with the implementation: > > 1. org--after-element-change-function can called many times even for > trivial operations. For example (insert "\n" ":TEST:") seems to call it > two times already. This has two implications: (1) potential performance > degradation; (2) org-element library may not be able to parse the > changed element because its intermediate modified state may not match > the element syntax. Specifically, inserting new property into > :PROPERTIES: drawer inserts a newline at some point, which makes > org-element-at-point think that it is not a 'property-drawer, but just > 'drawer. > > For (1), I did not really do any workaround for now. One potential way > is making use of combine-after-change-calls (info:elisp#Change Hooks). > At least, within org source code. > > For (2), I have introduced org--property-drawer-modified-re to override > org-property-drawer-re in relevant *-change-function. This seems to work > for property drawers. However, I am not sure if similar problem may > happen in some border cases with ordinary drawers or blocks. > > 2. I have noticed that results of org-element-at-point and > org-element-parse-buffer are not always consistent. > In my tests, they returned different number of empty lines after drawers >
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi, [All the changes below are relative to commit ed0e75d24. Later commits make it hard to distinguish between hidden headlines and drawers. I will need to figure out a way to merge this branch with master. It does not seem to be trivial.] I have finished a seemingly stable implementation of handling changes inside drawer and block elements. For now, I did not bother with 'modification-hooks and 'insert-in-font/behind-hooks, but simply used before/after-change-functions. The basic idea is saving parsed org-elements before the modification (with :begin and :end replaced by markers) and comparing them with the versions of the same elements after the modification. Any valid org element can be examined in such way by an arbitrary function (see org-track-modification-elements) [1]. For now, I have implemented tracking changes in all the drawer and block elements. If the contents of an element is changed and the element is hidden, the contents remains hidden unless the change was done with self-insert-command. If the begin/end line of the element was changed in the way that the element changes the type or extends/shrinks, the element contents is revealed. To illustrate: Case #1 (the element content is hidden): :PROPERTIES: :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 :END: is changed to :ROPERTIES: :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 :END: Text is revealed, because we have drawer in place of property-drawer Case #2 (the element content is hidden): :ROPERTIES: :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 :END: is changed to :OPERTIES: :ID: 279e797c-f4a7-47bb-80f6-e72ac6f3ec55 :END: The text remains hidden since it is still a drawer. Case #3: (the element content is hidden): :FOO: bar tmp :END: is changed to :FOO: bar :END: tmp :END: The text is revealed because the drawer contents shrank. Case #4: (the element content is hidden in both the drawers): :FOO: bar tmp :END: :BAR: jjd :END: is changed to :FOO: bar tmp :BAR: jjd :END: The text is revealed in both the drawers because the drawers are merged into a new drawer. > However, I think we can do better than that, and also fix the glitches > from overlays. Here are two of them. Write the following drawer: > > :FOO: > bar > :END: > > fold it and delete the ":f". The overlay is still there, and you cannot > remove it with TAB any more. Or, with the same initial drawer, from > beginning of buffer, evaluate: > > (progn (re-search-forward ":END:") (replace-match "")) > > This is no longer a drawer: you just removed its closing line. Yet, the > overlay is still there, and TAB is ineffective. I think the above examples cover what you described. Case #5 (the element content is hidden, point at ): :FOO: bar tmp :END: is changed (via self-insert-command) to :FOO:a bar tmp :END: The text is revealed. This last case sounds logical and might potentially replace org-catch-invisible-edits. Some potential issues with the implementation: 1. org--after-element-change-function can called many times even for trivial operations. For example (insert "\n" ":TEST:") seems to call it two times already. This has two implications: (1) potential performance degradation; (2) org-element library may not be able to parse the changed element because its intermediate modified state may not match the element syntax. Specifically, inserting new property into :PROPERTIES: drawer inserts a newline at some point, which makes org-element-at-point think that it is not a 'property-drawer, but just 'drawer. For (1), I did not really do any workaround for now. One potential way is making use of combine-after-change-calls (info:elisp#Change Hooks). At least, within org source code. For (2), I have introduced org--property-drawer-modified-re to override org-property-drawer-re in relevant *-change-function. This seems to work for property drawers. However, I am not sure if similar problem may happen in some border cases with ordinary drawers or blocks. 2. I have noticed that results of org-element-at-point and org-element-parse-buffer are not always consistent. In my tests, they returned different number of empty lines after drawers (:post-blank and :end properties). I am not sure here if I did something wrong in the code or if it is a real issue in org-element. For now, I simply called org-element-at-point with point at :begin property of all the elements returned by org-element-parse buffer to make things consistent. This indeed introduced overhead, but I do not see other way to solve the inconsistency. 3. This implementation did not directly solve the previously observed issue with two ellipsis displayed in folded drawer after adding hidden text inside: :PROPERTY: ... --> :PROPERTY: ... ... For now, I just did (org-hide-drawer-toggle 'off) (org-hide-drawer-toggle 'hide) to hide the second ellipsis, but I still don't understand why it is happening. Is it
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Completing myself, Nicolas Goaziou writes: > Each syntactical element has a "sensitive part", a particular area that > can change the nature of the element when it is altered. Luckily drawers > (and blocks) are sturdy. For a drawer, there are three things to check: > > 1. the opening line must match org-drawer-regexp > 2. the closing line must match org-property-end-re (case ignored) > 3. between those, you must not insert text match org-property-end-re or >org-outline-regexp-bol > > Obviously, point 3 needs not be checked during deletion. Point 3 above is inaccurate, one also needs to check that "^[ \t]#\\+end[:_]" doesn't match the body, either. > Instead of `after-change-functions', we may use `modification-hooks' for > deletions, and `insert-behind-hooks' for insertions. For example, we > might add modification-hooks property to both opening and closing line, > and `insert-behind-hooks' on all the drawer. If any of the 3 points > above is verified, we remove all properties. > > Note that if we can implement something robust with text properties, we > might use them for headlines too, for another significant speed-up. Another, less ambitious, possibility is to expand the drawer as soon as text is inserted or removed in the invisible part. Callers (e.g., `org-entry-put') are then responsible to fold it again, if necessary.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > Currently, `org-flag-region' only removes one SPEC type of overlays: > > (remove-overlays from to 'invisible spec) > > If we change it to > > (remove-overlays from to 'invisible spec) > (when flag > (remove-overlays from to 'invisible 'org-hide-drawer) > ... > ) > > then all the extra drawer overlays in the flagged region will be > removed. It will require re-creating those extra overlays later if the > region is revealed again though. Exactly. This would be equivalent to drop `org-hide-drawer' altogether, which we did for property drawers. You have to fold again every drawer after each visibility change. For the record, this is the initial bug `org-hide-drawer' was trying to solve. Back to square one. Also, we would have the same problem with blocks.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > This should be better: > https://gist.github.com/yantar92/e37c2830d3bb6db8678b14424286c930 Thank you. > This might get tricky in the following case: > > :PROPERTIES: > :CREATED: [2020-04-13 Mon 22:31] > > :SHOWFROMDATE: 2020-05-11 > :ID: e05e3b33-71ba-4bbc-abba-8a92c565ad34 > :END: > > > > :PROPERTIES: > :CREATED: [2020-04-27 Mon 13:50] > > :ID: b2eef49f-1c5c-4ff1-8e10-80423c8d8532 > :ATTACH_DIR_INHERIT: t > :END: > > If the text in the region is replaced by something else, in between> should not be fully hidden. We cannot simply look at the > 'invisible property before and after the changed region. Be careful: "replaced by something else" is ambiguous. "Replacing" is an unlikely change: you would need to do (setf (buffer-substring x y) "foo") We can safely assume this will not happen. If it does, we can accept the subsequent glitch. Anyway it is less confusing to think in terms of deletion and insertion. In the case above, you probably mean "the region is deleted then something else is inserted", or the other way. So there are two actions going on, i.e., `after-change-functions' are called twice. In particular the situation you foresee /cannot happen/ with an insertion. Text is inserted at a single point. Let's assume this is in the first drawer. Once inserted, both text before and after the new text were part of the same drawer. Insertion introduces other problems, though. More on this below. It is true the deletion can produce the situation above. But in this case, there is nothing to do, you just merged two drawers into a single one, which stays invisible. Problem solved. IOW, big changes like the one you describe are not an issue. I think the "check if previous and next parts match" trick gives us roughly the same functionality, and the same glitches, as overlays. However, I think we can do better than that, and also fix the glitches from overlays. Here are two of them. Write the following drawer: :FOO: bar :END: fold it and delete the ":f". The overlay is still there, and you cannot remove it with TAB any more. Or, with the same initial drawer, from beginning of buffer, evaluate: (progn (re-search-forward ":END:") (replace-match "")) This is no longer a drawer: you just removed its closing line. Yet, the overlay is still there, and TAB is ineffective. Here's an idea to develop that would make folding more robust, and still fast. Each syntactical element has a "sensitive part", a particular area that can change the nature of the element when it is altered. Luckily drawers (and blocks) are sturdy. For a drawer, there are three things to check: 1. the opening line must match org-drawer-regexp 2. the closing line must match org-property-end-re (case ignored) 3. between those, you must not insert text match org-property-end-re or org-outline-regexp-bol Obviously, point 3 needs not be checked during deletion. Instead of `after-change-functions', we may use `modification-hooks' for deletions, and `insert-behind-hooks' for insertions. For example, we might add modification-hooks property to both opening and closing line, and `insert-behind-hooks' on all the drawer. If any of the 3 points above is verified, we remove all properties. Note that if we can implement something robust with text properties, we might use them for headlines too, for another significant speed-up. WDYT? > I think that using fontification (something similar to > org-fontify-drawers) instead of after-change-functions should be > faster. I don't think it would be faster. With `after-change-functions', `modification-hooks' or `insert-behind-hook', we know exactly where the change happened. Fontification is fuzzier. It is not instantaneous either. It is an option only if we cannot do something fast and accurate with `after-change-functions', IMO.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> You're talking about "overview" (org-overview), whereas I'm talking > about "contents view" (org-content). They are not the same. In the > latter, you show every headline in the buffer, so you have one overlay > per headline. Thanks for the explanation. I finally understand you initial note. I was thinking about org-overview mostly because it is the case when next/previous-line was extremely slow with many overlays jammed between two subsequent lines. >> Now, thinking second time about this, using the following for >> org-flag-region would achieve similar effect: >> >> (remove-overlays from to 'invisible 'outline) >> (remove-overlays from to 'invisible 'org-hide-drawer) >> >> Now sure if it is going to break org-cycle though. >> What do you think? > > This is already the case. See first line of `org-flag-region'. Currently, `org-flag-region' only removes one SPEC type of overlays: (remove-overlays from to 'invisible spec) If we change it to (remove-overlays from to 'invisible spec) (when flag (remove-overlays from to 'invisible 'org-hide-drawer) ... ) then all the extra drawer overlays in the flagged region will be removed. It will require re-creating those extra overlays later if the region is revealed again though. Best, Ihor Nicolas Goaziou writes: > Ihor Radchenko writes: > >> If you want, I can test the file without :LOGBOOK: lines tomorrow. > > Don't worry, it doesn't matter now. > >> No, there are only 9 'outline overlays in the folded buffer if we do not >> create overlays for drawers. This is because outline-hide-sublevels >> called by org-overview is calling outline-flag-region on the whole >> buffer thus removing all the 'outline overlays in buffer >> (remove-overlays from to 'invisible 'outline) and re-creating a single >> overlay for each top-level heading. > > You're talking about "overview" (org-overview), whereas I'm talking > about "contents view" (org-content). They are not the same. In the > latter, you show every headline in the buffer, so you have one overlay > per headline. > >> Now, thinking second time about this, using the following for >> org-flag-region would achieve similar effect: >> >> (remove-overlays from to 'invisible 'outline) >> (remove-overlays from to 'invisible 'org-hide-drawer) >> >> Now sure if it is going to break org-cycle though. >> What do you think? > > This is already the case. See first line of `org-flag-region'. > > Regards, -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > If you want, I can test the file without :LOGBOOK: lines tomorrow. Don't worry, it doesn't matter now. > No, there are only 9 'outline overlays in the folded buffer if we do not > create overlays for drawers. This is because outline-hide-sublevels > called by org-overview is calling outline-flag-region on the whole > buffer thus removing all the 'outline overlays in buffer > (remove-overlays from to 'invisible 'outline) and re-creating a single > overlay for each top-level heading. You're talking about "overview" (org-overview), whereas I'm talking about "contents view" (org-content). They are not the same. In the latter, you show every headline in the buffer, so you have one overlay per headline. > Now, thinking second time about this, using the following for > org-flag-region would achieve similar effect: > > (remove-overlays from to 'invisible 'outline) > (remove-overlays from to 'invisible 'org-hide-drawer) > > Now sure if it is going to break org-cycle though. > What do you think? This is already the case. See first line of `org-flag-region'. Regards,
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Unfortunately, reviewing this way is not nice. This should be better: https://gist.github.com/yantar92/e37c2830d3bb6db8678b14424286c930 > The `insert-and-inherit' issue sounds serious. We cannot reasonably > expect any library outside Org to use it. > > We could automatically extend invisible area with > `after-change-functions', i.e., if we're inserting something and both > side have the same `invisible' property, extend it. Using > `after-change-functions' sounds bad, but this kind of check shouldn't > cost much. > > WDYT? This might get tricky in the following case: :PROPERTIES: :CREATED: [2020-04-13 Mon 22:31] :SHOWFROMDATE: 2020-05-11 :ID: e05e3b33-71ba-4bbc-abba-8a92c565ad34 :END: :PROPERTIES: :CREATED: [2020-04-27 Mon 13:50] :ID: b2eef49f-1c5c-4ff1-8e10-80423c8d8532 :ATTACH_DIR_INHERIT: t :END: If the text in the region is replaced by something else, should not be fully hidden. We cannot simply look at the 'invisible property before and after the changed region. I think that using fontification (something similar to org-fontify-drawers) instead of after-change-functions should be faster. Best, Ihor Nicolas Goaziou writes: > Ihor Radchenko writes: > >> My response to you was blocked by your mail server: >> >>> 550 5.7.1 Reject for policy reason RULE3_2. See >>> http://postmaster.gandi.net > > Aka "spam detected". Bah. > >> The message landed on the mail list though: >> https://www.mail-archive.com/emacs-orgmode@gnu.org/msg127972.html > > Unfortunately, reviewing this way is not nice. > > The `insert-and-inherit' issue sounds serious. We cannot reasonably > expect any library outside Org to use it. > > We could automatically extend invisible area with > `after-change-functions', i.e., if we're inserting something and both > side have the same `invisible' property, extend it. Using > `after-change-functions' sounds bad, but this kind of check shouldn't > cost much. > > WDYT? > > Regards, -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> I don't know how you made your test. You probably didn't > remove :LOGBOOK: lines. When headlines are fully folded, there are > 8 overlays in the buffer, where there used to be 10k. It cannot be > a "small improvement". Ouch. I did not remove :LOGBOOK: lines. I thought you referred to the original file in "I can navigate in your example file without much trouble." If you want, I can test the file without :LOGBOOK: lines tomorrow. >>> ... In current master, >>> it means there is at most 5217 overlays in the buffer. With text >>> properties, the worse situation in the same. >> >> Do you mean that number of overlays is same with text properties? I feel >> that I misunderstand what you want to say. > > AFAIU, you still use overlays for headlines. If you activate so-called > "contents view", all headlines are visible, and are all folded. You get > 5217 overlays in the buffer. No, there are only 9 'outline overlays in the folded buffer if we do not create overlays for drawers. This is because outline-hide-sublevels called by org-overview is calling outline-flag-region on the whole buffer thus removing all the 'outline overlays in buffer (remove-overlays from to 'invisible 'outline) and re-creating a single overlay for each top-level heading. Now, thinking second time about this, using the following for org-flag-region would achieve similar effect: (remove-overlays from to 'invisible 'outline) (remove-overlays from to 'invisible 'org-hide-drawer) Now sure if it is going to break org-cycle though. What do you think? Best, Ihor Nicolas Goaziou writes: > Ihor Radchenko writes: > >> I still do not feel much difference, so I used elp to quantify if there >> is any difference I cannot notice by myself. I tested the time to move >> from to bottom of the example file with next-logical-line. >> >> org master (7801e9236): >> 6(#calls) 2.852953989(total time, sec) 0.4754923315(average) >> >> org e39365e32: >> 6 2.991771891 0.4986286485 >> >> org feature/drawertextprop: >> 6 0.149731379 0.0249552298 >> >> There is small improvement in speed, but it is not obvious. > > I don't know how you made your test. You probably didn't > remove :LOGBOOK: lines. When headlines are fully folded, there are > 8 overlays in the buffer, where there used to be 10k. It cannot be > a "small improvement". > > Ah, well. It doesn't matter. At least the situation improved in some > cases, and the code is better. > >>> ... In current master, >>> it means there is at most 5217 overlays in the buffer. With text >>> properties, the worse situation in the same. >> >> Do you mean that number of overlays is same with text properties? I feel >> that I misunderstand what you want to say. > > AFAIU, you still use overlays for headlines. If you activate so-called > "contents view", all headlines are visible, and are all folded. You get > 5217 overlays in the buffer. > >>> Of course, that case happens less often with text properties. For >>> example, it happens in "contents" view in both cases. However, in "show >>> all" view, it is only a problem with overlays. >> >> I am completely lost. What do you mean by "that case"? > > I am talking about the "worse case" situation just above. > > I'll comment your patch in another message. > > Regards, -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Nicolas Goaziou writes: > Ihor Radchenko writes: > >> My response to you was blocked by your mail server: >> >>> 550 5.7.1 Reject for policy reason RULE3_2. See >>> http://postmaster.gandi.net > > Aka "spam detected". Bah. > >> The message landed on the mail list though: >> https://www.mail-archive.com/emacs-orgmode@gnu.org/msg127972.html > > Unfortunately, reviewing this way is not nice. It's probably not helpful at this point, but just in case: you can get that message's mbox with curl -fSs https://yhetil.org/orgmode/87imh5w1zt.fsf@localhost/raw >mbox
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > My response to you was blocked by your mail server: > >> 550 5.7.1 Reject for policy reason RULE3_2. See >> http://postmaster.gandi.net Aka "spam detected". Bah. > The message landed on the mail list though: > https://www.mail-archive.com/emacs-orgmode@gnu.org/msg127972.html Unfortunately, reviewing this way is not nice. The `insert-and-inherit' issue sounds serious. We cannot reasonably expect any library outside Org to use it. We could automatically extend invisible area with `after-change-functions', i.e., if we're inserting something and both side have the same `invisible' property, extend it. Using `after-change-functions' sounds bad, but this kind of check shouldn't cost much. WDYT? Regards,
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > I still do not feel much difference, so I used elp to quantify if there > is any difference I cannot notice by myself. I tested the time to move > from to bottom of the example file with next-logical-line. > > org master (7801e9236): > 6(#calls) 2.852953989(total time, sec) 0.4754923315(average) > > org e39365e32: > 6 2.991771891 0.4986286485 > > org feature/drawertextprop: > 6 0.149731379 0.0249552298 > > There is small improvement in speed, but it is not obvious. I don't know how you made your test. You probably didn't remove :LOGBOOK: lines. When headlines are fully folded, there are 8 overlays in the buffer, where there used to be 10k. It cannot be a "small improvement". Ah, well. It doesn't matter. At least the situation improved in some cases, and the code is better. >> ... In current master, >> it means there is at most 5217 overlays in the buffer. With text >> properties, the worse situation in the same. > > Do you mean that number of overlays is same with text properties? I feel > that I misunderstand what you want to say. AFAIU, you still use overlays for headlines. If you activate so-called "contents view", all headlines are visible, and are all folded. You get 5217 overlays in the buffer. >> Of course, that case happens less often with text properties. For >> example, it happens in "contents" view in both cases. However, in "show >> all" view, it is only a problem with overlays. > > I am completely lost. What do you mean by "that case"? I am talking about the "worse case" situation just above. I'll comment your patch in another message. Regards,
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
>> I tested with master + my personal config + native compilation of org, >> Emacs native-comp branch, commit c984a53b4e198e31d11d7bc493dc9a686c77edae. >> Did not see much improvement. >> Vertical motion in the folded buffer is still quite slow. > > Oh! This is embarrassing. I improved speed, then broke it again in > a later commit. Sorry for wasting your time. I think I fixed it again. > Thank you for the feedback. > > Could you have a look again? I still do not feel much difference, so I used elp to quantify if there is any difference I cannot notice by myself. I tested the time to move from to bottom of the example file with next-logical-line. org master (7801e9236): 6(#calls) 2.852953989(total time, sec) 0.4754923315(average) org e39365e32: 6 2.991771891 0.4986286485 org feature/drawertextprop: 6 0.149731379 0.0249552298 There is small improvement in speed, but it is not obvious. > ... In current master, > it means there is at most 5217 overlays in the buffer. With text > properties, the worse situation in the same. Do you mean that number of overlays is same with text properties? I feel that I misunderstand what you want to say. > Of course, that case happens less often with text properties. For > example, it happens in "contents" view in both cases. However, in "show > all" view, it is only a problem with overlays. I am completely lost. What do you mean by "that case"? Best, Ihor Nicolas Goaziou writes: > Hello, > > Ihor Radchenko writes: > >>> Oops, you are right. I fixed this. It should be way faster. I can >>> navigate in your example file without much trouble. >>> >>> Please let me know how it goes. >> >> I tested with master + my personal config + native compilation of org, >> Emacs native-comp branch, commit c984a53b4e198e31d11d7bc493dc9a686c77edae. >> Did not see much improvement. >> Vertical motion in the folded buffer is still quite slow. > > Oh! This is embarrassing. I improved speed, then broke it again in > a later commit. Sorry for wasting your time. I think I fixed it again. > Thank you for the feedback. > > Could you have a look again? > >> Apparently I misunderstood the purpose of: 1027e0256 >> "Implement `org-cycle-hide-property-drawers'" > > The function is meant to re-hide only property drawers after visibility > cycling. Its purpose is not to improve /unfolding/ speed. Unfolding is > very fast already, faster than using text properties. > > Folding has roughly the same speed in both cases: most time is spent > looking for the next location to fold. However, folding with text > properties is more resilient, so you fold less often. > > As a side note, your file contains 5217 headlines and 5215 property > drawers. I'll ignore the 3989 regular drawers for the time being > (although they do contribute to the slow navigation). In current master, > it means there is at most 5217 overlays in the buffer. With text > properties, the worse situation in the same. > > Of course, that case happens less often with text properties. For > example, it happens in "contents" view in both cases. However, in "show > all" view, it is only a problem with overlays. > > Regards, > > -- > Nicolas Goaziou -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: >> Oops, you are right. I fixed this. It should be way faster. I can >> navigate in your example file without much trouble. >> >> Please let me know how it goes. > > I tested with master + my personal config + native compilation of org, > Emacs native-comp branch, commit c984a53b4e198e31d11d7bc493dc9a686c77edae. > Did not see much improvement. > Vertical motion in the folded buffer is still quite slow. Oh! This is embarrassing. I improved speed, then broke it again in a later commit. Sorry for wasting your time. I think I fixed it again. Thank you for the feedback. Could you have a look again? > Apparently I misunderstood the purpose of: 1027e0256 > "Implement `org-cycle-hide-property-drawers'" The function is meant to re-hide only property drawers after visibility cycling. Its purpose is not to improve /unfolding/ speed. Unfolding is very fast already, faster than using text properties. Folding has roughly the same speed in both cases: most time is spent looking for the next location to fold. However, folding with text properties is more resilient, so you fold less often. As a side note, your file contains 5217 headlines and 5215 property drawers. I'll ignore the 3989 regular drawers for the time being (although they do contribute to the slow navigation). In current master, it means there is at most 5217 overlays in the buffer. With text properties, the worse situation in the same. Of course, that case happens less often with text properties. For example, it happens in "contents" view in both cases. However, in "show all" view, it is only a problem with overlays. Regards, -- Nicolas Goaziou
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Oops, you are right. I fixed this. It should be way faster. I can > navigate in your example file without much trouble. > > Please let me know how it goes. I tested with master + my personal config + native compilation of org, Emacs native-comp branch, commit c984a53b4e198e31d11d7bc493dc9a686c77edae. Did not see much improvement. Vertical motion in the folded buffer is still quite slow. > The last commits have nothing to do with unfolding. Apparently I misunderstood the purpose of: 1027e0256 "Implement `org-cycle-hide-property-drawers'" Best, Ihor Nicolas Goaziou writes: > Hello, > > Ihor Radchenko writes: > >> Just tested the master branch. >> Three observations on large org file: >> >> 1. Next/previous line on folder buffer is still terribly slow > > Oops, you are right. I fixed this. It should be way faster. I can > navigate in your example file without much trouble. > > Please let me know how it goes. > >> 2. Unfolding speed does not seem to be affected by the last commits - it >> is still much slower than text property version. There might be some >> improvement if I run Emacs for longer time though (Emacs generally >> becomes slower over time). > > The last commits have nothing to do with unfolding. > > I'm not pretending that overlays are faster than text properties, > either. > > With the current implementation property drawers add no overhead : last > commits reduce drastically the number of overlays active in a buffer at > a given time. > >> 3. on a headline with several levels of subheadings moves >> the cursor to the end of subtree, which did not happen in the past. > > Indeed. I fixed that, too. Thank you! > > Regards, > > -- > Nicolas Goaziou -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> Unfortunately, I don't see your patch. My response to you was blocked by your mail server: > 550 5.7.1 Reject for policy reason RULE3_2. See > http://postmaster.gandi.net The message landed on the mail list though: https://www.mail-archive.com/emacs-orgmode@gnu.org/msg127972.html Best, Ihor Nicolas Goaziou writes: > Ihor Radchenko writes: > >> Note that the following commits seems to break my patch: > > Unfortunately, I don't see your patch. -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Ihor Radchenko writes: > Note that the following commits seems to break my patch: Unfortunately, I don't see your patch.
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > Just tested the master branch. > Three observations on large org file: > > 1. Next/previous line on folder buffer is still terribly slow Oops, you are right. I fixed this. It should be way faster. I can navigate in your example file without much trouble. Please let me know how it goes. > 2. Unfolding speed does not seem to be affected by the last commits - it > is still much slower than text property version. There might be some > improvement if I run Emacs for longer time though (Emacs generally > becomes slower over time). The last commits have nothing to do with unfolding. I'm not pretending that overlays are faster than text properties, either. With the current implementation property drawers add no overhead : last commits reduce drastically the number of overlays active in a buffer at a given time. > 3. on a headline with several levels of subheadings moves > the cursor to the end of subtree, which did not happen in the past. Indeed. I fixed that, too. Thank you! Regards, -- Nicolas Goaziou
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Note that the following commits seems to break my patch: 074ea1629 origin/master master Deprecate `org-cycle-hide-drawers' 1027e0256 Implement `org-cycle-hide-property-drawers' 8b05c06d4 Use `outline' invisibility spec for property drawers The patch should work for commit ed0e75d24 in master. Best, Ihor Ihor Radchenko writes: > I have prepared a patch taking into account your comments and fixing > other issues, reported by Karl Voit and found by myself. > > Summary of what is done in the patch: > > 1. iSearching in drawers is rewritten using using > isearch-filter-predicate and isearch-mode-end-hook. > The idea is to create temporary overlays in place of drawers to make > isearch work as usual. > > 2. Change org-show-set-visibility to consider text properties. This > makes helm-occur open drawers. > > 3. Make sure (partially) that text inserted into hidden drawers is also > hidden (to avoid glitches reported by Karl Voit). > The reason why it was happening was because `insert' does not inherit > text properties by default, which means that all the inserted text is > visible by default. I have changes some instances of insert and > insert-before-markers to thair *-and-inherit versions. Still looking > into where else I need to do the replacement. > > Note that "glitch" might appear in many external packages writing into > org drawers. I do not think that insert-and-inherit is often used or > even known. > > Remaining problems: > > 1. insert-* -> insert-*-and-inherit replacement will at least need to be > done in org-table.el and probably other places > > 2. I found hi-lock re-opening drawers after exiting isearch for some > reason. This happens when hi-lock tries to highlight isearch matches. > Not sure about the cause. > > 3. There is still some visual glitch when unnecessary org-ellipsis is > shown when text was inserted into hidden property drawer, though the > inserted text itself is hidden. > >>> (defun org-find-text-property-region (pos prop) >>> "Find a region containing PROP text property around point POS." >>> (require 'org-macs) ;; org-with-point-at >>> (org-with-point-at pos >> >> Do we really need that since every function has a POS argument anyway? >> Is it for the `widen' part? > > Yes, it is not needed. Fixed. > >>> (let* ((beg (and (get-text-property pos prop) pos)) >>>(end beg)) >>> (when beg >>> (setq beg (or (previous-single-property-change pos prop) >>> beg)) >> >> Shouldn't fall-back be (point-min)? >> >>> (setq end (or (next-single-property-change pos prop) >>> end)) >> >> And (point-max) here? > > No, (point-min) and (point-max) may cause problems there. > previous/next-single-property-change returns nil when called at the > beginning/end of the region with given text property. Falling back to > (point-min/max) may wrongly return too large region. > >> Nitpick: `equal' -> = > > Fixed. > >> Or, it seems nicer to `add-function' around `isearch-filter-predicate' >> and extend isearch-filter-visible to support (i.e., stop at, and >> display) invisible text through text properties. > > Done. I used > (setq-local isearch-filter-predicate #'org--isearch-filter-predicate), > which should be even cleaner. > >> I wonder how it compares to drawers using the same invisible spec as >> headlines, as it was the case before. Could you give it a try? > >> I think hiding all property drawers right after opening a subtree is >> fast enough. > > I am not sure what you refer to. Just saw your relevant commit. Will > test ASAP. > > Without testing, the code does not seem to change the number of > overlays. A new overlay is still created for each property drawer. > As I mentioned in the first email, the large number of overlays is what > makes Emacs slow. Citing Eli Zaretskii's reply to my Bug#354553, > explaining why Emacs becomes slow on large org file: > > "... When C-n calls vertical-motion, the latter needs to find the > buffer position displayed directly below the place where you typed > C-n. Since much of the text between these places, vertical-motion > needs to skip the invisible text as quickly as possible, because from > the user's POV that text "doesn't exist": it isn't on the screen. > However, Org makes this skipping exceedingly hard, because (1) it uses > overlays (as opposed to text properties) to hide text, and (2) it puts > an awful lot of overlays on the hidden text: there are 18400 overlays > in this file's buffer, 17500 of them between the 3rd and the 4th > heading. Because of this, vertical-motion must examine each and every > overlay as it moves through the text, because each overlay can > potentially change the 'invisible' property of text, or it might have > a display string that needs to be displayed. So instead of skipping > all that hidden text in one go, vertical-motion loops over those 17.5K > overlays examining the properties of each one of them. And that takes > time." > > I imagine that
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> As a follow-up, I switched property drawers (and only those) back to > using `outline' invisible spec in master branch. Hopefully, navigating > in large folded files should be faster. Just tested the master branch. Three observations on large org file: 1. Next/previous line on folder buffer is still terribly slow 2. Unfolding speed does not seem to be affected by the last commits - it is still much slower than text property version. There might be some improvement if I run Emacs for longer time though (Emacs generally becomes slower over time). 3. on a headline with several levels of subheadings moves the cursor to the end of subtree, which did not happen in the past. Best, Ihor Nicolas Goaziou writes: > Nicolas Goaziou writes: > >> I wonder how it compares to drawers using the same invisible spec as >> headlines, as it was the case before. Could you give it a try? >> >> I think hiding all property drawers right after opening a subtree is >> fast enough. > > As a follow-up, I switched property drawers (and only those) back to > using `outline' invisible spec in master branch. Hopefully, navigating > in large folded files should be faster. > > Of course, this doesn't prevent us to continue exploring > text-properties. In particular, the problem is still open for regular > drawers (e.g., LOGBOOK). -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> I am not sure I understand how your follow-up code (below) needs to be
> incorporated. Would you mind
> sending a patch file? I hope that this ends up in the master branch at some
> point.
I have sent the patch in another email.
Will appreciate any feedback.
Best,
Ihor
Christian Heinrich writes:
> Hi,
>
> thanks for your (initial) patch! I traced another error down today and found
> your code by chance. I
> tested it on an org-drill file that I had (with over 3500 items and hence
> 3500 drawers) and this
> patch helps *a lot* already. (Performance broke in
> 4403d4685e19fb99ba9bfec2bd4ff6781c66981f when
> outline-flag-region was replaced with org-flag-region, as drawers are no
> longer opened using
> outline-show-all which I had to use anyways to deal with my huge file.)
>
> I am not sure I understand how your follow-up code (below) needs to be
> incorporated. Would you mind
> sending a patch file? I hope that this ends up in the master branch at some
> point.
>
> Thanks again!
> Christian
>
> On Mon, 2020-04-27 at 00:04 +0800, Ihor Radchenko wrote:
>> > You cannot. You may however mimic it with `cursor-sensor-functions' text
>> > property. These assume Cursor Sensor minor mode is active, tho.
>> > I haven't tested it, but I assume it would slow down text properties
>> > a bit, too, but hopefully not as much as overlays.
>>
>> Unfortunately, isearch sets inhibit-point-motion-hooks to non-nil
>> internally. Anyway, I came up with some workaround, which seems to work
>> (see below). Though it would be better if isearch supported hidden text
>> in addition to overlays.
>>
>> > Missing `isearch-open-invisible' is a deal breaker, IMO. It may be worth
>> > experimenting with `cursor-sensor-functions'.
>>
>> So far, I came up with the following partial solution searching and
>> showing hidden text.
>>
>> ;; Unfortunately isearch, sets inhibit-point-motion-hooks and we
>> ;; cannot even use cursor-sensor-functions as a workaround
>> ;; I used a less ideas approach with advice to isearch-search-string as
>> ;; a workaround
>>
>> (defun org-find-text-property-region (pos prop)
>> "Find a region containing PROP text property around point POS."
>> (require 'org-macs) ;; org-with-point-at
>> (org-with-point-at pos
>> (let* ((beg (and (get-text-property pos prop) pos))
>> (end beg))
>> (when beg
>> (setq beg (or (previous-single-property-change pos prop)
>>beg))
>> (setq end (or (next-single-property-change pos prop)
>>end))
>> (unless (equal beg end)
>> (cons beg end))
>>
>> ;; :FIXME: re-hide properties when point moves away
>> (define-advice isearch-search-string (:after ( _) put-overlay)
>> "Reveal hidden text at point."
>> (when-let ((region (org-find-text-property-region (point) 'invisible)))
>> (with-silent-modifications
>> (put-text-property (car region) (cdr region) 'org-invisible
>> (get-text-property (point)
>> 'invisible)))
>> (remove-text-properties (car region) (cdr region) '(invisible nil
>>
>> ;; this seems to be unstable, but I cannot figure out why
>> (defun org-restore-invisibility-specs ( _)
>> ""
>>(let ((pos (point-min)))
>> (while (< (setq pos (next-single-property-change pos 'org-invisible nil
>> (point-max))) (point-
>> max))
>>(when-let ((region (org-find-text-property-region pos
>> 'org-invisible)))
>> (with-silent-modifications
>> (put-text-property (car region) (cdr region) 'invisible
>> (get-text-property pos 'org-
>> invisible))
>> (remove-text-properties (car region) (cdr region) '(org-invisible
>> nil)))
>>
>> (add-hook 'post-command-hook #'org-restore-invisibility-specs)
>>
>> (defun org-flag-region (from to flag spec)
>> "Hide or show lines from FROM to TO, according to FLAG.
>> SPEC is the invisibility spec, as a symbol."
>> (pcase spec
>> ('outline
>> (remove-overlays from to 'invisible spec)
>> ;; Use `front-advance' since text right before to the beginning of
>> ;; the overlay belongs to the visible line than to the contents.
>> (when flag
>>(let ((o (make-overlay from to nil 'front-advance)))
>> (overlay-put o 'evaporate t)
>> (overlay-put o 'invisible spec)
>> (overlay-put o 'isearch-open-invisible #'delete-overlay
>> (_
>> (with-silent-modifications
>>(remove-text-properties from to '(invisible nil))
>>(when flag
>> (put-text-property from to 'invisible spec)
>> )
>>
>> ;; This normally deletes invisible text property. We do not want this now.
>> (defun org-unfontify-region (beg end _maybe_loudly)
>> "Remove fontification and activation overlays from links."
>> (font-lock-default-unfontify-region beg end)
>> (let* ((buffer-undo-list t)
>> (inhibit-read-only t) (inhibit-point-motion-hooks t)
>> (inhibit-modification-hooks t)
>> deactivate-mark
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> The visual glitch looks like that: > > :PROPERTIES:X:CREATED: [2020-05-04 Mon 18>54] > X Should be partially fixed in the latest patch I just sent. OLD <<< :PROPERTIES:X:CREATED: [2020-05-04 Mon 18>54] NEW >>> :PROPERTIES:X X Best, Ihor Karl Voit writes: > Hi Ihor, > > * Ihor Radchenko wrote: >> >> So far, I came up with the following partial solution searching and >> showing hidden text. >> >> (defun org-find-text-property-region (pos prop) >> (define-advice isearch-search-string (:after ( _) put-overlay) >> (defun org-restore-invisibility-specs ( _) >> (add-hook 'post-command-hook #'org-restore-invisibility-specs) >> (defun org-flag-region (from to flag spec) >> (defun org-unfontify-region (beg end _maybe_loudly) > > After a couple of hours working with these patches, my feedback is > very positive. Besides some visual glitches when creating a new > heading with org-expiry-insinuate activated (which automatically > adds :CREATED: properties), I could not detect any side-effect so > far (will keep testing). > > The visual glitch looks like that: > > :PROPERTIES:X:CREATED: [2020-05-04 Mon 18>54] > X > > ... with "X" being my character that symbolizes collapsed content. > The way it looked without the patch was a simple collapsed property > drawer. > > To me, this is acceptable considering the huge performance gain I > got. > > THANK YOU VERY MUCH! I can't remember where I had this way of > working within my large Org files[3] since ages. > >>> Anyway, the real fix should come from Emacs itself. There are ways to >>> make overlays faster. These ways have already been discussed on the >>> Emacs devel mailing list, but no one implemented them. It is a bit sad >>> that we have to find workarounds for that. >> >> I guess that it is a very old story starting from the times when XEmacs >> was a thing [1]. I recently heard about binary tree implementation of >> overlays (there should be a branch in emacs git repo) [2], but there was >> no update on that branch for a while. So, I do not have much hope on >> Emacs implementing efficient overlay access in the near future. (And I >> have problems with huge org files already). > > I can not express how this also reflects my personal situation. > >> [1] >> https://www.reddit.com/r/planetemacs/comments/e9lgwn/history_of_lucid_emacs_fsf_emacs_schism/ >> [2] https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00323.html > > [3] https://karl-voit.at/2020/05/03/current-org-files > > -- > get mail|git|SVN|photos|postings|SMS|phonecalls|RSS|CSV|XML into Org-mode: >> get Memacs from https://github.com/novoid/Memacs < > Personal Information Management > http://Karl-Voit.at/tags/pim/ > Emacs-related > http://Karl-Voit.at/tags/emacs/ > > -- Ihor Radchenko, PhD, Center for Advancing Materials Performance from the Nanoscale (CAMP-nano) State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China Email: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
I have prepared a patch taking into account your comments and fixing other issues, reported by Karl Voit and found by myself. Summary of what is done in the patch: 1. iSearching in drawers is rewritten using using isearch-filter-predicate and isearch-mode-end-hook. The idea is to create temporary overlays in place of drawers to make isearch work as usual. 2. Change org-show-set-visibility to consider text properties. This makes helm-occur open drawers. 3. Make sure (partially) that text inserted into hidden drawers is also hidden (to avoid glitches reported by Karl Voit). The reason why it was happening was because `insert' does not inherit text properties by default, which means that all the inserted text is visible by default. I have changes some instances of insert and insert-before-markers to thair *-and-inherit versions. Still looking into where else I need to do the replacement. Note that "glitch" might appear in many external packages writing into org drawers. I do not think that insert-and-inherit is often used or even known. Remaining problems: 1. insert-* -> insert-*-and-inherit replacement will at least need to be done in org-table.el and probably other places 2. I found hi-lock re-opening drawers after exiting isearch for some reason. This happens when hi-lock tries to highlight isearch matches. Not sure about the cause. 3. There is still some visual glitch when unnecessary org-ellipsis is shown when text was inserted into hidden property drawer, though the inserted text itself is hidden. >> (defun org-find-text-property-region (pos prop) >> "Find a region containing PROP text property around point POS." >> (require 'org-macs) ;; org-with-point-at >> (org-with-point-at pos > > Do we really need that since every function has a POS argument anyway? > Is it for the `widen' part? Yes, it is not needed. Fixed. >> (let* ((beg (and (get-text-property pos prop) pos)) >> (end beg)) >> (when beg >> (setq beg (or (previous-single-property-change pos prop) >>beg)) > > Shouldn't fall-back be (point-min)? > >> (setq end (or (next-single-property-change pos prop) >>end)) > > And (point-max) here? No, (point-min) and (point-max) may cause problems there. previous/next-single-property-change returns nil when called at the beginning/end of the region with given text property. Falling back to (point-min/max) may wrongly return too large region. > Nitpick: `equal' -> = Fixed. > Or, it seems nicer to `add-function' around `isearch-filter-predicate' > and extend isearch-filter-visible to support (i.e., stop at, and > display) invisible text through text properties. Done. I used (setq-local isearch-filter-predicate #'org--isearch-filter-predicate), which should be even cleaner. > I wonder how it compares to drawers using the same invisible spec as > headlines, as it was the case before. Could you give it a try? > I think hiding all property drawers right after opening a subtree is > fast enough. I am not sure what you refer to. Just saw your relevant commit. Will test ASAP. Without testing, the code does not seem to change the number of overlays. A new overlay is still created for each property drawer. As I mentioned in the first email, the large number of overlays is what makes Emacs slow. Citing Eli Zaretskii's reply to my Bug#354553, explaining why Emacs becomes slow on large org file: "... When C-n calls vertical-motion, the latter needs to find the buffer position displayed directly below the place where you typed C-n. Since much of the text between these places, vertical-motion needs to skip the invisible text as quickly as possible, because from the user's POV that text "doesn't exist": it isn't on the screen. However, Org makes this skipping exceedingly hard, because (1) it uses overlays (as opposed to text properties) to hide text, and (2) it puts an awful lot of overlays on the hidden text: there are 18400 overlays in this file's buffer, 17500 of them between the 3rd and the 4th heading. Because of this, vertical-motion must examine each and every overlay as it moves through the text, because each overlay can potentially change the 'invisible' property of text, or it might have a display string that needs to be displayed. So instead of skipping all that hidden text in one go, vertical-motion loops over those 17.5K overlays examining the properties of each one of them. And that takes time." I imagine that opening subtree will also require cycling over the [many] overlays in the subtree. > Another option, as I already suggested, would be to use text-properties > on property drawers only. Ignoring isearch inside those sounds > tolerable, at least. Hope the patch below is a reasonable solution to isearch problem with 'invisible text properties. Best, Ihor diff --git a/lisp/org-clock.el b/lisp/org-clock.el index 34179096d..463b28f47 100644 --- a/lisp/org-clock.el +++ b/lisp/org-clock.el @@ -1359,14
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Nicolas Goaziou writes: > I wonder how it compares to drawers using the same invisible spec as > headlines, as it was the case before. Could you give it a try? > > I think hiding all property drawers right after opening a subtree is > fast enough. As a follow-up, I switched property drawers (and only those) back to using `outline' invisible spec in master branch. Hopefully, navigating in large folded files should be faster. Of course, this doesn't prevent us to continue exploring text-properties. In particular, the problem is still open for regular drawers (e.g., LOGBOOK).
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > ;; Unfortunately isearch, sets inhibit-point-motion-hooks and we > ;; cannot even use cursor-sensor-functions as a workaround > ;; I used a less ideas approach with advice to isearch-search-string as > ;; a workaround OK. > (defun org-find-text-property-region (pos prop) > "Find a region containing PROP text property around point POS." > (require 'org-macs) ;; org-with-point-at > (org-with-point-at pos Do we really need that since every function has a POS argument anyway? Is it for the `widen' part? > (let* ((beg (and (get-text-property pos prop) pos)) > (end beg)) > (when beg > (setq beg (or (previous-single-property-change pos prop) > beg)) Shouldn't fall-back be (point-min)? > (setq end (or (next-single-property-change pos prop) > end)) And (point-max) here? > (unless (equal beg end) Nitpick: `equal' -> = > (cons beg end)) > ;; :FIXME: re-hide properties when point moves away > (define-advice isearch-search-string (:after ( _) put-overlay) > "Reveal hidden text at point." > (when-let ((region (org-find-text-property-region (point) 'invisible))) > (with-silent-modifications > (put-text-property (car region) (cdr region) 'org-invisible > (get-text-property (point) 'invisible))) > (remove-text-properties (car region) (cdr region) '(invisible nil Could we use `isearch-update-post-hook' here? Or, it seems nicer to `add-function' around `isearch-filter-predicate' and extend isearch-filter-visible to support (i.e., stop at, and display) invisible text through text properties. > ;; this seems to be unstable, but I cannot figure out why > (defun org-restore-invisibility-specs ( _) > "" >(let ((pos (point-min))) > (while (< (setq pos (next-single-property-change pos 'org-invisible nil > (point-max))) (point-max)) >(when-let ((region (org-find-text-property-region pos 'org-invisible))) > (with-silent-modifications >(put-text-property (car region) (cdr region) 'invisible > (get-text-property pos 'org-invisible)) >(remove-text-properties (car region) (cdr region) '(org-invisible > nil))) Could you use the hook above to store all visited invisible texts, and re-hide them at the end of the search, e.g., using `isearch-mode-end-hook'? > (add-hook 'post-command-hook #'org-restore-invisibility-specs) Ouch. I hope we can avoid that. I wonder how it compares to drawers using the same invisible spec as headlines, as it was the case before. Could you give it a try? I think hiding all property drawers right after opening a subtree is fast enough. Another option, as I already suggested, would be to use text-properties on property drawers only. Ignoring isearch inside those sounds tolerable, at least. Regards, -- Nicolas Goaziou
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi,
thanks for your (initial) patch! I traced another error down today and found
your code by chance. I
tested it on an org-drill file that I had (with over 3500 items and hence 3500
drawers) and this
patch helps *a lot* already. (Performance broke in
4403d4685e19fb99ba9bfec2bd4ff6781c66981f when
outline-flag-region was replaced with org-flag-region, as drawers are no longer
opened using
outline-show-all which I had to use anyways to deal with my huge file.)
I am not sure I understand how your follow-up code (below) needs to be
incorporated. Would you mind
sending a patch file? I hope that this ends up in the master branch at some
point.
Thanks again!
Christian
On Mon, 2020-04-27 at 00:04 +0800, Ihor Radchenko wrote:
> > You cannot. You may however mimic it with `cursor-sensor-functions' text
> > property. These assume Cursor Sensor minor mode is active, tho.
> > I haven't tested it, but I assume it would slow down text properties
> > a bit, too, but hopefully not as much as overlays.
>
> Unfortunately, isearch sets inhibit-point-motion-hooks to non-nil
> internally. Anyway, I came up with some workaround, which seems to work
> (see below). Though it would be better if isearch supported hidden text
> in addition to overlays.
>
> > Missing `isearch-open-invisible' is a deal breaker, IMO. It may be worth
> > experimenting with `cursor-sensor-functions'.
>
> So far, I came up with the following partial solution searching and
> showing hidden text.
>
> ;; Unfortunately isearch, sets inhibit-point-motion-hooks and we
> ;; cannot even use cursor-sensor-functions as a workaround
> ;; I used a less ideas approach with advice to isearch-search-string as
> ;; a workaround
>
> (defun org-find-text-property-region (pos prop)
> "Find a region containing PROP text property around point POS."
> (require 'org-macs) ;; org-with-point-at
> (org-with-point-at pos
> (let* ((beg (and (get-text-property pos prop) pos))
> (end beg))
> (when beg
> (setq beg (or (previous-single-property-change pos prop)
> beg))
> (setq end (or (next-single-property-change pos prop)
> end))
> (unless (equal beg end)
> (cons beg end))
>
> ;; :FIXME: re-hide properties when point moves away
> (define-advice isearch-search-string (:after ( _) put-overlay)
> "Reveal hidden text at point."
> (when-let ((region (org-find-text-property-region (point) 'invisible)))
> (with-silent-modifications
> (put-text-property (car region) (cdr region) 'org-invisible
> (get-text-property (point)
> 'invisible)))
> (remove-text-properties (car region) (cdr region) '(invisible nil
>
> ;; this seems to be unstable, but I cannot figure out why
> (defun org-restore-invisibility-specs ( _)
> ""
>(let ((pos (point-min)))
> (while (< (setq pos (next-single-property-change pos 'org-invisible nil
> (point-max))) (point-
> max))
>(when-let ((region (org-find-text-property-region pos 'org-invisible)))
> (with-silent-modifications
>(put-text-property (car region) (cdr region) 'invisible
> (get-text-property pos 'org-
> invisible))
>(remove-text-properties (car region) (cdr region) '(org-invisible
> nil)))
>
> (add-hook 'post-command-hook #'org-restore-invisibility-specs)
>
> (defun org-flag-region (from to flag spec)
> "Hide or show lines from FROM to TO, according to FLAG.
> SPEC is the invisibility spec, as a symbol."
> (pcase spec
> ('outline
> (remove-overlays from to 'invisible spec)
> ;; Use `front-advance' since text right before to the beginning of
> ;; the overlay belongs to the visible line than to the contents.
> (when flag
>(let ((o (make-overlay from to nil 'front-advance)))
>(overlay-put o 'evaporate t)
>(overlay-put o 'invisible spec)
>(overlay-put o 'isearch-open-invisible #'delete-overlay
> (_
> (with-silent-modifications
>(remove-text-properties from to '(invisible nil))
>(when flag
>(put-text-property from to 'invisible spec)
>)
>
> ;; This normally deletes invisible text property. We do not want this now.
> (defun org-unfontify-region (beg end _maybe_loudly)
> "Remove fontification and activation overlays from links."
> (font-lock-default-unfontify-region beg end)
> (let* ((buffer-undo-list t)
>(inhibit-read-only t) (inhibit-point-motion-hooks t)
>(inhibit-modification-hooks t)
>deactivate-mark buffer-file-name buffer-file-truename)
> (decompose-region beg end)
> (remove-text-properties beg end
> '(mouse-face t keymap t org-linked-text t
>;; Do not remove invisible during
> fontification
>
>;; invisible t
> intangible t
>
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi, * Karl Voit wrote: > Hi Ihor, > > * Ihor Radchenko wrote: >> >> So far, I came up with the following partial solution searching and >> showing hidden text. >> >> (defun org-find-text-property-region (pos prop) >> (define-advice isearch-search-string (:after ( _) put-overlay) >> (defun org-restore-invisibility-specs ( _) >> (add-hook 'post-command-hook #'org-restore-invisibility-specs) >> (defun org-flag-region (from to flag spec) >> (defun org-unfontify-region (beg end _maybe_loudly) > > After a couple of hours working with these patches, my feedback is > very positive. Besides some visual glitches when creating a new > heading with org-expiry-insinuate activated (which automatically > adds :CREATED: properties), I could not detect any side-effect so > far (will keep testing). > > The visual glitch looks like that: > >:PROPERTIES:X:CREATED: [2020-05-04 Mon 18>54] > X > > ... with "X" being my character that symbolizes collapsed content. > The way it looked without the patch was a simple collapsed property > drawer. Here some hard numbers to demonstrate the impact: my-org-agenda: from 11-16s down to 10 -> not much of a difference helm-org-contacts-refresh-cache: 29-59s down to 2½ -> HUGE Emacs boot time: 50-65s down to 10 -> HUGE Navigating the cursor in large Org files -> HUGE subjective impact >>> Anyway, the real fix should come from Emacs itself. There are ways to >>> make overlays faster. These ways have already been discussed on the >>> Emacs devel mailing list, but no one implemented them. It is a bit sad >>> that we have to find workarounds for that. >> >> I guess that it is a very old story starting from the times when XEmacs >> was a thing [1]. I recently heard about binary tree implementation of >> overlays (there should be a branch in emacs git repo) [2], but there was >> no update on that branch for a while. So, I do not have much hope on >> Emacs implementing efficient overlay access in the near future. (And I >> have problems with huge org files already). > > I can not express how this also reflects my personal situation. > >> [1] >> https://www.reddit.com/r/planetemacs/comments/e9lgwn/history_of_lucid_emacs_fsf_emacs_schism/ >> [2] https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00323.html > > [3] https://karl-voit.at/2020/05/03/current-org-files > -- get mail|git|SVN|photos|postings|SMS|phonecalls|RSS|CSV|XML into Org-mode: > get Memacs from https://github.com/novoid/Memacs < Personal Information Management > http://Karl-Voit.at/tags/pim/ Emacs-related > http://Karl-Voit.at/tags/emacs/
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hi Ihor, * Ihor Radchenko wrote: > > So far, I came up with the following partial solution searching and > showing hidden text. > > (defun org-find-text-property-region (pos prop) > (define-advice isearch-search-string (:after ( _) put-overlay) > (defun org-restore-invisibility-specs ( _) > (add-hook 'post-command-hook #'org-restore-invisibility-specs) > (defun org-flag-region (from to flag spec) > (defun org-unfontify-region (beg end _maybe_loudly) After a couple of hours working with these patches, my feedback is very positive. Besides some visual glitches when creating a new heading with org-expiry-insinuate activated (which automatically adds :CREATED: properties), I could not detect any side-effect so far (will keep testing). The visual glitch looks like that: :PROPERTIES:X:CREATED: [2020-05-04 Mon 18>54] X ... with "X" being my character that symbolizes collapsed content. The way it looked without the patch was a simple collapsed property drawer. To me, this is acceptable considering the huge performance gain I got. THANK YOU VERY MUCH! I can't remember where I had this way of working within my large Org files[3] since ages. >> Anyway, the real fix should come from Emacs itself. There are ways to >> make overlays faster. These ways have already been discussed on the >> Emacs devel mailing list, but no one implemented them. It is a bit sad >> that we have to find workarounds for that. > > I guess that it is a very old story starting from the times when XEmacs > was a thing [1]. I recently heard about binary tree implementation of > overlays (there should be a branch in emacs git repo) [2], but there was > no update on that branch for a while. So, I do not have much hope on > Emacs implementing efficient overlay access in the near future. (And I > have problems with huge org files already). I can not express how this also reflects my personal situation. > [1] > https://www.reddit.com/r/planetemacs/comments/e9lgwn/history_of_lucid_emacs_fsf_emacs_schism/ > [2] https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00323.html [3] https://karl-voit.at/2020/05/03/current-org-files -- get mail|git|SVN|photos|postings|SMS|phonecalls|RSS|CSV|XML into Org-mode: > get Memacs from https://github.com/novoid/Memacs < Personal Information Management > http://Karl-Voit.at/tags/pim/ Emacs-related > http://Karl-Voit.at/tags/emacs/
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
> You cannot. You may however mimic it with `cursor-sensor-functions' text
> property. These assume Cursor Sensor minor mode is active, tho.
> I haven't tested it, but I assume it would slow down text properties
> a bit, too, but hopefully not as much as overlays.
Unfortunately, isearch sets inhibit-point-motion-hooks to non-nil
internally. Anyway, I came up with some workaround, which seems to work
(see below). Though it would be better if isearch supported hidden text
in addition to overlays.
> Missing `isearch-open-invisible' is a deal breaker, IMO. It may be worth
> experimenting with `cursor-sensor-functions'.
So far, I came up with the following partial solution searching and
showing hidden text.
;; Unfortunately isearch, sets inhibit-point-motion-hooks and we
;; cannot even use cursor-sensor-functions as a workaround
;; I used a less ideas approach with advice to isearch-search-string as
;; a workaround
(defun org-find-text-property-region (pos prop)
"Find a region containing PROP text property around point POS."
(require 'org-macs) ;; org-with-point-at
(org-with-point-at pos
(let* ((beg (and (get-text-property pos prop) pos))
(end beg))
(when beg
(setq beg (or (previous-single-property-change pos prop)
beg))
(setq end (or (next-single-property-change pos prop)
end))
(unless (equal beg end)
(cons beg end))
;; :FIXME: re-hide properties when point moves away
(define-advice isearch-search-string (:after ( _) put-overlay)
"Reveal hidden text at point."
(when-let ((region (org-find-text-property-region (point) 'invisible)))
(with-silent-modifications
(put-text-property (car region) (cdr region) 'org-invisible
(get-text-property (point) 'invisible)))
(remove-text-properties (car region) (cdr region) '(invisible nil
;; this seems to be unstable, but I cannot figure out why
(defun org-restore-invisibility-specs ( _)
""
(let ((pos (point-min)))
(while (< (setq pos (next-single-property-change pos 'org-invisible nil
(point-max))) (point-max))
(when-let ((region (org-find-text-property-region pos 'org-invisible)))
(with-silent-modifications
(put-text-property (car region) (cdr region) 'invisible
(get-text-property pos 'org-invisible))
(remove-text-properties (car region) (cdr region) '(org-invisible
nil)))
(add-hook 'post-command-hook #'org-restore-invisibility-specs)
(defun org-flag-region (from to flag spec)
"Hide or show lines from FROM to TO, according to FLAG.
SPEC is the invisibility spec, as a symbol."
(pcase spec
('outline
(remove-overlays from to 'invisible spec)
;; Use `front-advance' since text right before to the beginning of
;; the overlay belongs to the visible line than to the contents.
(when flag
(let ((o (make-overlay from to nil 'front-advance)))
(overlay-put o 'evaporate t)
(overlay-put o 'invisible spec)
(overlay-put o 'isearch-open-invisible #'delete-overlay
(_
(with-silent-modifications
(remove-text-properties from to '(invisible nil))
(when flag
(put-text-property from to 'invisible spec)
)
;; This normally deletes invisible text property. We do not want this now.
(defun org-unfontify-region (beg end _maybe_loudly)
"Remove fontification and activation overlays from links."
(font-lock-default-unfontify-region beg end)
(let* ((buffer-undo-list t)
(inhibit-read-only t) (inhibit-point-motion-hooks t)
(inhibit-modification-hooks t)
deactivate-mark buffer-file-name buffer-file-truename)
(decompose-region beg end)
(remove-text-properties beg end
'(mouse-face t keymap t org-linked-text t
;; Do not remove invisible during
fontification
;; invisible t
intangible t
org-emphasis t))
(org-remove-font-lock-display-properties beg end)))
> Anyway, the real fix should come from Emacs itself. There are ways to
> make overlays faster. These ways have already been discussed on the
> Emacs devel mailing list, but no one implemented them. It is a bit sad
> that we have to find workarounds for that.
I guess that it is a very old story starting from the times when XEmacs
was a thing [1]. I recently heard about binary tree implementation of
overlays (there should be a branch in emacs git repo) [2], but there was
no update on that branch for a while. So, I do not have much hope on
Emacs implementing efficient overlay access in the near future. (And I
have problems with huge org files already).
[1]
https://www.reddit.com/r/planetemacs/comments/e9lgwn/history_of_lucid_emacs_fsf_emacs_schism/
[2]
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Nicolas Goaziou writes: > Hello, > > Ihor Radchenko writes: > >> To my surprise, the patch did not break org to unusable state and >> the performance on the sample org file [3] improved drastically. You can >> try by yourself! > > It is not a surprise, really. Text properties are much faster than > overlays, and very close to them features-wise. They are a bit more > complex to handle, however. > >> However, this did introduce some visual glitches with drawer display. >> Though drawers can still be folded/unfolded with , they are not >> folded on org-mode startup for some reason (can be fixed by running >> (org-cycle-hide-drawers 'all)). Also, some drawers (or parts of drawers) >> are unfolded for no apparent reason sometimes. A blind guess is that it >> is something to do with lack of 'isearch-open-invisible, which I am not >> sure how to set via text properties. > > You cannot. You may however mimic it with `cursor-sensor-functions' text > property. These assume Cursor Sensor minor mode is active, tho. > I haven't tested it, but I assume it would slow down text properties > a bit, too, but hopefully not as much as overlays. > > Note there are clear advantages using text properties. For example, when > you move contents around, text properties are preserved. So there's no > more need for the `org-cycle-hide-drawer' dance, i.e., it is not > necessary anymore to re-hide drawers. > >> Any thoughts about the use of text properties or about the patch >> suggestion are welcome. > > Missing `isearch-open-invisible' is a deal breaker, IMO. It may be worth > experimenting with `cursor-sensor-functions'. > > We could also use text properties for property drawers, and overlays for > regular ones. This might give us a reasonable speed-up with an > acceptable feature trade-off. That's great, making Org Mode faster will be great. (Even thought I have not found big performance problem on Org Mode yet.) I like Thor's try. This indeed is is an acceptable feature trade-off, if only related to `isearch-open-invisible'. > > Anyway, the real fix should come from Emacs itself. There are ways to > make overlays faster. These ways have already been discussed on the > Emacs devel mailing list, but no one implemented them. It is a bit sad > that we have to find workarounds for that. > > Regards, - -- [ stardiviner ] I try to make every word tell the meaning what I want to express. Blog: https://stardiviner.github.io/ IRC(freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 -BEGIN PGP SIGNATURE- iQFIBAEBCAAyFiEE8J9lDX1nSBmJJZFAG13xyVromsMFAl6jhG0UHG51bWJjaGls ZEBnbWFpbC5jb20ACgkQG13xyVromsPHDAf+OVnhOq5H5MYm1/RK+9xSzwAT6qc8 ajSNVNzI31q6CIesvO65GoiZ3Rpaiq/O31B9JQ1mTyXvyX81tFecKrDpsrqIc/bR Xo3Z4dCXzCbRKD1861t4tcphtPBk+rABpl83YpXafYNDKHnp2MuWSheV0ogF7LYd 6HWCl9D351onGAHGcebXEUTvvDiqLGx5qVnrpjomH00uCj5RoSI4cpdzXydBcIYY B6lDvsat8AHhvbPXqJc4PHOd4hPtNVehWyPfOGaAXhp/pS0y+c4cJMbHjXCwFCkj r8bUfdK+ZyMubNiboNI9xO8EwINvZLl+C5Lt5siYs/v2mrt1+UiVrxYWTw== =dnH4 -END PGP SIGNATURE-
Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Hello, Ihor Radchenko writes: > To my surprise, the patch did not break org to unusable state and > the performance on the sample org file [3] improved drastically. You can > try by yourself! It is not a surprise, really. Text properties are much faster than overlays, and very close to them features-wise. They are a bit more complex to handle, however. > However, this did introduce some visual glitches with drawer display. > Though drawers can still be folded/unfolded with , they are not > folded on org-mode startup for some reason (can be fixed by running > (org-cycle-hide-drawers 'all)). Also, some drawers (or parts of drawers) > are unfolded for no apparent reason sometimes. A blind guess is that it > is something to do with lack of 'isearch-open-invisible, which I am not > sure how to set via text properties. You cannot. You may however mimic it with `cursor-sensor-functions' text property. These assume Cursor Sensor minor mode is active, tho. I haven't tested it, but I assume it would slow down text properties a bit, too, but hopefully not as much as overlays. Note there are clear advantages using text properties. For example, when you move contents around, text properties are preserved. So there's no more need for the `org-cycle-hide-drawer' dance, i.e., it is not necessary anymore to re-hide drawers. > Any thoughts about the use of text properties or about the patch > suggestion are welcome. Missing `isearch-open-invisible' is a deal breaker, IMO. It may be worth experimenting with `cursor-sensor-functions'. We could also use text properties for property drawers, and overlays for regular ones. This might give us a reasonable speed-up with an acceptable feature trade-off. Anyway, the real fix should come from Emacs itself. There are ways to make overlays faster. These ways have already been discussed on the Emacs devel mailing list, but no one implemented them. It is a bit sad that we have to find workarounds for that. Regards, -- Nicolas Goaziou
