Bank account kata with F# MailboxProcessor slow

Your problem is that your code don't uses Async all the way up.

Your Account class has the method Open, Close, Balance and Transaction and you use a AsyncReplyChannel but you use PostAndReply to send the message. This means: You send a message to the MailboxProcessor with a channel to reply. But, at this point, the method waits Synchronously to finish.

Even with Async.Parallel and multiple threads it can mean a lot of threads lock themsels. If you change all your Methods to use PostAndAsyncReply then your problem goes away.

There are two other performance optimization that can speed up performance, but are not critical in your example.

1. Calling the Length of a list is bad. To calculate the length of a list, you must go through the whole list. You only use this in Transaction to print the length, but consider if the transaction list becomes longer. You alway must go through the whole list, whenever you add a transaction. This will be O(N) of your transaction list.

2. The same goes for calling (List.sum). You have to calculate the current Balance whenever you call Balance. Also O(N).

As you have a MailboxProcessor, you also could calculate those two values instead of completly recalculating those values again and again.Thus, they become O(1) operations.

On top, i would change the Open, Close and Transaction messages to return nothing, as in my Opinion, it doesn't make sense that they return anything. Your examples even makes me confused of what the bool return values even mean.

In the Close message you return state.Opened before you set it to false. Why?

In the Open message you return the negated state.Opened. How you use it later it just looks wrong.

If there is more meaning behind the bool please make a distinct Discriminated Union out of it, that describes the purpose of what it returns.

You used an option<Acount> throughout your code, i removed it, as i don't see any purpose of it.

Anyway, here is a full example, of how i would write your code that don't have the speed problems.

type Message =
    | Open
    | Close
    | Balance     of AsyncReplyChannel<decimal option>
    | Transaction of decimal

type AccountState = {
    Opened:             bool
    Transactions:       decimal list
    TransactionsLength: int
    CurrentBalance:     decimal
}

type Account() =
    let agent = MailboxProcessor<Message>.Start(fun inbox ->
        let rec loop (state: AccountState) = async {
            match! inbox.Receive() with
            | Close ->
                printfn "Closing"
                return! loop { state with Opened = false }
            | Open ->
                printfn "Opening"
                return! loop { state with Opened = true }
            | Transaction tran ->
                let l = state.TransactionsLength + 1
                printfn $"Adding transaction {tran}, nb = {l}"

                if state.Opened then
                    return! loop {
                        state with
                            Transactions       = tran :: state.Transactions
                            TransactionsLength = l
                            CurrentBalance     = state.CurrentBalance + tran
                    }
                else
                    return! loop state
            | Balance channel ->
                if   state.Opened
                then channel.Reply (Some state.CurrentBalance)
                else channel.Reply  None
                return! loop state
        }

        let defaultAccount = {
            Opened             = false
            Transactions       = []
            TransactionsLength = 0
            CurrentBalance     = 0m
        }
        loop defaultAccount
    )

    member _.Open        ()          = agent.Post(Open)
    member _.Close       ()          = agent.Post(Close)
    member _.Balance     ()          = agent.PostAndAsyncReply(Balance)
    member _.Transaction transaction = agent.Post(Transaction transaction)

(* Test *)

let should_equal expected actual =
    if expected = actual then
        Ok expected
    else
        Error (expected, actual)

(* --- API --- *)

let mkBankAccount = Account

(* Opens the Account *)
let openAccount  (account: Account) =
    account.Open ()

(* Closes the Account *)
let closeAccount (account: Account) =
    account.Close ()

(* Updates Account *)
let updateBalance transaction (account: Account) =
    account.Transaction(transaction)

(* Gets the current Balance *)
let getBalance (account: Account) =
    account.Balance ()

#time
let ``Account can be updated from multiple threads`` =
    let account = mkBankAccount ()
    openAccount account

    let updateBalanceAsync = async {
        updateBalance 1.0m account
    }

    let nb = 50

    List.replicate nb updateBalanceAsync
    |> Async.Parallel
    |> Async.RunSynchronously
    |> ignore

    Async.RunSynchronously (async {
        let! balance = getBalance account
        printfn "Balance is %A should be (Some %f)" balance (1.0m * decimal nb)
    })
#time