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

Consider tail recursion #62

Open
abelbraaksma opened this issue Nov 1, 2022 · 4 comments
Open

Consider tail recursion #62

abelbraaksma opened this issue Nov 1, 2022 · 4 comments
Labels
blocked: awaiting-fsharp-feature There's a change to F# compiler or library needed to implement this feature performance Performance questions, improvements and fixes
Milestone

Comments

@abelbraaksma
Copy link
Member

We've removed the tail recursion because it was hard to consolidate it into yield! (it was instead done with return!, which has no place in taskSeq).

However, today I helped someone with some code that he considered for taskSeq which had the following approach:

let getPoliciesAsync policyid =
    asyncSeq{
        use connection = new NpgsqlConnection(npgsqlConnectionStringBuilder.ConnectionString)
        use command = new NpgsqlCommand($"SELECT policy_data FROM policy.tbl_policy where policy_id = {Sql.uuid policyid};", connection)
        let! reader = command.ExecuteReaderAsync() |> Async.AwaitTask
        let rec someRec() = asyncSeq{
            let! rowExists = reader.ReadAsync() |> Async.AwaitTask
            if rowExists then
                yield {| dt1 = reader.GetString(0) |}
                yield! someRec()
        }
        yield! someRec()
    } |> AsyncSeq.toAsyncEnum

As you can see, it uses asyncSeq, but also: it is recursive. The same approach with taskSeq would likely be more performant, however, if there are a lot of rows, this becomes problematic. This code can be rewritten with a loop, though.

@dsyme, sharing this with you in case we want to revisit this at some point.

@dsyme
Copy link
Contributor

dsyme commented Nov 1, 2022

Yes, this should be rewritten with a loop

@abelbraaksma abelbraaksma added the performance Performance questions, improvements and fixes label Nov 3, 2022
@dsyme
Copy link
Contributor

dsyme commented Nov 5, 2022

@abelbraaksma BTW I think I am going to prioritise an F# 8 addition to allow builders to specify ReturnFromTailcall or YieldFromTailcall methods.

@abelbraaksma
Copy link
Member Author

@dsyme that would certainly simplify an addition like this, and would also allow task to support tail calls. Awesome! We can do some early-adoption testing here through TaskSeq.

@abelbraaksma
Copy link
Member Author

See fsharp/fslang-suggestions#1006 instead.

@abelbraaksma abelbraaksma added the blocked: awaiting-fsharp-feature There's a change to F# compiler or library needed to implement this feature label Oct 29, 2023
@abelbraaksma abelbraaksma added this to the vFuture milestone Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked: awaiting-fsharp-feature There's a change to F# compiler or library needed to implement this feature performance Performance questions, improvements and fixes
Projects
None yet
Development

No branches or pull requests

2 participants