Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Takes 1-2 seconds before it shows the message #108

Open
TonalidadeHidrica opened this issue Jul 11, 2022 · 10 comments
Open

Takes 1-2 seconds before it shows the message #108

TonalidadeHidrica opened this issue Jul 11, 2022 · 10 comments
Labels
help wanted We want help addressing this

Comments

@TonalidadeHidrica
Copy link

Every time I run an invalid command, it takes about 1-2 seconds before it shows the candidates of packages or shows the "Unknown command" message. The experience is far worse than the counterpart in, say, Ubuntu, where the results show up in like <0.1 seconds. I'm so impatient that I immediately type Ctrl-C once I recognized I typed a wrong command before the results are output. Now I'm going to disable this feature... Is there a room of improvement? If it is easy enough, maybe I can contribute.

@Rylan12
Copy link
Member

Rylan12 commented Jul 11, 2022

Personally, I don't use the handle to get feedback after every command. Instead, I will just run brew which formula

The reason this is slow is that the logic that figures out which formulae to suggest is in Ruby, so every time you mistype a command the entire brew environment is started. The way to fix this is to rewrite the brew which-formula command in bash (like the built-in update or --version commands). That way, the command could be executed each time without spinning up a while Homebrew ruby environment. I don't think this would be as fast as the <0.1 seconds from Ubuntu, but it would make a big difference:

$ hyperfine 'brew --version' 'brew --cellar wget' --warmup 2
Benchmark 1: brew --version
  Time (mean ± σ):     120.0 ms ±   2.1 ms    [User: 44.3 ms, System: 66.9 ms]
  Range (min … max):   117.4 ms … 125.1 ms    24 runs

Benchmark 2: brew --cellar wget
  Time (mean ± σ):     524.5 ms ±   4.6 ms    [User: 343.9 ms, System: 160.0 ms]
  Range (min … max):   516.9 ms … 532.6 ms    10 runs

Summary
  'brew --version' ran
    4.37 ± 0.09 times faster than 'brew --cellar wget'

brew --version is a bash command and brew --cache wget is a ruby command of similar complexity.


If you or someone else is interested in helping, a PR would be welcome! Most of the logic isn't super complicated since it's pretty much just "check the executables.txt file for the given binary and then suggest those formulae". The only part that may be problematic is the WhichFormula::reject_formula? method since it makes some checks that we can't directly make in bash (e.g. whether the latest version of a formula is installed and whether requirements are satisfied). However, I think dropping those conditions wouldn't be the end of the world if it makes using this feature better for most cases.


Finally, I'd also say that what I do on my machine is just manually run brew which-formula whenever I need this functionality (I've aliased this to brew which to make it even shorter). That way, I can still use this feature but don't need to worry about it taking so long and only get the information when I need it (i.e. not for typos).

@Rylan12 Rylan12 added the help wanted We want help addressing this label Jul 24, 2022
@apainintheneck
Copy link

I'm pretty sure that only internal commands like brew --prefix get to bypass Rubyland. I think that external commands don't get the benefit of that even when they're just Bash scripts.

@Rylan12
Copy link
Member

Rylan12 commented Nov 24, 2022

Hmm, is that so? In that case, yeah, my proposed solution would not work. There might be some other improvements that can be made to the search, but it will still be limited by the rube environment set-up time which just takes a really long time.

@oneElectron
Copy link

oneElectron commented Jan 19, 2024

I got tired of this too, but I didn't see this issue until I had already kind of made my solution, so I don't know if this solution is acceptable.

I re-implemented the which-formula command in rust, and because of this the program is separate from brew itself (I call it brew_which_formula). To use it instead of brew which-formula I changed what command was called in the handler.fish script. I imagine that if this were to be accepted that the script would check to see if brew_which_formula was installed, and to fall back to the included brew which-formula if it isn't.

This most likely is not what this issue was looking for especially because it's a binary instead of a script.

Should I PR this?
If so, I assume that this formula would be housed in this tap, so I would need to make a new Formula folder.

Benchmarks

It is slightly faster.

$ hyperfine "brew which-formula --explain wget" "brew_which_formula --explain wget"
Benchmark 1: brew which-formula --explain wget
  Time (mean ± σ):     840.2 ms ±  18.6 ms    [User: 586.6 ms, System: 91.3 ms]
  Range (min … max):   796.3 ms … 867.8 ms    10 runs

Benchmark 2: brew_which_formula --explain wget
  Time (mean ± σ):       1.6 ms ±   0.2 ms    [User: 1.4 ms, System: 0.2 ms]
  Range (min … max):     1.3 ms …   2.5 ms    596 runs

  Warning: Command took less than 5 ms to complete. Note that the results might be inaccurate because hyperfine can not calibrate the shell startup time much more precise than this limit. You can try to use the `-N`/`--shell=none` option to disable the shell completely.
  Warning: The first benchmarking run for this command was significantly slower than the rest (2.5 ms). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run.

Summary
  brew_which_formula --explain wget ran
  536.24 ± 61.47 times faster than brew which-formula --explain wget

@apainintheneck
Copy link

@oneElectron Very cool! Could you share a link to your approach?

I doubt we'd want to start adding formulae to this repository. The best approach to including this in Brew by default would be to rewrite it in Bash and upstream it to the main Brew project so that it can bypass the Ruby runtime which is slow to start. Even in that case it would likely take 50+ milliseconds instead of just a couple like you've shown in your example. It still would be a big performance improvement over what's currently in this repo though. The problem is that I'm not sure that there's really any appetite to add this to the main Brew repo assuming that the Bash script approach is performant enough.

The simplest thing to do would be to create your own formula for your personal project so that people could install it from a third party tap.

@oneElectron
Copy link

I kind of forgot to make a repo, but here it is: https://github.com/oneElectron/brew-command-not-found.

I will eventually find a solution to the hack that is required to get it to work, but this is only a proof of concept at this point.
If there is no appetite for this is this repo, then I will probably just add it to my own third party repo when it gets user friendly enough.

@oneElectron
Copy link

@apainintheneck It does kind of surprise me that this might not be accepted into the main brew repo. I the only issue that I see with just having the formula is that it requires this repo to be tapped. I don't know much about the approval process for formulae, so could you give me some insight on what guideline this breaks?

@oneElectron
Copy link

Also I don't know anything about bash programming, so I can't really make that solution.

@apainintheneck
Copy link

I kind of forgot to make a repo, but here it is: https://github.com/oneElectron/brew-command-not-found.

Very cool! Thanks for sharing.

@apainintheneck It does kind of surprise me that this might not be accepted into the main brew repo. I the only issue that I see with just having the formula is that it requires this repo to be tapped. I don't know much about the approval process for formulae, so could you give me some insight on what guideline this breaks?

By the main Brew repo I meant https://github.com/Homebrew/brew. This repo is mostly Ruby with some Bash startup scripts and the occasional Swift file for macOS compatibility. It's unlikely that we'd start to add programs written in other languages to that repo especially if they need to be compiled.

In general, we try to ship binaries for most platforms so that people don't have to build from source. Right now the only binary we ship for all platforms with the main Brew app is the Ruby runtime itself.

I don't think there would be any problems with adding this to https://github.com/Homebrew/homebrew-core beyond the popularity requirement for inclusion. See https://docs.brew.sh/Acceptable-Formulae#niche-or-self-submitted-stuff.

Also I don't know anything about bash programming, so I can't really make that solution.

Yeah, that was mostly just me brainstorming. I doubt that people would be willing to add this to the main repo either but technically it would be more likely to be accepted because it doesn't require a new programming language to be added to the repo. It would also be much faster since the Bash start up time is quicker than Ruby. That's why commands like brew --cache and brew --repo are much faster than commands like brew --help.

Thinking about this a bit more it's be great if we could bypass the Ruby runtime entirely for all self-contained binary scripts in taps but that would require a non-trivial refactor of some existing Ruby code to Bash to make that work.

@MikeMcQuaid
Copy link
Member

It's unlikely that we'd start to add programs written in other languages to that repo especially if they need to be compiled.

To be clear: it is 100% unlikely.

I don't think there would be any problems with adding this to https://github.com/Homebrew/homebrew-core beyond the popularity requirement for inclusion. See https://docs.brew.sh/Acceptable-Formulae#niche-or-self-submitted-stuff.

Agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We want help addressing this
Projects
None yet
Development

No branches or pull requests

5 participants