Closed Bug 1519990 Opened 3 years ago Closed 2 years ago

Improve mach autocompletion performance

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set
normal

Tracking

(firefox83 fixed)

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

Currently the bash completion script [1] calls mach mach-completion to find the targets for each command/subcommand. But this incurs python and mach startup overhead, which is especially noticeable when tab-completing.

One idea would be to save the targets to disk and only run 'mach' to (re)generate them.

I'd love to improve the 'mach' startup performance in general, and any wins there will improve the situation here.

[1] https://searchfox.org/mozilla-central/source/python/mach/bash-completion.sh

Duplicate of this bug: 1521482
Depends on: 1388894

This has long been a pet peeve of mine, so I've started poking around at this. I have a WIP patch that can generate an incomplete zsh completion script based on:
https://github.com/sdispater/cleo/blob/master/cleo/commands/completions_command.py

Setup would be something like:

$ ./mach mach-completion zsh > ~/.zfunc/_mach

Then in ~/.zshrc:

fpath += ~/.zfunc
autoload -U compinit && compinit

So far the results are looking promising, completion is instant! Though I'll need to fix some bugs, support subcommands and get it working for other shells (bash and fish at least) before it's ready for review.

Caveat: This is definitely a side project and I don't know how much time I'll be able to spend on it. There's a good chance I'll end up dropping this due to lack of time. But for now assigning to myself so others don't duplicate effort. If this ends up getting stalled out and you'd like to take over, please ping me and I'll send you my latest WIP patch.

Assignee: nobody → ahal
Status: NEW → ASSIGNED

Supported shells are 'bash', 'zsh' and 'fish'. Here are installation
instructions with various shells and contexts:

bash

$ mach mach-completion bash > mach.bash-completion
$ sudo mv mach.bash-completion /etc/bash_completion.d

bash (homebrew)

$ mach mach-completion bash > $(brew --prefix)/etc/bash_completion.d/mach.bash-completion

zsh

$ mach mach-completion zsh > ~/.zfunc/_mach
$ mkdir ~/.zfunc

then edit ~/.zshrc and add:

fpath += ~/.zfunc
autoload -U compinit && compinit

zsh (oh-my-zsh)

$ mkdir $ZSH/plugins/mach
$ mach mach-completion zsh > $ZSH/plugins/mach/_mach

then edit ~/.zshrc and add 'mach' to your enabled plugins:

plugins(mach ...)

zsh (prezto)

$ mach mach-completion zsh > ~/.zprezto/modules/completion/external/src/_mach

fish

$ ./mach mach-completion fish > ~/.config/fish/completions/mach.fish

fish (homebrew)

$ ./mach mach-completion fish > (brew --prefix)/share/fish/vendor_completions.d/mach.fish

Depends on D90415

Blocks: 1519858
Duplicate of this bug: 1536437

This seems to work great with bash, zsh and fish!

Still left to do:

  • Support subcommands
  • Add tests
  • Bootstrap integration
  • Support tcsh (bug 1519858)

The first two I hope to land along with this patch series. The latter two will be follow-ups (and likely put on the backburner indefinitely).

This was causing us to generate the build backend on 'tab' completion (since we
were importing the file to parse available options out of the ArgumentParser
defined therein).

I've managed to support subcommands! So just documentation and tests left to do in this bug. I think I'd like to look into bootstrap integration before advertising this, but it'll be a follow-up either way.

Attachment #9176055 - Attachment description: Bug 1519990 - [mach] Some light refactoring in commandinfo.py → Bug 1519990 - [mach] Some light refactoring in commandinfo.py, r?#firefox-build-system-reviewers
Attachment #9176056 - Attachment description: Bug 1519990 - [mach] Add ability to generate completion scripts via 'mach mach-completion <shell>' → Bug 1519990 - [mach] Add ability to generate completion scripts via 'mach mach-completion <shell>', r?#firefox-build-system-reviewers
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb0fc2efe6e7
[tryselect] Don't perform expensive computation on import from 'coverage' selector, r=marco
https://hg.mozilla.org/integration/autoland/rev/b6feb8c69d77
[mach] Some light refactoring in commandinfo.py, r=mhentges
https://hg.mozilla.org/integration/autoland/rev/93a35f9c6739
[mach] Add ability to generate completion scripts via 'mach mach-completion <shell>', r=mhentges
Blocks: 1670288
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

Looks like this broke unit tests in CI? (example)

Flags: needinfo?(ahal)
Regressions: 1670385

Oops, I'll address in bug 1670385.

Flags: needinfo?(ahal)
No longer depends on: 1388894
You need to log in before you can comment on or make changes to this bug.