Skip to content
New issue

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

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

Already on GitHub? # to your account

Reimplement the string-manipulation functions to write to variables instead of stdout #112

Merged
merged 1 commit into from
Sep 16, 2020

Conversation

dimo414
Copy link
Collaborator

@dimo414 dimo414 commented Sep 16, 2020

This change allows us to avoid unnecessary command substitutions which are (relatively speaking) quite expensive. Although __bp_sanitize_string is only used during installation __bp_trim_whitespace is invoked in a loop in the critical path as part of __bp_preexec_invoke_exec. Eliminating these subshells will reduce bash-preexec's overhead.

See dimo414/prompt.gem@9764cdb3 for another example of a similar change.

Note: the printf -v syntax was added to Bash in v3.1: https://mywiki.wooledge.org/BashFAQ/061

…nstead of stdout.

This change allows us to avoid unnecessary command substitutions which are (relatively speaking) quite expensive. Although __bp_sanitize_string is only used during installation __bp_trim_whitespace is invoked in a loop in the critial path as part of __bp_preexec_invoke_exec. Eliminating these subshells will reduce bash-preexec's overhead.

See dimo414/prompt.gem@9764cdb3 for another example of a similar change.

Note: the printf -v syntax was added to Bash in v3.1: https://mywiki.wooledge.org/BashFAQ/061
@rcaloras
Copy link
Owner

Wow! Playing with a bit locally timing my prompt and its looking like ~2x faster. @dimo414 thank you for this change 🙏 🎉 Much appreciated!

@rcaloras rcaloras merged commit 779efad into rcaloras:master Sep 16, 2020
@dimo414
Copy link
Collaborator Author

dimo414 commented Sep 16, 2020

Nice! That's even better than I expected. Thanks for the quick review!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants