JordanMarr / ReactiveElmish.Avalonia

Static Avalonia views for Elmish programs
Other
91 stars 8 forks source link

Experiment while doing bad things leads to small heap notice #27

Closed hhaynes-rower closed 8 months ago

hhaynes-rower commented 8 months ago

So I'm doing things kinda all wrong on purpose. πŸ˜„

I'm running multiple "agents" actually in the client that call to a locally installed DB. Not so bad you say? Well - consider that it's a timescaleDB instance where I'm injecting about 140 events per minute on a few threads "behind the scenes". So my poor little laptop is both the client with WAY too much BFF going on for my own taste. Add the heft of running a pretty busy little time series DB on an 8-core laptop and it's full on flirting with disaster. That said - I came back this morning to find a prompt in JetBrains Rider that complained about the small object heap running into issues. As soon as I see "DispatcherOperation" it immediately made me think of my questions surrounding how rapid-fire updates to the model might be handled in the app. I'm not sure if this is even related - and since I'm running such a hacky config I didn't want to raise too much ruckus over it. But I'm also in the "a finding is a finding" mode even as I start stripping out my agents and start putting them in a server-side IHostedService to run next to the db on a beefy home server. 🌯

Allocated object type: DispatcherOperation
Last observation: 11/10/2023 5:41 AM AidenDesktop.exe
  Allocated size: 402.2 MB

at Dispatcher.InvokeAsync(Action, DispatcherPriority, CancellationToken)
at MediaContext.ScheduleRender(bool)
at MediaContext.Avalonia.Rendering.Composition.ICompositorScheduler.CommitRequested(Compositor)
at DispatcherOperation.InvokeCore()
at DispatcherOperation.Execute()
at Dispatcher.ExecuteJob(DispatcherOperation)
at Dispatcher.ExecuteJobsCore(bool)
at Dispatcher.Signaled()
at Win32Platform.WndProc(int, uint, int, int)
at dynamicClass.IL_STUB_PInvoke()
at dynamicClass.IL_STUB_PInvoke()
at Win32DispatcherImpl.RunLoop(CancellationToken)
at DispatcherFrame.Run(IControlledDispatcherImpl)
at Dispatcher.PushFrame(DispatcherFrame)
at Dispatcher.MainLoop(CancellationToken)
at ClassicDesktopStyleApplicationLifetime.Start(String[])
at ClassicDesktopStyleApplicationLifetimeExtensions.StartWithClassicDesktopLifetime(AppBuilder, String[], ShutdownMode)
at Program.main(String[]) in C:\repos\Rower\Aiden\src\AidenDesktop\Program.fs:line 20 column 9

Screenshot 2023-11-10 075752

I'll keep an eye out as I peel out the hacked bits and put them in proper order. If you want to see the horror I've created you can check out this branch of my exploratory code.

https://github.com/Rower-Consulting/Aiden/blob/H3-desktop/src/AidenDesktop/ViewModels/DoughnutViewModel.fs

JordanMarr commented 8 months ago

Might be slightly more performant to use the native task CE. It might be easier since Npgsql uses tasks, so it would save on some of the Async.awaitTask calls. It might also be cleaner to replace Timer with Rx:

Using task instead of async for data call:

let fetchDataAsync () =
    task {
        // Connect to the database and execute the query
        use connection = new NpgsqlConnection(connectionString)
        do! connection.OpenAsync()
        let query = 
            @"SELECT UNNEST(string_to_array(vpn, ':')) AS vpn_part, COUNT(*) AS count
                FROM events_hourly
                WHERE event_time >= now() AT TIME ZONE 'UTC' - INTERVAL '1 minute'
                GROUP BY vpn_part;"

        use cmd = new NpgsqlCommand(query, connection)
        use! reader = cmd.ExecuteReaderAsync()
        let results = 
            [ while reader.Read() do
                yield (
                    reader.GetString(reader.GetOrdinal("vpn_part")),
                    reader.GetInt32(reader.GetOrdinal("count"))
                ) ]
        return results
    }

Replacing Timer with Rx:

open System.Reactive.Linq

let subscriptions (model: Model) : Sub<Msg> =

    let fetchDataForChart (dispatch: Msg -> unit) =
        Observable
            .Interval(TimeSpan.FromSeconds(2))
            .Select(fun _ -> Observable.FromAsync(fetchDataAsync))
            .Concat()
            .Subscribe(UpdateChartData >> dispatch)

    [
        [ nameof fetchDataForChart], fetchDataForChart
    ] 

That's so clean, I think I like it better!

hhaynes-rower commented 8 months ago

Thanks so much! I Plan to move this all into Akkling (as there will be many that will be spawned) and I love the idea of having a more performant baseline.

Speaking of which - I'm still curious about "where to fan out" in the update function. I'm going to be dispatching a LOT of updates - where 8+ visuals are all getting updates, many of which have similar data structure (but are drawn from different columns in the store, etc). Would it be better to create a single function that takes any series (and sub in the series name where "VPN" resides in my example) or is it better/worse to have a single dispatchable entry that is correlated to a single series? It just makes me wonder if MVU performs better in one pattern versus the other.

JordanMarr commented 8 months ago

Not sure I totally follow your question. Generally, I think it would be preferrable to consolidate the subscription data stream to (ideally) a single stream that updates everything in one go on each interval, rather than having multiple streams competing to do updates.

hhaynes-rower commented 8 months ago

I think you did a pretty good job despite my poorly worded question. 😊

That somewhat helps me develop the mental picture but it does beg the question that's in my head... if I can do a better job of pre-supposing it... πŸ€” So you have 12 visuals in a dashboard. If you have one "big" function that's performing the update for any series that's in the Model, that's only one level of locality. because if there are 12 "NOTIFY" events coming from the data layer you're still "seeing" twelve events streamed to it via SignalR that's still 12 calls to that one big update mechanism. So there would likely need to be a client-side semaphore to gather those events and send them into the update function in an orderly manner - as opposed to just trusting that the MVU loop will keep up with the rapid-fire messages arriving one after another. This - to me - means that an actor-like parallelism needs to exist in order to take all of those events and process those changes into ALL of the series that have updates in the model instance, and then post it back to the model "in one go".

So - I'm picturing an update operation that would take an optional input of N number of series (one for each in the model) that have been gathered for a particular update cycle. Inside that operation each series update would be "fanned out" to its own task/actor that updates the relevant series. Then a gathering mechanism would (such as a RunSynchronously operation) would then update all of the affected series into the model and return it at the end.

What I'm concerned about is how - in that time - any other model updates that have been sent in by SignalR should be queued/handled. That feels like a job for an actor system on the client side. But I only think that because I've done this sort of thing (albeit in a service-to-service mechanism on the backplane) with Akka and that could be a "when all you have is a hammer" kind of situation... And this not only has implications for general sense-making within the code-level approach. It also has bearing on how much delay there is in data showing up in the client for scenarios where "seconds count".

I believe I just answered my own question. πŸ” πŸ‘οΈ I think I would need to set up the actor system on the server side to deliver these model updates in a "chunks". This takes the burden off of the client and just lets the SignalR stream "do its thing" by queueing on the back end and delivering up to the client in an "orderly" fashion.

I'd be curious to know what you think about how much the MVU loop can be "leaned on" to provide updates to the model at short intervals: once per second? four times? ten?

JordanMarr commented 8 months ago

While you could do it in Akka, I think you would be making it way more difficult for yourself. My advice for your architecture is to further lean on the power of Rx to combine the incoming SignalR streams into one stream that then dispatches whatever it has collected on the specified time interval.

hhaynes-rower commented 8 months ago

OK - I suppose "difficult" is in the eye of he beholder. πŸ€• 😁 By that I mean the choice of "just handle it all on the server side" versus putting that loop/queuing logic on the client. I think that for the purposes of "it's just orchestrating updates to the UI" then I'll buy into it. But there are some scenarios that lead toward the need for "digital twinning" of the client state on-server. At that point keeping the client "dumb" or as thin as possible makes its own kind of sense. "In for a penny, in for a pound" and all that. But for early stages this is a great place to make an interim step. Thanks! I really appreciate your perspective.

JordanMarr commented 8 months ago

OK - I suppose "difficult" is in the eye of he beholder. πŸ€• 😁 By that I mean the choice of "just handle it all on the server side" versus putting that loop/queuing logic on the client. I think that for the purposes of "it's just orchestrating updates to the UI" then I'll buy into it. But there are some scenarios that lead toward the need for "digital twinning" of the client state on-server. At that point keeping the client "dumb" or as thin as possible makes its own kind of sense. "In for a penny, in for a pound" and all that. But for early stages this is a great place to make an interim step. Thanks! I really appreciate your perspective.

I see your point. I forgot that you are working with IoT devices with limited processing power and memory. In that case, it does make more sense to limit their potential load on the server-side. If you can guarantee that SignalR will only send 1 batch of updates per interval, then there will be no need to filter it on the client-side.

hhaynes-rower commented 8 months ago

Well - and to your point there's LOTS of potential on the client side with "thick" OS desktop clients - it's not like they're doing a lot anyway. 😬 Except in my case above where I was running a database AND and client while using a pair of consoles to throw 140 records per minute into a time series in one-second "buckets" 😊 πŸ”₯ πŸ™„

From the bigger picture, there will be "sweeps" over the database on the server-side that would be simmering in the background and would eventually lead to a "oh hey this is a wild anomaly we better notify the admins" and that message needs to be prioritized over others. That's pretty far down the road but that "hierarchical state" model could mean a mixture of both mechanisms depending on how much the client can be relied upon to carry the complexity of that "background state" and the need for heartbeat-level concurrency of that info "at the edge". This has been a very helpful convo - I think I have a few options for keeping things focused in early stages and not get too far into the weeds with trying to get MVU to do more work than necessary.

JordanMarr commented 8 months ago

Btw, did you happen to see the beta 2 post? I'm pretty excited about how it's turning out.

https://github.com/JordanMarr/Elmish.Avalonia/discussions/29

hhaynes-rower commented 8 months ago

I wow! I need to run an errand but I can't wait to get back to this and go through it this afternoon. πŸ‘