Implement Performance Tracking (Fixes #130)

Snuggle commented 5 years ago

Description

To make sure that each additional feature added to Spacefish doesn't result in a slower and worse experience for the user. Also to make sure that we keep our code as efficient + quick as possible and keep the actual prompt section as light as possible.

If we get this working, we could upstream as much as possible to Spaceship. (I'm not sure if Zsh/bash has a similar way to profile functions in the same way that fish -p does.)

To-do:

[ ] Have some way to compare the master with the proposed pull request. (master vs )
[ ] Figure out some way to add a comment to the pull request with This pull requests adds +20ms on average to prompt initialisation. The slowest lines of code in the diff are <insert output>.

Motivation and Context

Closes #130.

Types of changes

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

Screenshots (if appropriate):

How Has This Been Tested?

[ ] I have tested using MacOS
[ ] I have tested using Linux

Checklist:

[x] I have checked that no other PR duplicates mine
[ ] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have updated the tests accordingly.

Snuggle commented 5 years ago

I believe to add a similar comment to new PRs, we might have to use GitHub Actions like is done here: https://github.com/zeit/next.js/pull/6752#issuecomment-475607460. I've signed up and in the waiting queue.

Snuggle commented 5 years ago

Maybe it is also a good idea to measure against fish without any prompt as a baseline.

matchai commented 5 years ago

Great idea! I really like how you surface the slowest lines of code. Could certainly help surface unintended performance blunders in releases. 😄

We could start by creating a separate GitHub Action for spacefish to consume, like how Zeit does it here: https://github.com/zeit/next.js/blob/ecf6134a7fedb1cafad1501e8089fd0fb1ca695b/.github/main.workflow#L12, which appears to reference the Action created here: https://github.com/zeit/next-stats-action

On an unrelated note, I've begun experimenting with writing a spacefish/spaceship in Rust, as was once proposed by Denys in a discussion on the spaceship repo. For that reason, I don't know how much we should invest into performance tooling in spacefish.

I'm hoping to have a proof-of-concept done by the end of the week with a few sections, which will then be ready to be shared. 😄

Snuggle commented 5 years ago

RUST. Heck, I've been looking for an excuse to get started learning Rust! It's been #1 at my want-to-learn list. Make sure to leave at least some for me! 😛

What would the scope be? I'm guessing you're not making a new shell, right? I think if we used Rust, Fish might be the slowpoke.

Either way, I'd love the look of Hyperfine and I'd like to keep it regardless, at least for historical performance data if nothing else.

matchai commented 5 years ago

I'm also using this as an excuse to learn Rust, so the initial implementation may be a little rusty and will probably need some refactoring as I get a better understanding of Rust. 😄

The plan is to re-create spaceship as a binary, which could be used to contain the prompt logic for any shell. For example, the fish prompt wrapper would be like this:

function fish_prompt
    spaceship $status
end

It should make for a prompt that is much faster, concurrent, testable and easily cross-platform and cross-shell.

For sure! Hyperfine looks like a good tool for the job.

matchai commented 5 years ago

Closing this PR now that we're focused on building and supporting starship/starship. Feel free to reopen this PR if you're still passionate about this feature. 🙂

matchai / spacefish