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

Do not use Recursion for Expression compilation #132

Closed
jogibear9988 opened this issue Sep 2, 2018 · 7 comments
Closed

Do not use Recursion for Expression compilation #132

jogibear9988 opened this issue Sep 2, 2018 · 7 comments
Milestone

Comments

@jogibear9988
Copy link
Contributor

Maybe it's faster (or maybe not), to not Traverse the Expression Tree via recursive calls? I know, this would be a big rewrite.

Also a StackOverflow Exception could occur during expression Parsing.

see this, a StackOverflow happens with FastExpressionCompiler, with Net Framework it works

    [Test]
    public void Jit_compiler_internal_limitation2()
    {
        var blk = Block(Constant(7));
        for (int n = 0; n < 2000; n++)
        {
            blk = Block(blk);
        };

        var expr = Lambda<Func<int>>(blk);

        var c = expr.Compile();
        var compiled = expr.CompileFast();

        var obj = new TestClass1();

        Assert.AreEqual(7, compiled());
    }
@jogibear9988
Copy link
Contributor Author

Net Frameworks ExpressionCompile fails with about 4000 loops

@dadhi
Copy link
Owner

dadhi commented Sep 2, 2018

Yep, I think at some point of time we need to replace recursion with loop and hold our own stack.

@jogibear9988
Copy link
Contributor Author

I think Microsoft also uses, because it crashes also with a stack overflow. But later, so we have more methods in the Stack. Maybe we can reduce this

@dzmitry-lahoda
Copy link
Contributor

dzmitry-lahoda commented Sep 3, 2018

Or disallow more than 256 items of something #89

dadhi added a commit that referenced this issue Sep 7, 2018
…that some nodes container single sub-node, and the order of collecting cann be done on any order
@dadhi
Copy link
Owner

dadhi commented Sep 11, 2018

After inlining a lot of intermediate calls in TryEmit we may consider to un-roll recursion at least for some single child parents. Using the same way as in TryCollectBoundConstants

@dadhi
Copy link
Owner

dadhi commented Sep 12, 2018

To be clear it is not a recursion trampolining, we are just replacing recursion calls with loop until possible, then it still would go deeper for remaining nodes.

@dadhi dadhi added this to the 2.0.0 milestone Jan 25, 2019
@dadhi
Copy link
Owner

dadhi commented Jan 25, 2019

I don't think to spend more than already done on this.

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

No branches or pull requests

3 participants