1519990 - Improve mach autocompletion performance

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Andrew Halberstadt [:ahal]]

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

Comment 2

[Andrew Halberstadt [:ahal]]

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.

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1519990 - [mach] Some light refactoring in commandinfo.py, r?#firefox-build-system-reviewers

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1519990 - [mach] Add ability to generate completion scripts via 'mach mach-completion <shell>', r?#firefox-build-system-reviewers

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

Comment 6

[Andrew Halberstadt [:ahal]]

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

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1519990 - [tryselect] Don't perform expensive computation on import from 'coverage' selector, r?marco

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

Comment 8

[Andrew Halberstadt [:ahal]]

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.

Comment 9

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1519990 - [mach] Add better tests for command completion

Depends on D90416

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/bb0fc2efe6e7
https://hg.mozilla.org/mozilla-central/rev/b6feb8c69d77
https://hg.mozilla.org/mozilla-central/rev/93a35f9c6739

Comment 12

[Ricky Stewart]

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

Comment 13

[Andrew Halberstadt [:ahal]]

Oops, I'll address in bug 1670385.