Skip to content
This repository has been archived by the owner on Feb 3, 2021. It is now read-only.

Implement Performance Tracking (Fixes #130) #182

Closed
wants to merge 1 commit into from

Conversation

Snuggle
Copy link
Collaborator

@Snuggle Snuggle commented Apr 7, 2019

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 <new_branch>)
  • 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)
  • 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):

image

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux

Checklist:

  • 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.

To-do: Have some way to compare the previous change with the current.
(master vs <new_branch>)
@Snuggle
Copy link
Collaborator Author

Snuggle commented Apr 7, 2019

I believe to add a similar comment to new PRs, we might have to use GitHub Actions like is done here: vercel/next.js#6752 (comment). I've signed up and in the waiting queue.

@Snuggle
Copy link
Collaborator Author

Snuggle commented Apr 7, 2019

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

@matchai
Copy link
Owner

matchai commented Apr 8, 2019

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
Copy link
Collaborator Author

Snuggle commented Apr 9, 2019

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
Copy link
Owner

matchai commented Apr 9, 2019

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
Copy link
Owner

matchai commented May 21, 2019

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 matchai closed this May 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance Tracking
2 participants