perf: Remove shared_ptr in defaultRoleManager (#206)

* perf: Remove shared_ptr in defaultRoleManager * Share_ptr will cause performance loss. * Share_ptr may cause circular reference of Roles, which will cause memeory leak. * RoleManager take ownership of the resource, responsible for the application and release of Role*, Role only use this pointer. Signed-off-by: Stonexx <[email protected]> chore: Update include/casbin/rbac/default_role_manager.h Update casbin/rbac/default_role_manager.cpp Update casbin/rbac/default_role_manager.cpp Co-authored-by: Yash Pandey (YP) <[email protected]> * chore: update header align with src file Signed-off-by: Stonexx <[email protected]> Co-authored-by: Yash Pandey (YP) <[email protected]>
casbin · Jul 20, 2022 · 7051507 · 7051507
1 parent 3e1e242
commit 7051507
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 21 deletions.
diff --git a/casbin/rbac/default_role_manager.cpp b/casbin/rbac/default_role_manager.cpp
@@ -14,8 +14,6 @@
  * limitations under the License.
  */
 
-#include "casbin/pch.h"
-
 #ifndef DEFAULT_ROLE_MANAGER_CPP
 #define DEFAULT_ROLE_MANAGER_CPP
 
@@ -24,13 +22,13 @@
 
 namespace casbin {
 
-std::shared_ptr<Role> Role ::NewRole(std::string name) {
-    auto role = std::make_shared<Role>();
+std::unique_ptr<Role> Role :: NewRole(const std::string& name) {
+    auto role = std::make_unique<Role>();
     role->name = name;
     return role;
 }
 
-void Role ::AddRole(std::shared_ptr<Role> role) {
+void Role ::AddRole(Role* role) {
     for (int i = 0; i < this->roles.size(); i++) {
         if (this->roles[i]->name == role->name)
             return;
@@ -39,7 +37,7 @@ void Role ::AddRole(std::shared_ptr<Role> role) {
     this->roles.push_back(role);
 }
 
-void Role ::DeleteRole(std::shared_ptr<Role> role) {
+void Role ::DeleteRole(Role* role) {
     for (int i = 0; i < roles.size(); i++) {
         if (roles[i]->name == role->name)
             roles.erase(roles.begin() + i);
@@ -113,25 +111,26 @@ bool DefaultRoleManager ::HasRole(std::string name) {
     return ok;
 }
 
-std::shared_ptr<Role> DefaultRoleManager ::CreateRole(std::string name) {
-    std::shared_ptr<Role> role;
+Role* DefaultRoleManager ::CreateRole(const std::string& name) {
+    Role* role;
     bool ok = this->all_roles.find(name) != this->all_roles.end();
     if (!ok) {
         all_roles[name] = Role ::NewRole(name);
-        role = all_roles[name];
+        role = all_roles[name].get();
     } else
-        role = all_roles[name];
+        role = all_roles[name].get();
 
     if (this->has_pattern) {
         for (auto it = this->all_roles.begin(); it != this->all_roles.end(); it++) {
             if (this->matching_func(name, it->first) && name != it->first) {
-                std::shared_ptr<Role> role1;
+                Role* role1 = nullptr;
                 bool ok1 = this->all_roles.find(it->first) != this->all_roles.end();
                 if (!ok1) {
                     all_roles[it->first] = Role ::NewRole(it->first);
-                    role1 = all_roles[it->first];
+                    role1 = all_roles[it->first].get();
                 } else
-                    role1 = all_roles[it->first];
+                    role1 = all_roles[it->first].get();
+
                 role->AddRole(role1);
             }
         }
@@ -259,7 +258,7 @@ std::vector<std::string> DefaultRoleManager ::GetUsers(std::string name, std::ve
 
     std::vector<std::string> names;
     for (auto it = this->all_roles.begin(); it != this->all_roles.end(); it++) {
-        auto role = it->second;
+        auto role = it->second.get();
         if (role->HasDirectRole(name))
             names.push_back(role->name);
     }

diff --git a/include/casbin/rbac/default_role_manager.h b/include/casbin/rbac/default_role_manager.h
@@ -19,7 +19,8 @@
 
 #include <unordered_map>
 
-#include "./role_manager.h"
+#include "casbin/pch.h"
+#include "casbin/rbac/role_manager.h"
 
 namespace casbin {
 
@@ -30,16 +31,16 @@ typedef bool (*MatchingFunc)(const std::string&, const std::string&);
  */
 class Role {
 private:
-    std::vector<std::shared_ptr<Role>> roles;
+    std::vector<Role*> roles;
 
 public:
     std::string name;
 
-    static std::shared_ptr<Role> NewRole(std::string name);
+    static std::unique_ptr<Role> NewRole(const std::string& name);
 
-    void AddRole(std::shared_ptr<Role> role);
+    void AddRole(Role* role);
 
-    void DeleteRole(std::shared_ptr<Role> role);
+    void DeleteRole(Role* role);
 
     bool HasRole(std::string name, int hierarchy_level);
 
@@ -52,14 +53,14 @@ class Role {
 
 class DefaultRoleManager : public RoleManager {
 private:
-    std::unordered_map<std::string, std::shared_ptr<Role>> all_roles;
+    std::unordered_map<std::string, std::unique_ptr<Role>> all_roles;
     bool has_pattern;
     int max_hierarchy_level;
     MatchingFunc matching_func;
 
     bool HasRole(std::string name);
 
-    std::shared_ptr<Role> CreateRole(std::string name);
+    Role* CreateRole(const std::string& name);
 
 public:
     /**