jcasbin / jdbc-adapter

JDBC adapter for Casbin
https://github.com/casbin/jcasbin
Apache License 2.0
34 stars 37 forks source link

Load policies is slow #56

Closed damienmiheev closed 2 years ago

damienmiheev commented 2 years ago

org.casbin.jcasbin.persist.JDBCBaseAdapter has loadPolicy method to fetch data from db. Then it iterates over the rows and for each of the row it calls loadPolicyLine -> Helper.loadPolicyLine.

Inside helper there is:

    public static void loadPolicyLine(String line, Model model) {
        if (!"".equals(line)) {
            if (line.charAt(0) != '#') {
                String[] tokens = Util.splitCommaDelimited(line);
                String key = tokens[0];
                String sec = key.substring(0, 1);
                Assertion ast = (Assertion)((Map)model.model.get(sec)).get(key);
                ast.policy.add(Arrays.asList(Arrays.copyOfRange(tokens, 1, tokens.length)));

                for(int i = 0; i < ast.policy.size(); ++i) {
                    ast.policyIndex.put(((List)ast.policy.get(i)).toString(), i);
                }

            }
        }
    }

one more row loop. so for the db rows it will iterate like this:

1 1 1 ... n
  2 2     2
     3     3
            ...
            n

why do you need this internal for loop? it can be changed to:

    public static void loadPolicyLine(String line, Model model) {
        if (!"".equals(line)) {
            if (line.charAt(0) != '#') {
                String[] tokens = Util.splitCommaDelimited(line);
                String key = tokens[0];
                String sec = key.substring(0, 1);
                Assertion ast = (Assertion)((Map)model.model.get(sec)).get(key);

                List<String> nPolicy = Arrays.asList(Arrays.copyOfRange(tokens, 1, tokens.length))
                ast.policy.add(nPolicy);
                ast.policyIndex.put(policy.toString(), ast.policy.size() - 1);
            }
        }
    }

so no loop, faster load. what is the reason of it? Or maybe i missed something?

hsluoyz commented 2 years ago

@damienmiheev I think it's a bug, can you make a PR to fix it?

hsluoyz commented 2 years ago

Fixed by: https://github.com/casbin/jcasbin/pull/293