Skip to content

Commit

Permalink
Remove the check for a relative path
Browse files Browse the repository at this point in the history
This isn't a _huge_ perf boost, but every little helps :^)

In the old Pathname method, this if was useful because it was quicker
to skip files that were already resolved, assuming there was a mix of
relative and absolute paths

With File.realpath, however, it's quicker to just run the function as
it does nothing if the path is already resolved. This is roughtly
equal in terms of performance to using the new File.absolute_path?
method (they are so close in benchmarks that it's essentially noise)
  • Loading branch information
imjoehaines committed Jul 15, 2020
1 parent 0128907 commit f14f39a
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Changelog

* Improve performance when processing stacktraces
| [#602](https://github.com/bugsnag/bugsnag-ruby/pull/602)
| [#603](https://github.com/bugsnag/bugsnag-ruby/pull/603)

### Deprecated

Expand Down
5 changes: 1 addition & 4 deletions lib/bugsnag/stacktrace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ def initialize(backtrace, configuration)
next(nil) if file.nil?

# Expand relative paths
p = Pathname.new(file)
if p.relative?
file = File.realpath(file) rescue file
end
file = File.realpath(file) rescue file

# Generate the stacktrace line hash
trace_hash = {}
Expand Down
10 changes: 4 additions & 6 deletions spec/stacktrace_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,11 @@
configuration = Bugsnag::Configuration.new
configuration.send_code = false

dir = File.dirname(__FILE__)

backtrace = [
"/foo/bar/app/models/user.rb:1:in `something'",
"/foo/bar/other_vendor/lib/dont.rb:2:in `to_s'",
"/foo/bar/vendor/lib/ignore_me.rb:3:in `to_s'",
"/foo/bar/.bundle/lib/ignore_me.rb:4:in `to_s'",
"#{dir}/../spec/stacktrace_spec.rb:5:in `something_else'",
]

stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration).to_a
Expand All @@ -112,7 +109,6 @@
{ file: "/foo/bar/other_vendor/lib/dont.rb", lineNumber: 2, method: "to_s" },
{ file: "/foo/bar/vendor/lib/ignore_me.rb", lineNumber: 3, method: "to_s" },
{ file: "/foo/bar/.bundle/lib/ignore_me.rb", lineNumber: 4, method: "to_s" },
{ file: "#{dir}/../spec/stacktrace_spec.rb", lineNumber: 5, method: "something_else" },
])
end

Expand Down Expand Up @@ -140,20 +136,22 @@
configuration = Bugsnag::Configuration.new
configuration.send_code = false

dir = File.dirname(__FILE__)

backtrace = [
"./spec/spec_helper.rb:1:in `something'",
"./lib/bugsnag/breadcrumbs/../configuration.rb:100:in `to_s'",
"lib/bugsnag.rb:20:in `notify'",
"#{dir}/../spec/stacktrace_spec.rb:5:in `something_else'",
]

stacktrace = Bugsnag::Stacktrace.new(backtrace, configuration).to_a

dir = File.dirname(__FILE__)

expect(stacktrace).to eq([
{ file: "#{dir}/spec_helper.rb", lineNumber: 1, method: "something" },
{ file: "#{File.dirname(dir)}/lib/bugsnag/configuration.rb", lineNumber: 100, method: "to_s" },
{ file: "#{File.dirname(dir)}/lib/bugsnag.rb", lineNumber: 20, method: "notify" },
{ file: "#{dir}/stacktrace_spec.rb", lineNumber: 5, method: "something_else" },
])
end
end
Expand Down

0 comments on commit f14f39a

Please sign in to comment.