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

Mon Jun 17 00:10:22 PDT 2019

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

LGTM, modulo the inline comments.

================
Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:25-26
 // NativeProcessProtocol Members
+const size_t NativeProcessProtocol::g_cache_line_size =
+    llvm::sys::Process::getPageSizeEstimate();

----------------
Make this a local static in `ReadCStringFromMemory`. There's no need to execute this immediately on program startup..

================
Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:100
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory) {
+  NiceMock<MockDelegate> DummyDelegate;
----------------
labath wrote:
> It would be nice to have one more test where reads cross the page boundary, to make sure chunks are concatenated properly.
How about this? I still think it would be useful to have it, as the cross-page handling is the main source of complexity in this function.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503