ios: Optimize small memory reads when writing intermediate dumps. [crashpad/crashpad : main]
0 views
Skip to first unread message
Justin Cohen (Gerrit)
Mar 24, 2026, 11:56:38 AM (19 hours ago) Mar 24
to Crashpad LUCI CQ, Code Review Nudger, Mark Mentovai, crashp...@chromium.org
Justin Cohen added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 15 (Latest):
Justin Cohen . resolved
Thank you -- this is much simpler now.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Cohen (Gerrit)
Mar 24, 2026, 11:56:53 AM (19 hours ago) Mar 24
to Crashpad LUCI CQ, Code Review Nudger, Mark Mentovai, crashp...@chromium.org
Attention needed from Mark Mentovai
Justin Cohen added 1 comment![]( "Open in Gerrit")
File util/ios/ios_intermediate_dump_writer.h
Line 178, Patchset 9:
//! \brief Adds a property using `vm_read` for arbitrary-sized memory blocks.
//!
//! \return `true` if able to `vm_read` `value` `value_length` and write a
//! kProperty command with the `key` `value` `value_length` tuple.
//!
//! \note This method allocates a new VM page via `vm_read` in the calling
//! task's footprint. It should only be used for large memory reads
//! (e.g. thread stacks) where allocating a local stack buffer is unsafe.
bool AddPropertyBytesLarge(IntermediateDumpKey key,
const void* value,
size_t value_length);
//! \brief Adds a property using `vm_read_overwrite` and a local stack buffer.
//!
//! \return `true` if able to `vm_read_overwrite` `value` `value_length`
//! and write a kProperty command with the `key` `value` `value_length`
//! tuple.
//!
//! \note This method uses a `kMaxPropertyBytesSmallSize`-byte local stack
//! buffer to avoid allocating new virtual memory pages during the read.
//! `value_length` must be <= `kMaxPropertyBytesSmallSize`.
bool AddPropertyBytesSmall(IntermediateDumpKey key,
const void* value,
size_t value_length);
Mark Mentovai . resolved
Justin CohenCan `AddPropertyBytes` take care of making the decision on its own, so we don’t need to pollute all of the call sites?
(Then `kMaxPropertyBytesSmallSize` wouldn’t need to be exposed either.)
Done
Related details
Attention is currently required from:
- Mark Mentovai
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mar 24, 2026, 11:58:19 AM (19 hours ago) Mar 24
to Justin Cohen, Crashpad LUCI CQ, Code Review Nudger, crashp...@chromium.org
Attention needed from Justin Cohen
Mark Mentovai voted and added 1 comment![]( "Open in Gerrit")
Votes added by Mark Mentovai
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Justin Cohen
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Cohen (Gerrit)
Mar 24, 2026, 3:47:12 PM (15 hours ago) Mar 24
to Mark Mentovai, Crashpad LUCI CQ, Code Review Nudger, crashp...@chromium.org
Justin Cohen voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Crashpad LUCI CQ (Gerrit)
Mar 24, 2026, 4:10:14 PM (15 hours ago) Mar 24
to Justin Cohen, Mark Mentovai, Code Review Nudger, crashp...@chromium.org
Crashpad LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
ios: Optimize small memory reads when writing intermediate dumps.
This CL optimizes memory reads by using `vm_read_overwrite` with a
512-byte stack buffer for small properties, avoiding unnecessary virtual
memory allocations. Larger reads continue to use `vm_read`.
The logic is encapsulated within `AddPropertyBytes`, which automatically
selects the optimal read method based on the property size.
This also adds a test to verify `CaptureMemoryPointedToByThreadState`.
Bug: 40846915
Change-Id: I2013329ce719b2bbcb61ab83fb3c77e8af825ff2
Reviewed-by: Mark Mentovai <ma...@chromium.org>
Commit-Queue: Justin Cohen <justi...@google.com>
Files:
- M client/ios_handler/in_process_intermediate_dump_handler_test.cc
- M util/ios/ios_intermediate_dump_writer.cc
- M util/ios/ios_intermediate_dump_writer.h
Change size: M
Delta: 3 files changed, 96 insertions(+), 18 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mark Mentovai
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages