Skip to content

perf: avoid dirname in activation shell hooks#550

Open
nickajacks1 wants to merge 4 commits intocashapp:masterfrom
nickajacks1:shell-hooks-dirname
Open

perf: avoid dirname in activation shell hooks#550
nickajacks1 wants to merge 4 commits intocashapp:masterfrom
nickajacks1:shell-hooks-dirname

Conversation

@nickajacks1
Copy link
Copy Markdown
Contributor

Repeated forking of a dirname process can get rather heavy in deep directory structures, especially with distributions that are moving to uutils. This is most problematic for bash, which runs PROMPT_COMMAND for every prompt as opposed to zsh and fish which run the activation check on chdir. Switching to a pure shell calculation of the parent directory makes performance scale much better.

On an Intel Core Ultra 7 165H with uutils, the existing dirname solution takes about 30 milliseconds to traverse 10 directories, which is enough to be noticeable. Using the updated pure shell approach reduces the time down to 5-10 milliseconds at a directory depth of 80.

Repeated forking of a dirname process can get rather heavy in deep
directory structures, especially with distributions that are moving to
uutils. This is most problematic for bash, which runs PROMPT_COMMAND
for every prompt as opposed to zsh and fish which run the activation
check on chdir. Switching to a pure shell calculation of the parent
directory makes performance scale much better.

On an Intel Core Ultra 7 165H with uutils, the existing dirname solution
takes about 30 milliseconds to traverse 10 directories, which is enough
to be noticeable. Using the updated pure shell approach reduces the time
down to 5-10 milliseconds at a directory depth of 80.
@nickajacks1
Copy link
Copy Markdown
Contributor Author

I didn't make the change to the fish scripts because I figured only running on cd isn't as bad, but I can update that one too for consistency.

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

Successfully merging this pull request may close these issues.

1 participant