Giter Club home page Giter Club logo

Comments (23)

metacollin avatar metacollin commented on May 21, 2024 7

I figured out the problem. We're getting differing results because some of us have an old completion script for homebrew that is getting loaded in addition to the new one.

Everyone, check your $(brew --prefix)/etc/bash_completion.d/ directory for any other files starting with 'brew'. There should be exactly one file, which is called simply 'brew'. I found I had a 'brew_bash_completion.sh' and 'brew_bash_completion.sh.default' AND 'brew_bash_completion.sh.default.save' in that directory. They're leftovers from brew-legacy. Or something. I don't know where that .save came from. Maybe I caused it, or maybe other people have it, just check for any suspicious files with brew in the name. Leave only the file named 'brew'.

One removed, the speed issues immediately go away while all completions continue to work. You probably will want to open a fresh shell to test.

And @tdsmith is right, the brew bash completion script appears to have been optimized, so anyone who had an unpolluted bash_completion.d directory (bash_completion automatically loads everything in that directory) would see exactly what @tdsmith sees: a slightly faster brew tab completion.

The old completion scripts do some things that result in very inefficient completion if there are a large number of formulae in a tap. Before the split, this was fine, as there would not be a huge number of formulae there. Now however, having 3500 Formulae in the taps folder makes the old completion script basically unusable.

I have no idea how or why those old files are there, but I think its just a case of a tiny detail getting overlooked.

Man, I am so happy to have my completions back. Fast as ever now!

from brew.

MikeMcQuaid avatar MikeMcQuaid commented on May 21, 2024 5

@pavelrad @metacollin Glad to hear it. We could/should perhaps have Homebrew remove them for you.

from brew.

tdsmith avatar tdsmith commented on May 21, 2024

They were already unusably slow but I think maybe it got even worse?

from brew.

tdsmith avatar tdsmith commented on May 21, 2024

Oh, it's scanning Library/Formulas which is a symlink to the core tap now, and then it scans all the taps, so it's ~twice as bad as it used to be.

from brew.

tdsmith avatar tdsmith commented on May 21, 2024

also i'm pretty sure the algorithm used for taps is quadratic in the number of formulas

from brew.

pavelrad avatar pavelrad commented on May 21, 2024

This was very fast before all core formulae were moved to a separate core tap. Now it's very slow. Just compare to brew-cask completion, which takes just a moment to complete formula name.

from brew.

MikeMcQuaid avatar MikeMcQuaid commented on May 21, 2024

@pavelrad Casks are also in a tap so there's a bug here we'll need help fixing.

from brew.

pavelrad avatar pavelrad commented on May 21, 2024

Why this issue is closed? It's definitely not fixed yet.

from brew.

MikeMcQuaid avatar MikeMcQuaid commented on May 21, 2024

@pavelrad Because it's less slow than it was.

from brew.

tdsmith avatar tdsmith commented on May 21, 2024

In my subjective experience it's now much faster than it was before the split! I don't know why we're seeing different things.

from brew.

metacollin avatar metacollin commented on May 21, 2024

No, it is not fixed at all.

It used to take a couple hundred milliseconds.

It now takes 3-5 seconds of one of my 3.2GHz Xeon cores maxed out at 100% utilization. Performing tab completion in brew actually makes my fans spin up.

I am not sure what the exact mechanism is yet, but the issue is homebrew core being a tap. That's what is causing the unacceptably poor tab completion performance.

It's not the aliases, nor is it having to look through stuff twice (Formula, AND the tap). It's just the fact that is there at all. If you remove the Formula alias in /usr/local/Library/, as well as the Aliases alias, tap completion performance is not improved even slightly. It's still dog slow.

If you move the Formula directory out of the homebrew tap and anywhere else, then create a new symlink to it in the /usr/local/Library directory, homebrew's old 200ms tab completion performance immediately returns. Or just putting it back in its old spot works too. Whatever is going on, it has nothing to do with the number of formula, and everything to do with simply being in the taps folder.

from brew.

pavelrad avatar pavelrad commented on May 21, 2024

Great! Removing 'brew_bash_completion.sh' really helped with this issue, thank you, @metacollin.

from brew.

pavelrad avatar pavelrad commented on May 21, 2024

Yes, that would be nice to have some automatic cleanup of these old scripts.

from brew.

metacollin avatar metacollin commented on May 21, 2024

Thirded.

from brew.

UniqMartin avatar UniqMartin commented on May 21, 2024

We could/should perhaps have Homebrew remove them for you.

Any idea where to put this? I could imagine this being a (very lightweight) new check in brew doctor, but in this case we could only suggest the removal and not perform the removal automatically. (Since we're always asking for brew doctor output for troubleshooting, this might be sufficient.)

from brew.

MikeMcQuaid avatar MikeMcQuaid commented on May 21, 2024

@UniqMartin Feels like the sort of cleanup update-report.rb does?

from brew.

UniqMartin avatar UniqMartin commented on May 21, 2024

Feels like the sort of cleanup update-report.rb does?

Didn't think of it, but this sounds like the right place. I'm still a bit hesitant to automatically delete stuff in etc, even if we're pretty sure the files are created by Homebrew. I think so far we've refrained from doing something like this. (There might be a tiny minority of users who have customized etc/bash_completion.d/brew_bash_completion.sh and would be unhappy about it being silently wiped from their disk. Just wanted to mention this; I'm personally not very concerned that's a significant issue—unlike the tab completion slowness.)

from brew.

MikeMcQuaid avatar MikeMcQuaid commented on May 21, 2024

I think so far we've refrained from doing something like this.

I think we've actually taken more extreme measures previously:

brew/Library/brew.rb

Lines 81 to 84 in 0123e04

# Uninstall old brew-cask if it's still around; we just use the tap now.
if cmd == "cask" && (HOMEBREW_CELLAR/"brew-cask").exist?
system(HOMEBREW_BREW_FILE, "uninstall", "--force", "brew-cask")
end

from brew.

tdsmith avatar tdsmith commented on May 21, 2024

There's also the mv foo foo.bak approach. 👍 for auto-cleanup one way or another.

Probably paranoid, but would we want to replace it with a zero-length file in case someone's shell profile sources it without checking for existence so we don't break their terminal?

from brew.

UniqMartin avatar UniqMartin commented on May 21, 2024

There's also the mv foo foo.bak approach. 👍 for auto-cleanup one way or another.

We'd have to move it out of the directory, but I generally like the idea of moving files instead of wiping them. The question here would be where to move them to? Do we want to introduce the equivalent of lost+found for Homebrew and where would that live?

Probably paranoid, but would we want to replace it with a zero-length file in case someone's shell profile sources it without checking for existence so we don't break their terminal?

I think in this case it's better to break the profile, so that it can be fixed. The error message will be sufficiently descriptive and if people come to us with that error, we'll know what to do. If too many issues are created after doing this (I hope not), we can still create a dummy file (with a comment that it's obsoleted by brew) there to avoid being swamped.

from brew.

metacollin avatar metacollin commented on May 21, 2024

It's worth noting that uninstalling bash completion (either the actual package or from within homebrew) removes a large number of scripts from bash_completion.d/ and overwrites modifications etc. upon installation. bash_completion's documentation explicitly states that user modifications to installed completion scripts in that directory will not persist over upgrades, installations, or uninstalls. In my opinion, brew's expected behavior would be to overwrite any files in that folder that it created (even if it was quite a while ago) upon an update if needed. There is a separate folder for user modified or user created completions, $(brew --prefix)/etc/profile.d/.

But, I think that someone might sourcing that file directly without doing file test is actually a pretty valid concern, I've seen a lot of people who do some pretty terrible things in the name of 'ricing' their shell, like aliasing cp to call rsync, or having which python return a bash script that would return a path instead of a path, and I'm sure there is at least one unfortunate bastard out there who is sourcing this homebrew file without checking if it exists first, and he or she is of course doing it in their system-wide bash profile no less. Probably on a production machine used to keep baby kittens alive or something.

But, noting what I said in the first paragraph, we could simply overwrite the file with an empty one, thus solving the problem without causing Mr. or Mrs. Unfortunate Bastard (and their baby kittens) a very bad day.

An even safer option might be to only do that if it is unchanged from the original homebrew-legacy script (this would need to be a manual check I think - but it could just pull it in from github or a gist to do the check), and if there are changes, move it elsewhere, symlink it into bash_completion.d/, track it with git in its new home (since a .git folder in bash_completions.d/ would be no bueño), and emit the familiar 'git stash pop' warning about user changes. But, I also feel that homebrew would be completely within its bounds to overwrite that file with an empty one.

from brew.

apjanke avatar apjanke commented on May 21, 2024

Am out & about, so will give more feedback later, but one quick thing to be aware of: "bash completion" can mean two things in Homebrew land. There's brew's own support for bash completion, and there's the bash-completion formula, which is a separate project that provides completions for many commands (but not brew). These two things are independent. "Uninstalling bash completion" and the $(brew --prefix)/etc/profile.d sounds like you're referring to the bash-completion formula stuff. Brew's own bash completion support gets installed directly to /usr/local/etc/bash_completion.d/brew, should be independent of the bash-completion formula, and there is no support for uninstallation of it.

(There's a similar issue with zsh-completion vs "zsh completion". That was more complicated because zsh-completion did provide completions for brew, and then removed it when brew started shipping its own zsh completion support.)

The stuff about leaving user modifications alone to "bash_completion" might be from bash-completion's own doco. AFAIK, brew's bash completion does not support user modification at all. And, as a separate issue, the way we install bash-completion might be messing with their support for unmolested user modifications, but I don't think that's the problem this GitHub issue is addressing.

Will give this a close reading and feedback later tonight.

from brew.

MikeMcQuaid avatar MikeMcQuaid commented on May 21, 2024

This has been improved as much as it will be for now. If this is still broken for you: we'll accept PRs for this but we're not actively working on it at this time.

from brew.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.