[Lldb-commits] [PATCH] Add Utility/ModuleCache class and integrate it with PlatformGDBRemoteServer - in order to allow modules caching from remote targets.

[Oleksiy Vyalov]

Please see my comments.


================
Comment at: include/lldb/Core/ModuleSpec.h:17
@@ -16,2 +16,3 @@
 #include "lldb/Host/FileSpec.h"
+#include "lldb/Host/Mutex.h"
 #include "lldb/Target/PathMappingList.h"
----------------
clayborg wrote:
> Does this need to be in the header file?
ModuleSpec::m_mutex needs it - https://github.com/llvm-mirror/lldb/blob/master/include/lldb/Core/ModuleSpec.h#L589, otherwise compilation is failing for me.

It seems to me that ModuleSpec has grown pretty big as for inlined class - we may add ModuleSpec.cpp and move methods' implementation over there. In this case we can use std::unique<Mutex> instead of Mutex, so it will be enough to have Mutex forward declaration inside of the header file.

================
Comment at: source/Core/Debugger.cpp:700-703
@@ -699,2 +699,6 @@
                                      Target::GetGlobalProperties()->GetValueProperties());
+    m_collection_sp->AppendProperty (ConstString("platform"),
+                                     ConstString("Platform settings."),
+                                     true,
+                                     Platform::GetGlobalPlatformProperties()->GetValueProperties());
     if (m_command_interpreter_ap.get())
----------------
clayborg wrote:
> Does this play well with the other "platform" settings that are already there?
> 
> platform.plugin.linux.use-llgs-for-local (boolean) = false
> platform.plugin.darwin-kernel.search-locally-for-kexts (boolean) = true
> platform.plugin.darwin-kernel.kext-directories (file-list) =
> 
I didn't notice any runtime issues with existing properties - here is the partial output of "settings show" from my Ubuntu:

```

...
target.process.thread.trace-thread (boolean) = false
platform.use-module-cache (boolean) = true
platform.module-cache-directory (file) = "/usr/local/google/home/ovyalov/lldb/module_cache/lldb"
platform.plugin.linux.use-llgs-for-local (boolean) = true
interpreter.expand-regex-aliases (boolean) = false

```
....

on OSX:


```
...
target.process.thread.trace-thread (boolean) = false

platform.use-module-cache (boolean) = true

platform.module-cache-directory (file) = "/Users/ovyalov/lldb/module_cache/lldb"

platform.plugin.linux.use-llgs-for-local (boolean) = true

platform.plugin.darwin-kernel.search-locally-for-kexts (boolean) = true

platform.plugin.darwin-kernel.kext-directories (file-list) =

interpreter.expand-regex-aliases (boolean) = false

interpreter.prompt-on-quit (boolean) = true



```


Since Platform a base class we're not allowed to use DebuggerInitialize method to initialize settings as PlatformLinux, for instance, does.

Will upload fixed version of the CL shortly to use user's home directory as a root folder for module cache instead of LLDB temp directory (which is cleaned when a process exits).

http://reviews.llvm.org/D8037

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/