Optimize stack trace capture #138
Merged
Conversation
This file contains hidden or 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
added 4 commits
August 11, 2018 17:52
Neither of these variables were used anywhere else. Removing the functions that assigned them shaved O(0.2s) off of the running time of the entire suite on a Mid 2015 Macbook Pro with a 2.5GHz Intel Core i7 with 16GB RAM. Bash 3.2.57(1)-release before: real 0m6.959s user 0m3.259s sys 0m1.941s Bash 3.2.57(1)-release after: real 0m6.730s user 0m3.072s sys 0m1.921s Bash 4.4.23(1)-release before: real 0m7.309s user 0m3.197s sys 0m2.064s Bash 4.4.23(1)-release after: real 0m7.127s user 0m3.060s sys 0m2.047s
Eliminating some conditionals, the `frame` variable, and comparing the `test_file` and `funcname` values directly resulted in an O(0.1s) reduction in the running time of the entire suite on a Mid 2015 Macbook Pro with a 2.5GHz Intel Core i7 with 16GB RAM. Bash 3.2.57(1)-release before: real 0m6.544s user 0m3.157s sys 0m1.988s Bash 3.2.57(1)-release after: real 0m6.437s user 0m3.057s sys 0m1.985s Bash 4.2.23(1)-release before: real 0m6.950s user 0m3.150s sys 0m2.114s Bash 4.2.23(1)-release after: real 0m6.822s user 0m3.097s sys 0m2.091s
This improves the execution time only slightly, but reduces the complexity of the code by managing only one BATS_STACK_TRACE. We use BATS_LINENO to account for cases where BASH_LINENO isn't handled consistently between Bash versions, or when it's reset in the presence of unbound variable access errors. All relevant variables are now created in bats_perform_test as well.
This provides a very slight optimization over the original API, but makes the code a little bit cleaner.
|
Merging this now to get things moving; per usual, comment or open a new issue if any of these changes provide cause for concern. |
mbland
pushed a commit
that referenced
this pull request
Aug 19, 2018
Note that this commit also extracts the `bats_emit_trace` function and makes use of variables introduced by #138.
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.
With these changes, the current test suite runs O(0.2s-0.3s) faster on a Mid 2015 Macbook Pro with a 2.5GHz Intel Core i7 with 16GB RAM. Not all of the optimizations have the same degree of impact, but all make the code a little less complex and a bit more readable.
Most notably, there is now only the
BATS_STACK_TRACEvariable, rather thanBATS_CURRENT_STACK_TRACE,BATS_PREVIOUS_STACK_TRACE, andBATS_ERROR_STACK_TRACE. The only difference between the two previous stacks was the line number of the frame at the top of each. The stack trace is captured both before a command runs and after it triggers theERRorEXITtrap. Depending on the Bash version or type of error, theBASH_LINENOwould change when capturing the second stack trace. We now manage that difference viaBATS_LINENOandBATS_CURRENT_LINENO.Note that these timings are a little different than the ones in the commit messages, since I rebooted the machine before collecting these in order to ensure there was as little interference as possible from other processes and memory pressure.
Bash 3.2.57(1)-release before:
Bash 3.2.57(1)-release after:
Bash 4.2.23(1)-release before:
Bash 4.2.23(1)-release after: