backtrace-rs icon indicating copy to clipboard operation
backtrace-rs copied to clipboard

Optimize proc maps parsing code size

Open Noratrieb opened this issue 3 months ago • 5 comments

The current code size is really wastefully large. Originally, it was 1500 lines of assembly in Godbolt, now I reduced it to just under 800. The effect of .text size in hello world is from 297028 to 295453 (measured with -Clto=fat -Copt-level=s with a normal sysroot) which is small but not completely irrelevant. It's just a small fish in the bigger pond of DWARF parsing, but it's better than nothing.

I extracted the parsing of each component into a separate function to allow for better sharing. I replaced the string methods with manual iteration since that results in simpler code because it has to handle fewer cases. I also had to use unsafe because the bounds checks were sadly not optimized out and were really large.

I also made the parser less resilient against whitespace, now it no longer handles Unicode whitespace (an obvious simplification) but also no longer handles any whitespace except the normal SP. I think this is fine, it seems highly unlikely that a system would suddenly use another type of whitespace (but I guess not impossible?).

Another simplification was simply removing the parsing of unused fields that were not needed.

I can split it into separate commits if that helps review and maybe some of these changes are more of a hassle than it's worth (while some others like the field removals are obviously good), but I'll let you choose.

Noratrieb avatar Sep 23 '25 19:09 Noratrieb

I think we can safely assume that procfs will only ever use '\x20', yes.

This will require careful review for the unsafe usage, but I am fairly happy to see it. I made a similar pass at this but wasn't quite happy with it.

workingjubilee avatar Sep 23 '25 19:09 workingjubilee

apparently it's normal that these tests fail, lol

Noratrieb avatar Sep 23 '25 19:09 Noratrieb

I am certainly treating the current macOS failures as nonblocking as it is very possible a rustc change is the underlying reason. I have opened https://github.com/rust-lang/backtrace-rs/issues/730

workingjubilee avatar Sep 23 '25 19:09 workingjubilee

I can split it into separate commits if that helps review

@Noratrieb If you can cleanly split the changes that micro-optimize the parsing using bytes and such from those that do straight reduction of code, like removing fields, into separate commits, that would be appreciated. Order doesn't matter. Though if they don't have a reasonably clean split then it doesn't matter and it should remain one big mash.

workingjubilee avatar Sep 23 '25 20:09 workingjubilee

@workingjubilee is this still in your queue or did this fall through the cracks?

Noratrieb avatar Oct 31 '25 18:10 Noratrieb