Trim dependencies of addr2line for backtrace
#160
Conversation
This commit trims the dependencies of `addr2line` to be used in the `backtrace` crate. The purpose here is to slowly trim down the list of dependencies to make it easier to eventually include this in the standard library. Fow now this handles three previously required dependenies of `addr2line`: * `smallvec` - this is made optional where if not present a standard `alloc::vec::Vec<T>` is used. * `fallible-iterator` - this is simply made optional. * `lazycell` - this is dropped in favor of a small hand-rolled implementation. The new features are enabled by default and the thinking is that the `backtrace` crate would disable all these features by default. If the `std` feature of `backtrace` is activated it'd activate these features, however.
| // method which will only perform mutation if we aren't already | ||
| // `Some`. | ||
| let val = closure(); | ||
| (*ptr).get_or_insert(val) |
There was a problem hiding this comment.
| (*ptr).get_or_insert(val) | |
| (*self.contents.get()).get_or_insert(val) |
If borrow_with is called from inside the closure, then ptr is invalidated according to stacked borrows I think.
There was a problem hiding this comment.
I don't believe this is necessary since the value of ptr doesn't change, and borrows into it are temporary. If miri or something throws an error here it can of course be changed.
There was a problem hiding this comment.
ptr is of course a raw pointer. The following worked fine in miri:
let c = LazyCell::new();
c.borrow_with(|| {
*c.borrow_with(|| 0)
});
There was a problem hiding this comment.
@bjorn3 I'm taking your comment to mean that this conversation is resolved.
| let ptr = self.contents.get(); | ||
| if let Some(val) = &*ptr { |
There was a problem hiding this comment.
| let ptr = self.contents.get(); | |
| if let Some(val) = &*ptr { | |
| if let Some(val) = &*self.contents.get() { |
There was a problem hiding this comment.
Looks good.
I checked that smallvec is still a gain here. Is backtrace-rs ok with giving that up?
name vec ns/iter smallvec ns/iter diff ns/iter diff % speedup
context_query_with_functions_rc 3,333,028 3,035,749 -297,279 -8.92% x 1.10
context_query_with_functions_slice 2,956,228 2,763,440 -192,788 -6.52% x 1.07
| // method which will only perform mutation if we aren't already | ||
| // `Some`. | ||
| let val = closure(); | ||
| (*ptr).get_or_insert(val) |
There was a problem hiding this comment.
@bjorn3 I'm taking your comment to mean that this conversation is resolved.
|
I think a small perf hit is probably ok, but I'll measure one more time before merging all updates to confirn |
This commit trims the dependencies of
addr2lineto be used in thebacktracecrate. The purpose here is to slowly trim down the list ofdependencies to make it easier to eventually include this in the
standard library. Fow now this handles three previously required
dependenies of
addr2line:smallvec- this is made optional where if not present a standardalloc::vec::Vec<T>is used.fallible-iterator- this is simply made optional.lazycell- this is dropped in favor of a small hand-rolledimplementation.
The new features are enabled by default and the thinking is that the
backtracecrate would disable all these features by default. If thestdfeature ofbacktraceis activated it'd activate these features,however.