-
Notifications
You must be signed in to change notification settings - Fork 16
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
core: Replace fold
in @go.printf
#80
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While starting to work on #59, and thinking about #79, I thought about whether I could dispense with the `fold` command altogether and whether it would have an impact on Windows performance in particular. The upshot is, the difference isn't dramatic, but since there's definitely a big performance hit and I'd prefer depending on as few external commands and subshells in general, I'm going to go ahead and commit the change. The nice feature is that the output will always be folded to the terminal width, since it no longer depends on the presence of `fold`. After implementing the folding algorithm first on macOS 10.12.2, I ran the following command (with `/bin` for Bash 3.2, and `/usr/local/bin` for Bash 4.4.5) to produce a timing comparison, since `./go help builtins` produces a fair amount of text printed with `@go.printf`: $ time PATH="/bin:$PATH" bash ./go help builtins The running times didn't appear impacted at all when `COLUMNS == 161`. When `COLUMNS == 40`, the new version was very very marginally slower, O(0.004s): Bash 3.2.57(1)-release, new implementation without `fold` real 0m0.062s user 0m0.046s sys 0m0.014s Bash 3.2.57(1)-release, new implementation without `fold` real 0m0.066s user 0m0.051s sys 0m0.013s Bash 4.4.5(1)-release, old implementation with `fold` real 0m0.058s user 0m0.041s sys 0m0.013s Bash 4.4.5(1)-release, new implementation without `fold` real 0m0.062s user 0m0.045s sys 0m0.013s On Windows 10, the results weren't super dramatic, but were generally an improvement for most Bash environments: Bash 4.3.46(2)-release, Git Bash from Command Prompt, old implementation with `fold`, COLUMNS=161 real 0m0.156s user 0m0.030s sys 0m0.106s Bash 4.3.46(2)-release, Git Bash from Command Prompt, new implementation with `fold`, COLUMNS=161 real 0m0.156s user 0m0.030s sys 0m0.075s Bash 4.3.46(2)-release, Git Bash from Command Prompt, old implementation with `fold`, COLUMNS=40 real 0m0.236s user 0m0.045s sys 0m0.090s Bash 4.3.46(2)-release, Git Bash from Command Prompt, new implementation with `fold`, COLUMNS=40 real 0m0.231s user 0m0.046s sys 0m0.153s Bash 4.3.46(2)-release, Git Bash, old implementation with `fold`, COLUMNS=161 real 0m0.148s user 0m0.030s sys 0m0.075s Bash 4.3.46(2)-release, Git Bash, new implementation with `fold`, COLUMNS=161 real 0m0.164s user 0m0.046s sys 0m0.107s Bash 4.3.46(2)-release, Git Bash, old implementation with `fold`, COLUMNS=40 real 0m0.202s user 0m0.030s sys 0m0.137s Bash 4.3.46(2)-release, Git Bash, new implementation with `fold`, COLUMNS=40 real 0m0.194s user 0m0.045s sys 0m0.091s Bash 4.3.48(8)-release, Cygwin Bash, old implementation with `fold`, COLUMNS=161 real 0m0.135s user 0m0.030s sys 0m0.030s Bash 4.3.48(8)-release, Cygwin Bash, new implementation without `fold`, COLUMNS=161 real 0m0.123s user 0m0.060s sys 0m0.015s Bash 4.3.48(8)-release, Cygwin Bash, old implementation with `fold`, COLUMNS=40 real 0m0.189s user 0m0.076s sys 0m0.091s Bash 4.3.48(8)-release, Cygwin Bash, new implementation without `fold`, COLUMNS=40 real 0m0.181s user 0m0.076s sys 0m0.046s Bash 4.3.11(1)-release, Windows Subsystem for Linux, old implementation with `fold`, COLUMNS=161 real 0m0.093s user 0m0.031s sys 0m0.063s Bash 4.3.11(1)-release, Windows Subsystem for Linux, new implementation without `fold`, COLUMNS=161 real 0m0.078s user 0m0.016s sys 0m0.063s Bash 4.3.11(1)-release, Windows Subsystem for Linux, old implementation with `fold`, COLUMNS=40 real 0m0.155s user 0m0.016s sys 0m0.125s Bash 4.3.11(1)-release, Windows Subsystem for Linux, new implementation without `fold`, COLUMNS=40 real 0m0.141s user 0m0.016s sys 0m0.109s
Realized after the fact that splitting a line with no blank characters appearing in the first `COLUMNS`, with a space appearing in the next `COLUMNS` characters, will result in the beginning of the next line losing characters.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While starting to work on #59, and thinking about #79, I thought about whether I could dispense with the
fold
command altogether and whether it would have an impact on Windows performance in particular.The upshot is, the difference isn't dramatic, but since there's definitely a big performance hit and I'd prefer depending on as few external commands and subshells in general, I'm going to go ahead and commit the change. The nice feature is that the output will always be folded to the terminal width, since it no longer depends on the presence of
fold
.After implementing the folding algorithm first on macOS 10.12.2, I ran the following command (with
/bin
for Bash 3.2, and/usr/local/bin
for Bash 4.4.5) to produce a timing comparison, since./go help builtins
produces a fair amount of text printed with@go.printf
:The running times didn't appear impacted at all when
COLUMNS == 161
. WhenCOLUMNS == 40
, the new version was very very marginally slower, O(0.004s):On Windows 10, the results weren't super dramatic, but were generally an improvement for most Bash environments: