Skip to content

Use universal variables instead of temp files - #76

Open
andrasmaroy wants to merge 3 commits into
acomagu:masterfrom
andrasmaroy:master
Open

Use universal variables instead of temp files#76
andrasmaroy wants to merge 3 commits into
acomagu:masterfrom
andrasmaroy:master

Conversation

@andrasmaroy

Copy link
Copy Markdown

While setting up my prompt with fish_async_prompt I noticed that after adding multiple async functions it started getting noticable slow. After a bit of digging around I found that adding an additional async function adds around 80ms of execution time to fish_prompt:

# 1 async function configured in async_prompt_functions
$ time fish_prompt
________________________________________________________
Executed in   87.69 millis    fish           external
   usr time   62.95 millis  243.00 micros   62.71 millis
   sys time   21.77 millis  885.00 micros   20.89 millis

# 2 async functions configured in async_prompt_functions
$ time fish_prompt
________________________________________________________
Executed in  166.54 millis    fish           external
   usr time  123.61 millis    1.29 millis  122.32 millis
   sys time   41.11 millis    3.16 millis   37.94 millis

# 3 async functions configured in async_prompt_functions
$ time fish_prompt
________________________________________________________
Executed in   256.66 millis    fish           external
   usr time   186.53 millis   1.44 millis  185.08 millis
   sys time   64.06 millis    3.92 millis   60.14 millis

While having 1 function like this isn't really noticable as it is below 100ms, this starts adding up quickly and becomes really noticable, especially when the whole goal is to make the prompt faster with making it async.

I didn't dig deep enough to investigate why reading from temporary files make this process this slow, but quickly doing a proof of concept using universal variables instead showed a huge performance increase:

# 5 async functions configured in async_prompt_functions
$ time fish_prompt                                                                                                                                                                       
________________________________________________________
Executed in  292.00 micros    fish           external
   usr time  265.00 micros  265.00 micros    0.00 micros
   sys time   28.00 micros   28.00 micros    0.00 micros

I've only tested it on my local setup (MacOS 13.2.1, fish 3.7.1), so I can't comment if this has unintended side effects on other systems, but since this variable handling is part of the shell itself I would presume no.

and cat $__async_prompt_tmpdir'/'$fish_pid'_'$func
if set -q $__async_prompt_var'_'$func
set -l result $__async_prompt_var'_'$func
echo -n $$result

@petertriho petertriho Jul 18, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using starship and this causes the multiline prompts to merge into a single line.
I changed this to printf "%s\n" $$result and seems to work for both single and multiline prompts

EDIT: only needed printf "%s" $$result nvm need \n

@ilyagr

ilyagr commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

Using universal variables here seems like it might invite a new class of bugs. Hopefully, fish is careful when concurrently writing to the universal variables file, but the degree of care that's possible might depend on the user's OS, the kind of filesystem the file is on, etc.

@acomagu

acomagu commented Dec 14, 2025

Copy link
Copy Markdown
Owner

In previous versions, Universal Variable was used, but occasionally the prompt content was not reflected immediately, so we changed to the tmpfile strategy.
But we felt there were performance issues, and the problem occurred before the Rustification, so it's possible there are no longer any issues. We will leave this open, so if anyone has used this branch for some time, please share your experience here.
Thank you!

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.

4 participants