[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D62503#1518839 <https://reviews.llvm.org/D62503#1518839>, @labath wrote:

> It sounds like these improved reads would be beneficial for all kinds of reads, and not just for reading c strings. Could this chunking logic be implemented directly in ReadMemory?


We should probably do this in ReadMemory directly to ensure we have the fastest memory reads possible for all situations. Then ignore my inline comment.

> That would also allow you to test this by sending `m` packets which are deliberately chosen to cross readable/unreadable page boundaries in interesting ways...

BTW: LLDB will usually always send down aligned reads due to it populating its memory cache.



================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1502-1504
+Status NativeProcessLinux::ReadCStringFromMemory(lldb::addr_t addr,
+                                                 char *buffer, size_t max_size,
+                                                 size_t &total_bytes_read) {
----------------
Seems like this function could be added to the NativeProcess base class? We would need to add "virtual size_t NativeProcess::GetCacheLineSize() = 0;" to NativeProcess too in that case.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2222-2224
+  error = ReadCStringFromMemory(link_map.l_name,
+                                reinterpret_cast<char *>(&name_buffer),
+                                sizeof(name_buffer), bytes_read);
----------------
do we really need to read PATH_MAX each time?Seems like ReadCStringFromMemory could read 32 bytes at a time and check for NULL characters along the way and making sure we don't cross page boundaries?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62503/new/

https://reviews.llvm.org/D62503