Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timer spam when org file is opened by org-agenda #11

Closed
aavanian opened this issue Dec 21, 2023 · 15 comments
Closed

Timer spam when org file is opened by org-agenda #11

aavanian opened this issue Dec 21, 2023 · 15 comments

Comments

@aavanian
Copy link

I'm not 100% sure that's what's going on but it seems that if an org file is opened in a certain way (typically in the case I observe, by org-agenda), org-indent doesn't finish 'preparing' the buffer and adding it to org-indent-agentized-buffers (I suppose because the buffer hasn't been displayed yet) and so org-modern-indent's timer resets itself indefinitely until the buffer is interactively visited or some other condition that lead to org-indent adding the buffer to org-indent-agentized-buffers.

With a large amount of files in org-agenda-files, this slows down emacs since #number-of-buffer timers are executing every 0.1s. I'm not sure if there's a setting I could use to force org-indent to complete (from what I understand, org-indent-agent-*-delay wouldn't help since they skip the buffer if it's not active.

@jdtsmith
Copy link
Owner

Thanks for the report. I can confirm that org-modern-indent--wait-and-refresh simply arranges to have itself called again every 100ms until the buffer appears on org-indent-agentized-buffers. Can you spell this out in more detail? Are the buffer of interest actually being opened and viewed? Maybe we need a more reliable hook from org-indent.

@jdtsmith
Copy link
Owner

Please try my most recent commit and report back. I reuse the timer (to prevent garbage buildup) and give up after 5 tries per buffer. I don't know why org-indent starts but never finishes initializing in your buffers; probably the right thing is to prevent org(-modern)-indent from running in that context.

@aavanian
Copy link
Author

Can you spell this out in more detail? Are the buffer of interest actually being opened and viewed? Maybe we need a more reliable hook from org-indent.

My understanding (I'm no emacs/lisp expert): when the buffers are opened by org-agenda or org-refile, etc.. with org-indent-mode enabled, the opened buffers are pushed to org-indent-agentized-buffers and not being active, it will try to initialize them 'softly': the agent will stop initialization of the first buffer of the list after 400ms (org-indent-agent-passive-delay default) and reschedule itself 100ms (org-indent-agent-resume-delay) later in that case.

There's only one agent/timer: when a new buffer is opened and needs to be initialized, if the agent is already running, it just pushes the buffer into the agentized list, otherwise it starts the agent/timer (with the repeating flag, so it will keep running until the list is empty).

I don't think there's a better hook offered by org-indent, or that you could use advices since org-indent seems to complete initialization via three different paths in org-indent-add-properties or through the uninterrupted org-indent-initialize-buffer (but I'm not knowledgeable enough to know if advice would work here, it'd feel brittle at best to me)

An alternative would be to model the initialization of org-modern-indent in the same way as org-indent: you'd maintain a list of buffers to initialize and run a single timer that executes immediately if it's the current buffer and it's been indent-initialized or check other buffers in the list against a time budget and give until next idle time until the list is empty.

Please try my most recent commit and report back. I reuse the timer (to prevent garbage buildup) and give up after 5 tries per buffer. I don't know why org-indent starts but never finishes initializing in your buffers;

It works in the sense it stops trying, I have 450 buffers (I know it's unusual but except when loading the agenda the first time in the session, it doesn't hinder usage) and it gives up almost instantly (as fast as it can run 0.5s worth of timers concurrently).

probably the right thing is to prevent org(-modern)-indent from running in that context.

org-indent delaying itself doesn't slow down emacs the way org-modern-indent does. I guess because it runs only one timer. Note also org-indent uses run-with-idle-timer rather than run-at-time.

So I modified org-modern-indent to test with run-with-idle-timer (while disabling the > 5 count check) and it somewhat stops slowing down emacs but it does slow down org-indent agent (i suppose it's crowding it out of the idle opportunities) leading to org-indent finishing passive initialization slower than with org-modern-indent. That's acceptable though since the buffers are not actively used. It still leads to slowdowns (particulary when C-g out of a minibuffer selection, I have no idea why).

@jdtsmith
Copy link
Owner

jdtsmith commented Dec 26, 2023

So it sounds like the max-5 fix helps but there are still slowdowns due to all the timers proliferating. One thing I still don't understand: do you want all 450 buffers fully org-(modern)-indent'ed? Or would you prefer org-indent to just leave those alone? If you don't care to actually look at those buffers, the right thing to do is to disable org-indent and org-modern-indent when buffers are loaded in this automated manner via org-agenda, except if/when you actually display them in a window. This is simlar to what, e.g., consult preview does. Why spend time initializing buffers for display which you will never actually look at?

In terms of a more durable fix, since org-modern-indent uses the line-prefixes org-indent places there, it's pretty much captive to whatever setup cadence the latter implements. It would be really useful for org-indent to have an org-indent-buffer-init-finished abnormal hook (called with the buffer). I agree that using advice or anything more invasive would be quite fragile and prone to breaking, given the convoluted design of the "agentized" delay initialization.

There in fact is a perfect spot in org-indent to call such a hook:

         ;; Job is complete: un-agentize buffer.
	 (unless interruptp
	   (setq org-indent-agentized-buffers
		 (delq buffer org-indent-agentized-buffers))
           (run-hook-with-args 'org-indent-buffer-init-finished buffer)) ;; <-- added

Do you want to engage the org mailing list folks to see about adding such an abnormal hook to org-indent? Then we could get out of the timer business altogether. Copy @minad in case he has thoughts about a suitable design here.

@aavanian
Copy link
Author

aavanian commented Dec 26, 2023

If you don't care to actually look at those buffers, the right thing to do is to disable org-indent and org-modern-indent when buffers are loaded in this automated manner via org-agenda, except if/when you actually display them in a window. This is simlar to what, e.g., consult preview does. Why spend time initializing buffers for display which you will never actually look at?

You're right that would be the better way (marginally: though non-zero, I don't think the cost is perceptible). I don't need them org-(+modern)-indented in the background as long as they do get org-(+modern)-indented as I activate them throughout the day. But I couldn't find yet a way to do it except in post-command-hook but that seemed inappropriate/overkill. Thanks for the pointer to consult-preview, I'll have a look.

EDIT: seems like post-command-hook is what consult-preview uses so I guess that's the way for this solution

Agree also a hook would be better than a timer, not sure I'll be able to follow-through after raising it with org but I can try.

@jdtsmith
Copy link
Owner

Great. If the org folks agree it's a trivial update.

@jdtsmith
Copy link
Owner

Any traction on an org-indent-finished hook?

@aavanian
Copy link
Author

aavanian commented Jan 1, 2024

yes, ok on principle, need patch. Will you do it (since you proposed the change in the first place) or shall I?

@jdtsmith
Copy link
Owner

jdtsmith commented Jan 1, 2024

Great, thanks for pushing this along. Please feel free to submit a patch. You'd need to add a defcustom as well. Once that is in we can try using it on a branch; should be effective and straightforward. Then we'll need to decide how to handle version dependence. One idea would be to use the hook if present, and stick with current timer approach otherwise.

@aavanian
Copy link
Author

aavanian commented Jan 3, 2024

It's in (see here). The hook variable org-indent-post-buffer-init-functions is versioned to Org 9.7 so that can be another way to handle version dependence.

@jdtsmith
Copy link
Owner

jdtsmith commented Jan 3, 2024

Great, thanks for this. Can you give the org-buffer-init-hook branch a try with (and without) this new version and let me know how it goes for your tons-of-org-files use case?

@aavanian
Copy link
Author

aavanian commented Jan 4, 2024

Thanks. With that branch and:

  • org version 9.6.7: same behaviour as before. By the way, if org-modern-indent fails to initialize, should it leave its minor-mode activated (currently the case with the 5 fails -> stop)?
  • org version 9.7-pre (includes the hook patch): works well. When activating a buffer, it's quickly initialized both by org-indent and org-modern-indent (if still pending init) or already initialized.

@jdtsmith
Copy link
Owner

jdtsmith commented Jan 4, 2024

OK thanks. Good point about leaving it active after init failure; pushed a fix; please give a try and see if that works well in your case.

I don't know when org 9.7 will release (soon?), but given that your case is pretty rare I think the current setup is OK.

@aavanian
Copy link
Author

aavanian commented Jan 5, 2024

All good for me. I'll withdraw my PR then.

As for org's release, I can run the 9.7 pre-release or a custom branch with the hook patch so no issue for me.

@jdtsmith
Copy link
Owner

jdtsmith commented Jan 6, 2024

Great, thanks for bringing this to my attention.

@jdtsmith jdtsmith closed this as completed Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants