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

Support faster methods of reading process memory #118

Merged
merged 11 commits into from
Aug 17, 2024
Merged

Conversation

Jake-Shadle
Copy link
Collaborator

@Jake-Shadle Jake-Shadle commented Apr 25, 2024

This adds 2 new methods for reading external process memory, in addition to the existing PTRACE_PEEKDATA approach currently used.

  1. process_vm_readv - Reads contiguous blocks of a specified size, available since Linux 3.2, so realistically available in every reasonable environment...
  2. /proc/{pid}/mem - As a fallback in case running in an ancient Linux, we can read from this procfs file, which also allows easy reading of blocks of memory.
  3. PTRACE_PEEKDATA - The reliable but extremely slow fallback

These 3 methods are probed in the order above until one succeeds, as they all require the same permissions.

Resolves: #72

@gabrielesvelto
Copy link
Contributor

I'm testing this in Firefox today, I want to get a feel of the speedup and make sure everything is working alright.

@Jake-Shadle
Copy link
Collaborator Author

Cool, I didn't actually do any benchmarking as this was just a side task while doing other stuff, so it'd be good to see in a real world scenario if this moves the needle (I would hope it would when doing 1 syscall vs n syscalls :p)

@gabrielesvelto
Copy link
Contributor

Didn't have time to review properly but I did test it, and it's looking good! On my machine, using a debug build of Firefox it takes 160-180ms to grab a minidump w/o this patch. With this applied it drops to 50-75ms. I hope to have time to test an optimized build tomorrow. I also tried on a cheap Android tablet but the I/O is so slow there that the improvements are lost in the noise.

@Jake-Shadle
Copy link
Collaborator Author

I guess one thing to note is that I don't think the file path is being taken in any test so I'm not sure about that one, I'll add a test specifically for that before merging.

@gabrielesvelto
Copy link
Contributor

Sorry for the super-long delay here but I've finally had time to come back to this. I've tested this extensively on Linux builds and I see at minimum a speedup of an order of magnitude. When running Firefox crash generation tests in debug builds the speedup is 25 times (you read that right, ~12ms with the patch applied vs ~300ms without it). This is going to be extremely useful.

I didn't have time to test Android again but I expect larger gains there (because syscalls are slower) but maybe a smaller impact in the overall time because I/O is also slower and takes a larger portion of the time.

@gabrielesvelto
Copy link
Contributor

@Jake-Shadle if you don't have other changes to make we can merge it today.

@Jake-Shadle
Copy link
Collaborator Author

Ahh sounds good, I just want to add a test specifically for the file case since that will never currently be hit on CI/my local machine, then we can merge.

src/linux/mem_reader.rs Outdated Show resolved Hide resolved
@Jake-Shadle Jake-Shadle merged commit d5726d3 into main Aug 17, 2024
16 checks passed
@Jake-Shadle Jake-Shadle deleted the experiment branch August 17, 2024 06:26
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.

Speed up reading the target process memory on Linux
2 participants