Fix ModuleCache usage in Platform - ask remote platform for module's ModuleSpec beforehand so we can look for a module by UUID locally without need to download it.

ovyalov · ovyalov · commit 037f6b9c0f52 · 2015-03-24T23:45:49.000Z
http://reviews.llvm.org/D8557 llvm-svn: 233136
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
@@ -12,6 +12,7 @@
 
 // C Includes
 // C++ Includes
+#include <functional>
 #include <map>
 #include <memory>
 #include <string>
@@ -1130,31 +1131,36 @@ class ModuleCache;
                              const FileSpecList *module_search_paths_ptr,
                              Platform &remote_platform);
 
-        bool
-        GetCachedSharedModule (const ModuleSpec &module_spec,
-                               Process* process,
-                               lldb::ModuleSP &module_sp);
-
         Error
         DownloadModuleSlice (const FileSpec& src_file_spec,
                              const uint64_t src_offset,
                              const uint64_t src_size,
                              const FileSpec& dst_file_spec);
 
-        bool
-        GetModuleFromLocalCache (const ModuleSpec& module_spec,
-                                 Process* process,
-                                 lldb::ModuleSP &module_sp);
+    private:
+        typedef std::function<Error (const ModuleSpec &)> ModuleResolver;
+
+        Error
+        GetRemoteSharedModule (const ModuleSpec &module_spec,
+                               Process* process,
+                               lldb::ModuleSP &module_sp,
+                               const ModuleResolver &module_resolver,
+                               bool *did_create_ptr);
 
-        FileSpec GetModuleCacheRoot ();
+        bool
+        GetCachedSharedModule (const ModuleSpec& module_spec,
+                               lldb::ModuleSP &module_sp,
+                               bool *did_create_ptr);
 
-    private:
         Error
         LoadCachedExecutable (const ModuleSpec &module_spec,
                               lldb::ModuleSP &module_sp,
                               const FileSpecList *module_search_paths_ptr,
                               Platform &remote_platform);
 
+        FileSpec
+        GetModuleCacheRoot ();
+
         DISALLOW_COPY_AND_ASSIGN (Platform);
     };
 
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
@@ -253,23 +253,23 @@ Platform::GetSharedModule (const ModuleSpec &module_spec,
                            ModuleSP *old_module_sp_ptr,
                            bool *did_create_ptr)
 {
-    if (!IsHost () && GetGlobalPlatformProperties ()->GetUseModuleCache ())
-    {
-        // Use caching only when talking to a remote platform.
-        if (GetCachedSharedModule (module_spec, process, module_sp))
-        {
-            if (did_create_ptr)
-                *did_create_ptr = true;
-
-            return Error ();
-        }
-    }
-    return ModuleList::GetSharedModule (module_spec, 
-                                        module_sp,
-                                        module_search_paths_ptr,
-                                        old_module_sp_ptr,
-                                        did_create_ptr,
-                                        false);
+    if (IsHost ())
+        return ModuleList::GetSharedModule (module_spec,
+                                            module_sp,
+                                            module_search_paths_ptr,
+                                            old_module_sp_ptr,
+                                            did_create_ptr,
+                                            false);
+
+    return GetRemoteSharedModule (module_spec,
+                                  process,
+                                  module_sp,
+                                  [&](const ModuleSpec &spec)
+                                  {
+                                      return ModuleList::GetSharedModule (
+                                          spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, did_create_ptr, false);
+                                  },
+                                  did_create_ptr);
 }
 
 bool
@@ -1779,66 +1779,80 @@ Platform::LoadCachedExecutable (const ModuleSpec &module_spec,
                                 const FileSpecList *module_search_paths_ptr,
                                 Platform &remote_platform)
 {
-    if (GetGlobalPlatformProperties ()->GetUseModuleCache ())
-    {
-        if (GetCachedSharedModule (module_spec, nullptr, module_sp))
-            return Error ();
-    }
-
-    return remote_platform.ResolveExecutable (module_spec,
-                                              module_sp,
-                                              module_search_paths_ptr);
+    return GetRemoteSharedModule (module_spec,
+                                  nullptr,
+                                  module_sp,
+                                  [&](const ModuleSpec &spec)
+                                  {
+                                      return remote_platform.ResolveExecutable (
+                                          spec, module_sp, module_search_paths_ptr);
+                                  },
+                                  nullptr);
 }
 
-bool
-Platform::GetCachedSharedModule (const ModuleSpec &module_spec,
+Error
+Platform::GetRemoteSharedModule (const ModuleSpec &module_spec,
                                  Process* process,
-                                 lldb::ModuleSP &module_sp)
-{
-    return (m_module_cache && GetModuleFromLocalCache (module_spec, process, module_sp));
-}
-
-bool
-Platform::GetModuleFromLocalCache (const ModuleSpec& module_spec,
-                                   Process* process,
-                                   lldb::ModuleSP &module_sp)
+                                 lldb::ModuleSP &module_sp,
+                                 const ModuleResolver &module_resolver,
+                                 bool *did_create_ptr)
 {
-    Log *log = GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PLATFORM);
-
-    
+    // Get module information from a target.
     ModuleSpec resolved_module_spec;
     bool got_module_spec = false;
-
     if (process)
     {
         // Try to get module information from the process
         if (process->GetModuleSpec (module_spec.GetFileSpec (), module_spec.GetArchitecture (), resolved_module_spec))
-            got_module_spec = true;
+          got_module_spec = true;
     }
 
     if (!got_module_spec)
     {
         // Get module information from a target.
         if (!GetModuleSpec (module_spec.GetFileSpec (), module_spec.GetArchitecture (), resolved_module_spec))
-            return false;
+            return module_resolver (module_spec);
     }
 
+    // Trying to find a module by UUID on local file system.
+    const auto error = module_resolver (resolved_module_spec);
+    if (error.Fail ())
+     {
+          if (GetCachedSharedModule (resolved_module_spec, module_sp, did_create_ptr))
+              return Error ();
+     }
+
+    return error;
+}
+
+bool
+Platform::GetCachedSharedModule (const ModuleSpec &module_spec,
+                                 lldb::ModuleSP &module_sp,
+                                 bool *did_create_ptr)
+{
+    if (IsHost() ||
+        !GetGlobalPlatformProperties ()->GetUseModuleCache ())
+        return false;
+
+    Log *log = GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PLATFORM);
+
     // Check local cache for a module.
     auto error = m_module_cache->Get (GetModuleCacheRoot (),
                                       GetHostname (),
-                                      resolved_module_spec,
-                                      module_sp);
+                                      module_spec,
+                                      module_sp,
+                                      did_create_ptr);
     if (error.Success ())
         return true;
 
     if (log)
         log->Printf("Platform::%s - module %s not found in local cache: %s",
-                    __FUNCTION__, resolved_module_spec.GetUUID ().GetAsString ().c_str (), error.AsCString ());
+                    __FUNCTION__, module_spec.GetUUID ().GetAsString ().c_str (), error.AsCString ());
 
     // Get temporary file name for a downloaded module.
     llvm::SmallString<PATH_MAX> tmp_download_file_path;
     const auto err_code = llvm::sys::fs::createTemporaryFile (
-        "lldb", resolved_module_spec.GetUUID ().GetAsString ().c_str (), tmp_download_file_path);
+        "lldb", module_spec.GetUUID ().GetAsString ().c_str (), tmp_download_file_path);
     if (err_code)
     {
         if (log)
@@ -1851,9 +1865,9 @@ Platform::GetModuleFromLocalCache (const ModuleSpec& module_spec,
 
     const FileSpec tmp_download_file_spec (tmp_download_file_path.c_str (), true);
     // Download a module file.
-    error = DownloadModuleSlice (resolved_module_spec.GetFileSpec (),
-                                 resolved_module_spec.GetObjectOffset (),
-                                 resolved_module_spec.GetObjectSize (),
+    error = DownloadModuleSlice (module_spec.GetFileSpec (),
+                                 module_spec.GetObjectOffset (),
+                                 module_spec.GetObjectSize (),
                                  tmp_download_file_spec);
     if (error.Fail ())
     {
@@ -1867,21 +1881,22 @@ Platform::GetModuleFromLocalCache (const ModuleSpec& module_spec,
     // Put downloaded file into local module cache.
     error = m_module_cache->Put (GetModuleCacheRoot (),
                                  GetHostname (),
-                                 resolved_module_spec,
+                                 module_spec,
                                  tmp_download_file_spec);
     if (error.Fail ())
     {
         if (log)
             log->Printf("Platform::%s - failed to put module %s into cache: %s",
-                        __FUNCTION__, resolved_module_spec.GetUUID ().GetAsString ().c_str (),
+                        __FUNCTION__, module_spec.GetUUID ().GetAsString ().c_str (),
                         error.AsCString ());
         return false;
     }
 
     error = m_module_cache->Get (GetModuleCacheRoot (),
                                  GetHostname (),
-                                 resolved_module_spec,
-                                 module_sp);
+                                 module_spec,
+                                 module_sp,
+                                 did_create_ptr);
     return error.Success ();
 }
 
diff --git a/lldb/source/Utility/ModuleCache.cpp b/lldb/source/Utility/ModuleCache.cpp
@@ -82,7 +82,8 @@ Error
 ModuleCache::Get (const FileSpec &root_dir_spec,
                   const char *hostname,
                   const ModuleSpec &module_spec,
-                  ModuleSP &cached_module_sp)
+                  ModuleSP &cached_module_sp,
+                  bool *did_create_ptr)
 {
     const auto find_it = m_loaded_modules.find (module_spec.GetUUID ().GetAsString());
     if (find_it != m_loaded_modules.end ())
@@ -109,6 +110,8 @@ ModuleCache::Get (const FileSpec &root_dir_spec,
     cached_module_spec.GetFileSpec () = module_file_path;
     cached_module_spec.GetPlatformFileSpec () = module_spec.GetFileSpec ();
     cached_module_sp.reset (new Module (cached_module_spec));
+    if (did_create_ptr)
+        *did_create_ptr = true;
 
     m_loaded_modules.insert (std::make_pair (module_spec.GetUUID ().GetAsString (), cached_module_sp));
 
diff --git a/lldb/source/Utility/ModuleCache.h b/lldb/source/Utility/ModuleCache.h
@@ -54,7 +54,8 @@ class ModuleCache
     Get (const FileSpec &root_dir_spec,
          const char *hostname,
          const ModuleSpec &module_spec,
-         lldb::ModuleSP &cached_module_sp);
+         lldb::ModuleSP &cached_module_sp,
+         bool *did_create_ptr);
 
 private:
     static FileSpec
