Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

collectCycles() is being called many times in a single transaction (Performance issue) #77

Closed
clinuxrulz opened this issue Mar 8, 2019 · 2 comments

Comments

@clinuxrulz
Copy link
Collaborator

I think in Transaction.ts:

        if (this.collectCyclesAtEnd) {
            Vertex.collectCycles();
            this.collectCyclesAtEnd = false;
        }

needs to change to:

        if (this.collectCyclesAtEnd && Transaction.currentTransaction == null) {
            Vertex.collectCycles();
            this.collectCyclesAtEnd = false;
        }

I tried out a hook to see how often collectCycles was called in 1 transaction via:

sodium.Vertex.collectCycles = (function() {
    let realCollectCycles = sodium.Vertex.collectCycles;
    return function() {
        console.log("collectCycles start");
        realCollectCycles();
        console.log("collectCycles end");
    };
})();

And it was called way too many times.

I've got a temporary work-around hook though, so it is not causing me grief at the moment:

sodium.Vertex.collectCycles = (function() {
    let realCollectCycles = sodium.Vertex.collectCycles;
    let collectingCycles = false;
    return function() {
        if (collectingCycles) {
            return;
        }
        collectingCycles = true;
        window.setTimeout(() => {
            console.log("collectCycles start");
            realCollectCycles();
            console.log("collectCycles end");
            collectingCycles = false;
        });
    };
})();
@clinuxrulz
Copy link
Collaborator Author

this.collectCyclesAtEnd && Transaction.currentTransaction == null did not seem to do the trick for me. Will need to think about this one some more.

@clinuxrulz clinuxrulz changed the title collectCycles() is being called hundreds of times in a single transaction (Performance issue) collectCycles() is being called many times in a single transaction (Performance issue) Mar 8, 2019
@clinuxrulz
Copy link
Collaborator Author

Could this be from Transaction.post which is updating cell values before the next transactions? Like the updating of cells causing many intermediate transactions re-triggering collectCycles() over and over again. We might just need a better location to call collectCycles().

When I was profiling some slow code at work, collectCycles()'s total time was 80% compared to other total times. The "temporary work-around hook" I've listed above made the problem completely go away, but it can only be used if your using sodiumjs for the browser. If your using sodiumjs for node.js or some other target, then we probably wont have window.setTimeout() available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant