Remove unnecessary memory copy

When reading multiple handles create a handle indicating the shared
memory's area, and copy handles  only when it passes the filter

Change-Id: If0a6351d5cfe21545fbbbf7acce64a1ddafcac6a
Signed-off-by: Ilho Kim <ilho159.kim@samsung.com>

diff --git a/src/common/shared_memory/shm_app_reader.cc b/src/common/shared_memory/shm_app_reader.cc
index 72027c77da9d71abad04b404d87323bae0146abf..d7f61067e5285e1065e9477d7fb1545af17791ad 100644 (file)
--- a/src/common/shared_memory/shm_app_reader.cc
+++ b/src/common/shared_memory/shm_app_reader.cc
@@ -116,14 +116,20 @@ int ShmAppReader::GetHandles(pkgmgrinfo_filter_x* filter,
     if (!filter_checker.HasPrivilegeFilter()) {
       for (auto& reader : readers_) {
         ShmError result;
-        auto handles = reader->GetHandles(&result);
+        auto app_lock = reader->GetLock();
+        if (!app_lock) {
+          LOGW("Failed to get lock");
+          return PMINFO_R_ERROR;
+        }
+
+        auto handles = reader->GetHandlesWithoutLock(&result);
         if (!handles && result != ShmError::NONE)
           return PMINFO_R_ERROR;
 
         if (!handles)
           continue;
 
-        for (const auto& [_ , handle] : handles->GetMap())
+        for (const auto& handle : handles->GetHandles())
           if (filter_checker.Check(handle))
             list.emplace(handle.GetAppId(), handle.Clone());
       }
@@ -148,7 +154,7 @@ int ShmAppReader::GetHandles(pkgmgrinfo_filter_x* filter,
         }
 
         ShmError result;
-        auto app_handles = (*app_it)->GetHandles(&result);
+        auto app_handles = (*app_it)->GetHandlesWithoutLock(&result);
         if (!app_handles && result != ShmError::NONE)
           return PMINFO_R_ERROR;
 
@@ -158,25 +164,15 @@ int ShmAppReader::GetHandles(pkgmgrinfo_filter_x* filter,
           continue;
         }
 
-        auto pkg_handles = (*pkg_it)->GetHandles(&result);
-        if (!pkg_handles && result != ShmError::NONE)
-          return PMINFO_R_ERROR;
-
-        pkg_lock->Unlock();
-        app_lock->Unlock();
-
-
-        const auto& pkg_maps = pkg_handles->GetMap();
-        for (const auto& [_ , a_handle] : app_handles->GetMap()) {
-          auto pkg_it = pkg_maps.find(a_handle.GetPackage());
-          if (pkg_it == pkg_maps.end())
-            continue;
-
+        for (const auto& a_handle : app_handles->GetHandles()) {
           if (filter_checker.Check(a_handle) &&
-              filter_checker.CheckPrivilege(pkg_it->second))
+              filter_checker.CheckPrivilege(a_handle.GetPackage(), (*pkg_it)))
             list.emplace(a_handle.GetAppId(), a_handle.Clone());
         }
 
+        pkg_lock->Unlock();
+        app_lock->Unlock();
+
         app_it++;
         pkg_it++;
       }
@@ -250,8 +246,13 @@ bool ShmAppReader::FilterChecker::Check(const AppInfoHandle& handle) {
 }
 
 bool ShmAppReader::FilterChecker::CheckPrivilege(
-    const PkgInfoHandle& p_handle) {
-  if (!CheckPrivilegeFilters(p_handle, privilege_filters_))
+    const char* pkgid, ShmReader<PkgInfoHandle>* pkg_reader) {
+  ShmError result;
+  auto p_handle = pkg_reader->GetHandleWithoutLock(pkgid, &result);
+  if (!p_handle && result != ShmError::NONE)
+    return false;
+
+  if (!CheckPrivilegeFilters(*p_handle, privilege_filters_))
     return false;
 
   return true;

diff --git a/src/common/shared_memory/shm_app_reader.hh b/src/common/shared_memory/shm_app_reader.hh
index 38d811aef8a8406ab4b21ee6ba737e6f8f9efff6..b48cac5c1692d5425f3e51f2ce003b60e31c0810 100644 (file)
--- a/src/common/shared_memory/shm_app_reader.hh
+++ b/src/common/shared_memory/shm_app_reader.hh
@@ -32,7 +32,8 @@ class EXPORT_API ShmAppReader {
     FilterChecker(uid_t uid);
     bool Init(pkgmgrinfo_filter_x* filter);
     bool Check(const AppInfoHandle& handle);
-    bool CheckPrivilege(const PkgInfoHandle& p_handle);
+    bool CheckPrivilege(const char* pkgid,
+        ShmReader<PkgInfoHandle>* pkg_reader);
     const char* AppIdFilterValue();
     bool HasPrivilegeFilter();
 

diff --git a/src/common/shared_memory/shm_pkg_reader.cc b/src/common/shared_memory/shm_pkg_reader.cc
index cacfa557dda0d21bb807e95e0467181c4e8b468c..b2f980cd6a8dbed3edbc3d7df48bbd6c7ac34b36 100644 (file)
--- a/src/common/shared_memory/shm_pkg_reader.cc
+++ b/src/common/shared_memory/shm_pkg_reader.cc
@@ -132,7 +132,7 @@ int ShmPkgReader::GetHandles(pkgmgrinfo_filter_x* filter,
       if (!handles)
         continue;
 
-      for (const auto& [_ , handle] : handles->GetMap())
+      for (const auto& handle : handles->GetHandles())
         if (filter_checker.Check(handle))
           list.emplace(handle.GetPackage(), handle.Clone());
     }

diff --git a/src/common/shared_memory/shm_reader.cc b/src/common/shared_memory/shm_reader.cc
index bfbd4a2c49f10ca01a32e8a6d563e373984db29d..9e72f8ded8f2f5efcea269df857e58587236b68d 100644 (file)
--- a/src/common/shared_memory/shm_reader.cc
+++ b/src/common/shared_memory/shm_reader.cc
@@ -126,7 +126,8 @@ std::optional<ShmLockGuard> ShmReader<T>::Load(ShmError* result,
 }
 
 template<typename T>
-std::optional<typename ShmReader<T>::HandleCollection> ShmReader<T>::GetHandles(ShmError* result) {
+std::optional<typename ShmReader<T>::HandleCollection>
+    ShmReader<T>::GetHandles(ShmError* result) {
   std::shared_lock<std::shared_mutex> s(lock_);
   auto lock = Load(result, s);
   if (!lock)
@@ -135,6 +136,12 @@ std::optional<typename ShmReader<T>::HandleCollection> ShmReader<T>::GetHandles(
   return ReadHandles(result);
 }
 
+template<typename T>
+std::optional<typename ShmReader<T>::HandleCollection>
+    ShmReader<T>::GetHandlesWithoutLock(ShmError* result) {
+  return ReadHandles(result);
+}
+
 template<typename T>
 std::optional<T> ShmReader<T>::GetHandle(const char* key, ShmError* result) {
   std::shared_lock<std::shared_mutex> s(lock_);
@@ -159,6 +166,26 @@ std::optional<T> ShmReader<T>::GetHandle(const char* key, ShmError* result) {
   return handle;
 }
 
+template<typename T>
+std::optional<T> ShmReader<T>::GetHandleWithoutLock(
+    const char* key, ShmError* result) {
+  HandleMappingData* index_ptr = FindIndex(key, result);
+  if (!index_ptr) {
+    LOGD("Can't get index by key[%s]", key);
+    return std::nullopt;
+  }
+
+  auto handle = ReadHandle(index_ptr->index, index_ptr->len, false);
+  if (!handle) {
+    LOGE("Failed to read app");
+    *result = ShmError::SYSTEM;
+    return std::nullopt;
+  }
+
+  *result = ShmError::NONE;
+  return handle;
+}
+
 template<typename T>
 std::optional<ShmLockGuard> ShmReader<T>::GetLock() {
   std::shared_lock<std::shared_mutex> s(lock_);
@@ -181,31 +208,32 @@ std::optional<typename ShmReader<T>::HandleCollection> ShmReader<T>::ReadHandles
 
   if (index_mapper_.GetMemSize() == 0) {
     LOGD("Empty Shm Data");
-    ShmReader<T>::HandleCollection({}, {});
+    ShmReader<T>::HandleCollection({});
   }
 
-  std::map<std::string, T> handles;
-  std::vector<uint8_t> data(
-      handle_mapper_.GetPtr(),
-      handle_mapper_.GetPtr() + handle_mapper_.GetMemSize());
-  uint8_t* ptr = data.data();
+  std::vector<T> handles;
+  uint8_t* ptr = handle_mapper_.GetPtr();
   auto index_ptr = (HandleMappingData*)index_mapper_.GetPtr();
   auto end_ptr =
       (HandleMappingData*)(index_mapper_.GetPtr() + index_mapper_.GetMemSize());
   for (; index_ptr != end_ptr; index_ptr++) {
     T handle(std::make_unique<tizen_base::Parcel>(
         ptr + index_ptr->index, index_ptr->len, false, false));
-    handles.emplace(handle.GetKey(), std::move(handle));
+    handles.emplace_back(std::move(handle));
   }
 
-  return ShmReader<T>::HandleCollection(std::move(data), std::move(handles));
+  return ShmReader<T>::HandleCollection(std::move(handles));
 }
 
 template<typename T>
 std::optional<T> ShmReader<T>::ReadHandle(
-    uint32_t index, uint32_t len) {
-  return T(std::make_unique<tizen_base::Parcel>(
-      handle_mapper_.GetPtr() + index, len, true));
+    uint32_t index, uint32_t len, bool copy) {
+  if (copy)
+    return T(std::make_unique<tizen_base::Parcel>(
+        handle_mapper_.GetPtr() + index, len, true));
+  else
+    return T(std::make_unique<tizen_base::Parcel>(
+        handle_mapper_.GetPtr() + index, len, false, false));
 }
 
 template<typename T>

diff --git a/src/common/shared_memory/shm_reader.hh b/src/common/shared_memory/shm_reader.hh
index dc1bc5b2ad615514c83c889cd635ae1049b60ca6..716122337e2cc9f0719dd2a39fb363bac69ae00d 100644 (file)
--- a/src/common/shared_memory/shm_reader.hh
+++ b/src/common/shared_memory/shm_reader.hh
@@ -23,17 +23,14 @@ class EXPORT_API ShmReader {
  public:
   class HandleCollection {
    public:
-    HandleCollection(std::vector<uint8_t> data,
-        std::map<std::string, T> handles)
-            : data_(std::move(data)), handles_(std::move(handles)) {}
+    HandleCollection(std::vector<T> handles) : handles_(std::move(handles)) {}
 
-    const std::map<std::string, T>& GetMap() {
+    const std::vector<T>& GetHandles() {
       return handles_;
     }
 
    private:
-    std::vector<uint8_t> data_;
-    std::map<std::string, T> handles_;
+    std::vector<T> handles_;
   };
 
   ShmReader(std::string base_name) :
@@ -42,14 +39,16 @@ class EXPORT_API ShmReader {
       handle_mapper_(base_name_ + "_handle"),
       index_mapper_(base_name_ + "_indexs") {}
   std::optional<HandleCollection> GetHandles(ShmError* result);
+  std::optional<HandleCollection> GetHandlesWithoutLock(ShmError* result);
   std::optional<T> GetHandle(const char* key, ShmError* result);
+  std::optional<T> GetHandleWithoutLock(const char* key, ShmError* result);
   std::optional<ShmLockGuard> GetLock();
 
  private:
   std::optional<ShmLockGuard> Load(ShmError* result,
       std::shared_lock<std::shared_mutex>& s);
   std::optional<HandleCollection> ReadHandles(ShmError* result);
-  std::optional<T> ReadHandle(uint32_t index, uint32_t len);
+  std::optional<T> ReadHandle(uint32_t index, uint32_t len, bool copy = true);
   HandleMappingData* FindIndex(const char* key, ShmError* result);
 
  protected: