perf: definitely reduce scheduler task check to 10 second interval

zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager

MIT License

2.89k stars 125 forks source link

perf: definitely reduce scheduler task check to 10 second interval #435

Open psprint opened 1 year ago

psprint commented 1 year ago

Description

The condition in #429 was reversed, it should say [[ -z … ]] not [[ -n ${ZINIT_TASKS:#<no-data>} ]] && return 0. Now after fixing this it should be working without problems.

Motivation and Context

To fix the long duration problem with the scheduler / CPU usage.

Related Issue(s)

Related issue: #429

How Has This Been Tested?

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.

jankatins commented 1 year ago

Is there any chance of creating a test that the scheduler actually works? E.g. like running a zsh -x -l 2>&1 with an zinit call with wait ice in .zshrc, run this in a background process, killing it after a few seconds and then see if some specific log entries show up after a few seconds in the output?

psprint commented 1 year ago

Yes it would have to use zsh/zpty module. For example autosuggestions uses it to capture completion candidates.

vladdoster commented 1 year ago

@psprint Could you elaborate on how to test this?

psprint commented 1 year ago

@vladdoster I would do: zi is-snippet wait for /dev/null and observe the delay to see that it oscillates around 10 seconds. Looking for the under prompt message.

jankatins commented 1 year ago

@psprint Does that test the happy path of testing the wait ice, i.e. that a plugin with that ice actually gets loaded?

psprint commented 1 year ago

Yes. You can do some atclone'touch x' to easy test.

vladdoster commented 1 year ago

@psprint

Can you write one or two Zunit tests for this PR? I want verification to avoid borking main a third time and it would be a good test to have.

zdharma-continuum's zunit fork.

vladdoster commented 1 year ago

@psprint,

I want to merge this.

Can you elaborate on the following comment?:

Yes. You can do some atclone'touch x' to easy test.

Am I following correctly the test would be validating time difference ± N variance (e.g., using stat) from when the initial repository is cloned and file x was created?