[Lldb-commits] [PATCH] D60957: Read ObjC class names in one large read, instead of reading them individually

Sun Apr 21 23:19:52 PDT 2019

jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
Herald added subscribers: teemperor, abidh.
Herald added a project: LLDB.

There's a perf problem with Objective-C programs as we add more classes to the Darwin libraries over time - when lldb goes to run its first expression on one of these systems, it will load the ObjC class names from the run time, both the shared cache libraries and any app-contributed classes.  lldb has two expressions it runs in the inferior to collect this information into a buffer allocated by lldb - returning the isa pointers and a hash of each class name used to id them.  lldb would then read the names of the classes out of memory, which were not localized to one region of memory, and this has become a larger and larger performance issue.

This patch modifies:

1. The two jitted functions, g_get_dynamic_class_info_body and g_get_shared_cache_class_info_body, now take the address of an allocated string pool buffer and size.  If the address of the string pool is 0, no names are copied.  The functions will copy the class names in to the string pool buffer and add an offset to the (isa, hash) that were previously being returned.  If an entry in the (isa, hash, stroffset) array does not have an entry in the string pool, UINT32_MAX is used.  If lldb's pre-allocated string pool buffer is too small, entries that did not fit will get UINT32_MAX and lldb will read the class names the old slow way.

2. Modifies the two methods that call these jitted functions, AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic and AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache, to allocate the string pool buffer, pass the address and size to the jitted expression, scan the array of (isa, hash, stroffset) tuples to find the highest stroffset, read that part of the string pool out of the inferior with a single read [*], pass the buffer to ParseClassInfoArray, deallocate the stringpool from the inferior.

[X] nb: I just saw an unintended behavior on the last class name as I was writing this.  Given that it uses the highest stroffset it finds to read the string pool back out of the inferior, the final class name won't be copied up.  The test in ParseClassInfoArray will not

3. Modifies AppleObjCRuntimeV2::ParseClassInfoArray to grab the class name from the stringpool buffer if the stroffset is != UINT32_MAX and is within the copied buffer size.

[X] nb: I just saw an unintended behavior on the last class name as I was writing this.  Given that it uses the highest stroffset it finds to read the string pool back out of the inferior, the final class name won't be copied up.  The test in ParseClassInfoArray to check that the stroffset is within the bounds of the copied buffer should mean that we read the last class name out of the inferior aka the old method.  I'll double check this is handled correctly tomorrow.

This patch also includes a change that Frederic Riss wrote the other month but hadn't upstreamed yet, where we detect swift names in the shared cache and don't compute the hash in the jitted function, doing it up in lldb later.

Testing the patch shows no testsuite difference on Mac native.  As an experiment, I intentionally introduced a bug where the class names in the string pool were corrupted and it caused the testsuite failures I expected.  From a performance point of view, this shows the packet behavior I was aiming for.

rdar://problem/27798609

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D60957

Files:
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60957.196035.patch
Type: text/x-patch
Size: 24591 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190422/cfb41f89/attachment-0001.bin>