Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Mac support #10

Merged
merged 5 commits into from
Dec 6, 2019
Merged

Add Mac support #10

merged 5 commits into from
Dec 6, 2019

Conversation

nnethercote
Copy link
Contributor

This is a lot more complex than the Linux and Windows support. First, we
must handle fat binaries, which contain code for multiple architectures,
using the symbolic_debuginfo::Archive type. Second, we must consult
the binary's symbol table and then read debug info from the object files
and archive files mentioned. We use the goblin crate for this.

This makes the code a little simpler, and will also make it easier to
split `build_file_info` into more functions.
For the reasons explained in the new comment.
This makes it easier to read, and will be helpful for adding Mac
support.
Because I decided I prefer it this way.
@nnethercote
Copy link
Contributor Author

r? @gabrielesvelto

Copy link
Collaborator

@gabrielesvelto gabrielesvelto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and I'm eager to see this being used. The comments I left are mostly questions or suggestions to make the code more readable but don't consider them as blocking. This can land as is and be refactored later.

Cargo.toml Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
This is a lot more complex than the Linux and Windows support. First, we
must handle fat binaries, which contain code for multiple architectures,
using the `symbolic_debuginfo::Archive` type. Second, we must consult
the binary's symbol table and then read debug info from the object files
and archive files mentioned. We use the `goblin` crate for this.
@nnethercote
Copy link
Contributor Author

I forgot to mention that this is about 10x faster than fix_macosx_stack.py: for the DMD output file I was testing with the processing time drops from 6 minutes to 40 seconds on my 2013 MacBook Pro. Not as good as the 100x speed-up we saw on Linux, but still good.

@nnethercote
Copy link
Contributor Author

Thanks for the quick review, @gabrielesvelto.

@nnethercote nnethercote merged commit c65a202 into mozilla:master Dec 6, 2019
@nnethercote nnethercote deleted the mac-support branch December 6, 2019 00:38
@nnethercote nnethercote mentioned this pull request Dec 6, 2019
@gabrielesvelto
Copy link
Collaborator

I forgot to mention that this is about 10x faster than fix_macosx_stack.py: for the DMD output file I was testing with the processing time drops from 6 minutes to 40 seconds on my 2013 MacBook Pro. Not as good as the 100x speed-up we saw on Linux, but still good.

That's still an order of magnitude faster! Also if you're on a 2013 MacBook Pro your I/O calls will be slowed down by various Meltdown mitigations, it's possible that a more recent machine will be a fair bit faster than that.

gabrielesvelto pushed a commit to gabrielesvelto/fix-stacks that referenced this pull request Feb 3, 2021
Allow pdb in cabinet archive as input
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants