pmap: implemented more-extended and most-extended options #415
Conversation
| // | ||
| // 1234: /some/path | ||
| // Address Perm Offset Device Inode Size Rss Pss Pss_Dirty Referenced Anonymous LazyFree ShmemPmdMapped FilePmdMapped Shared_Hugetlb Private_Hugetlb Swap SwapPss Locked THPeligible Mapping | ||
| // 000073eb5f4c7000 r-xp 0000000000036000 008:00008 2274176 1284 1148 1148 0 1148 0 0 0 0 0 0 0 0 0 0 ld-linux-x86-64.so.2 |
There was a problem hiding this comment.
There are some differences with leading zeros compared to the output of the original pmap:
- for the
Address, the originalpmapdoesn't show leading zeros, it uses spaces instead - the length of the
Offsetis eight characters with the originalpmap - for the
Device, the originalpmapuses thexx:xxformat
I don't know whether you want to fix it in this PR. As the values themselves are correct (and the PR is already on the bigger side), it's also ok to add a todo.
There was a problem hiding this comment.
Yes, I'm aware of these differences and plan to address them later, separate from this PR. (Since it seems you originally authored these lines, please let me know if you recall any specific intentions for implementing them as they are now.)
There was a problem hiding this comment.
I implemented them to work for the default, extended (-x), and device (-d) formats. The more-extended and most-extended formats unfortunately differ in how some of the same data is shown.
There was a problem hiding this comment.
Thank you, understood. Because managing those different formats would also be related to the range option, please allow me to address it independently of this PR.
| // | ||
| // 1234: /some/path | ||
| // Address Perm Offset Device Inode Size KernelPageSize MMUPageSize Rss Pss Pss_Dirty Shared_Clean Shared_Dirty Private_Clean Private_Dirty Referenced Anonymous LazyFree AnonHugePages ShmemPmdMapped FilePmdMapped Shared_Hugetlb Private_Hugetlb Swap SwapPss Locked THPeligible VmFlags Mapping | ||
| // 000073eb5f4c7000 r-xp 0000000000036000 008:00008 2274176 1284 4 4 1148 1148 0 0 0 1148 0 1148 0 0 0 0 0 0 0 0 0 0 0 rd ex mr mw me ld-linux-x86-64.so.2 |
There was a problem hiding this comment.
See my other comment about leading zeros.
| pub vmflags: bool, | ||
| pub mapping: bool, | ||
| // Misc | ||
| pub custom_format_enabled: bool, |
There was a problem hiding this comment.
Hm, what's the reason for this property?
There was a problem hiding this comment.
This property is used to determine which output format to choose in pmap main. The more-extended and most-extended options seem to share the same format as the read-rc options (used for output field customization), while the extended, device, and default options have their own formats. This property will also be used when implementing the read-rc options.
There was a problem hiding this comment.
I'm not sure whether this property should be part of PmapConfig. At least conceptually it is different from all the other properties: they are about enabling a specific field whereas custom_format_enabled, if true, is more like a format like more-extended. And you could say a "custom format" has a PmapConfig. On the other hand it's also possible I misunderstand something :)
There was a problem hiding this comment.
I would include this property in PmapConfig because it essentially corresponds to a pmaprc configuration file. When you look at a pmaprc file generated by pmap -N, you'll see two categories of configurations: [Fields Display], which we're currently dealing with, and [Mapping], which is yet to be implemented. From this, we understand that pmaprc controls which fields to print and how to print them. I realize it might seem a bit odd to have custom_format_enabled in PmapConfig right now, but once we add the show_path property in later PRs, it will make sense to have custom_format_enabled here, wouldn't it?
There was a problem hiding this comment.
Ok, let's try it :) We can always refactor it later if necessary.
|
I plan to continue with the review of the other files tomorrow. |
Co-authored-by: Daniel Hofstetter <[email protected]>
Co-authored-by: Daniel Hofstetter <[email protected]>
|
Thanks! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #415 +/- ##
===========================
===========================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Added support for the more-extended and most-extended options.