Optimize SaveEventDelegate#preSaveCheck by removing if possible the recursion

[nithril]

Expected Behavior

The recursion algorithm implemented in preSaveCheck might not be necessary and might be transformed into a loop on a stack/deque.

Context

On a project I'm working on we have performance issues. We made the exercise to profile the application to fix the bottleneck.
We have identified the aforementionned methods as responsible of a fraction of the time taken (not negligible).

On graph with entities with many bidir relationships it can lead to deep stack trace.

Please let me know if it makes sense. If so I can submit a pull request.

Your Environment

• Java version: 1.8.0.265
• Neo4j version (The version of the database): 3.5.21
• OGM Version (The version of this software): 3.2.17
• OGM Transport used (One of Bolt, HTTP or embedded): Bolt
• Neo4j Java Driver version (in case of Bolt transport): 4.0.2

[nithril]

Something like that

        if (visit(object)) {

            Deque<Object> process = new ArrayDeque<>();
            process.add(object);

            while (!process.isEmpty()) {
                Object curObj = process.poll();
                children(curObj).stream().filter(o -> visit(o)).forEach(process::add);

                if (!preSaveFired(curObj) && dirty(curObj)) {
                    firePreSave(curObj);
                }
            }
        } else {
            logger.debug("already visited: {}", object);
        }

[nithril]

Apparently the dirty(object) requires a depth first traversal, not as trivial as I was expecting :)

[michael-simons]

Nope, definitiv not. But I like the idea and the delegate class needed some love anyway.

While technical, the both the following approaches are DFS:

    static class X {
        String name;

        public X(String name) {
            this.name = name;
        }

        List<X> children;

        @Override public String toString() {
            return "X{" +
                "name='" + name + '\'' +
                '}';
        }
    }

    static class PrintR {
        Set<X> visited = new HashSet<>();

        void print(X x) {
            if(!visited.contains(x)) {
                visited.add(x);
                System.out.println(x);
                x.children.forEach(this::print);
            }
        }
    }

    static class PrintI {
        Set<X> visited = new HashSet<>();

        void print(X x) {

            Deque<X> stack = new ArrayDeque<>();
            stack.push(x);
            while(!stack.isEmpty()) {
                X cur = stack.pop();
                if(!visited.contains(cur)) {
                    visited.add(cur);
                    System.out.println(cur);
                    cur.children.forEach(stack::push);
                }
            }
        }
    }

you can see how the order differs a tiny bit.

    public static void main(String...args) {

        X a = new X("A");
        X b = new X("B");
        X c = new X("C");
        X d = new X("D");
        X e = new X("E");
        X f = new X("F");
        X g = new X("G");

        a.children = Arrays.asList(b, c, e);
        b.children = Arrays.asList(d, f);
        c.children = Arrays.asList(g);
        d.children = new ArrayList<>();
        e.children = Arrays.asList(f);
        f.children = Arrays.asList(e,b);
        g.children = new ArrayList<>();

        new PrintR().print(a);
        System.out.println("---");
        new PrintI().print(a);

    }

I have a solution and the main point to it are two changes, which I write down in the PR.