This repository was archived by the owner on Nov 30, 2024. It is now read-only.
Memoize exception_lines in ExceptionPresenter #2743
Merged
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.
When an exception occurs and is displayed,
ExceptionPresenter
is used. However, there is an obvious slow amplification doing in it. This PR fixes it.The method
failure_lines
usesexception_lines
3 times, which usesexception.message
. In some situation,exception.message
can be slow (when inspect is called on a big object for example, such as during NoMethodError).Without caching, this delay, which can become considerable, will be amplified by 3x.
Here is a simple example of an rspec that would be amplified
Instead of taking 1 second, the print of the failure message takes 3 seconds.
I did memoization (
@exception_lines ||= ...
), but the call result could have been saved and reused in the failure_lines method. Let me know if you prefer that i change it to just startfailure_lines
withexception_lines = self.exception_lines
.As a side note, I think it would be helpful if the
--profile
or a new flag would help debug slow failures messages (top 10 of slowest failures to display).