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

Cache compinit for 20h #8

Closed
wants to merge 1 commit into from
Closed

Conversation

rjcoelho
Copy link
Contributor

Omit checking for new functions during 20h, see http:https://zsh.sourceforge.net/Doc/Release/Completion-System.html

I get a speed bump from 0.06s to 0.04s on zsh startup.

Don't know how this interacts with zimfw compile (we compile zcompdump). Code was taken from prezto/omz

init.zsh Show resolved Hide resolved
@ericbn
Copy link
Member

ericbn commented Dec 10, 2019

I like the idea of having some mechanism to update the dumpfile, as indeed we never allow it to be updated in the current code, as we always skip the check with -C. But we have something in our favor, that is the install/update actions in zimfw. That's when things can change for the completion: when a module is installed or updated. So maybe we can add some logic there instead.

@rjcoelho
Copy link
Contributor Author

I like the idea of having some mechanism to update the dumpfile, as indeed we never allow it to be updated in the current code, as we always skip the check with -C. But we have something in our favor, that is the install/update actions in zimfw. That's when things can change for the completion: when a module is installed or updated. So maybe we can add some logic there instead.
I also thought about it.

Correct me if i'm wrong but currently if we add/remove completions we should zimfw clean (rm zdumpfile) the file, then do a login (which generates a new zdumpfile since we call it with -C) and then zimfw compile (to build .zwc) ?

This change basically automates it by "forcing" to generate a new zdumpfile every 20h. Unlike what I say, it isn't a performance improvement. Every 20h we force a new zdumpfile (by calling w/o -C) which we should then zcompile it (we should probably zcompile it here in this PR). What do you think ?

@ericbn
Copy link
Member

ericbn commented Dec 11, 2019

Yes, currently the only way to force compinit to dump a new configuration is to run zimfw clean-dumpfile.

As explained in the compinit documentation

If the number of completion files changes, compinit will recognise this and produce a new dump file. However, if the name of a function or the arguments in the first line of a #compdef function (as described below) change, it is easiest to delete the dump file by hand so that compinit will re-create it the next time it is run. The check performed to see if there are new functions can be omitted by giving the option -C. In this case the dump file will only be created if there isn’t one already.

I see a couple issues with the proposed solution:

  1. The 20 hours expiration is arbitrary and will impact the startup performance every 20 hours. Startup time is precious to us. Depending on the user configuration and how often he/she updates it, the completion would require effective regeneration every 20 hours, or 20 days, or 20 weeks, or 20 months... So we don't want to penalize all users every 20 hours.
  2. The way compinit recognizes that the configuration needs to be updated is too simple, and only covers the scenario when the number of completion files changes. All other scenarios still require the dumpfile to be manually deleted.

But you raised a very good point, and I already have an idea on how to cover all scenarios during the usage of the zimfw tool, which also has the advantage of not being executed during start up. Will work on it soon.

@ericbn
Copy link
Member

ericbn commented Dec 11, 2019

Benchmark with the default zimfw installation (from the develop branch as the time of writing this):

  • with -C (which is how it currently works):

    time                 34.34 ms   (34.17 ms .. 34.59 ms)
                         1.000 R²   (0.999 R² .. 1.000 R²)
    mean                 34.67 ms   (34.48 ms .. 34.86 ms)
    std dev              405.1 μs   (334.0 μs .. 524.8 μs)
    
  • without -C:

    time                 66.78 ms   (66.16 ms .. 68.10 ms)
                         1.000 R²   (0.999 R² .. 1.000 R²)
    mean                 66.47 ms   (66.13 ms .. 66.91 ms)
    std dev              640.3 μs   (420.8 μs .. 985.6 μs)
    

So impact is significant! (And benefit is debatable, as I pointed out in the previous comment.)

@rjcoelho
Copy link
Contributor Author

rjcoelho commented Dec 11, 2019

I want do be able to install/delete/update packages on my machine without worrying if it needs to dump a new compinit. That is the problem I am trying to solve here.

The 20h is a "daily" interval. Should probably be 24h. So I pay a 30ms penalty on the first daily login. It's acceptable imo.

Every day I will call once without -C and then zcompile it. Do you automatically zcompile on login if .zwc doest exist or the respective zsh has changed ?

@ericbn
Copy link
Member

ericbn commented Jan 25, 2022

I want do be able to install/delete/update packages on my machine without worrying if it needs to dump a new compinit.

Created a check-dumpfile action for zimfw that runs the same checks as compinit and is intended solve the problem you describe. This was released with version 1.8.0 of zimfw.

Instead of a check during shell startup with an arbitrary schedule (e.g. 20h), this runs after files are updated by zimfw (in the build, install and update actions). Only if a change in the number of completion functions or in the Zsh version is detected, then the dumpfile is removed in order to be dumped again in the next shell startup.

@ericbn ericbn closed this Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants