darklang / dark

Darklang main repo, including language, backend, and infra
https://darklang.com
Other
1.67k stars 91 forks source link

Create minimal repro for AOT Blazor memory issue #4071

Closed StachuDotNet closed 2 years ago

StachuDotNet commented 2 years ago

https://github.com/darklang/dark/pull/4070 introduced a "magic" line of code. Figure out how it helps, create a minimal repro for the .NET team, and send it to them.

Also, try using .Result instead of Task.map.

StachuDotNet commented 2 years ago

This issue was made assuming #4070 would solve our problem; it was insufficient. I'm having to create a minimal repro as part of #4059; closing this in favor of that.

StachuDotNet commented 2 years ago

https://github.com/StachuDotNet/ply-aot-serialization-bug-repro

Can still be thinned out a bit, but this is a pretty-minimal repro. Going to trim it a bit further, then figure out who to ping. Ply? STJ? FSharp.STJ? .NET?

StachuDotNet commented 2 years ago

still feels like the 'depth' of the AST really matters.

StachuDotNet commented 2 years ago

I've thinned this out quite a bit, in that repo. Most important parts are here and here.

While we're past it mattering in Dark[1], it's gonna bug me until I figure out what's wrong.

[1] - we chose the "use a non-Ply task" option.

Feeling like I've no idea what the problem is, still, somehow :)

pbiggar commented 2 years ago

Probably the blazor people would be interested in this. If it turns out not to be a fault of blazor, they are best positioned to tell the library what they're doing wrong (though the most likely thing is nothing, cause it's supposed to be a managed language unless they're doing native stuff).

StachuDotNet commented 2 years ago

I wrote a really long Issue. If you have a few minutes to review and edit it, I'd highly appreciate. Thinking to post to aspnet repo upon review.

StachuDotNet commented 2 years ago

I've recently encountered an odd bug.

A repository has been created to demonstrate a "minimal" reproduction, yet it's difficult to pinpoint the exact issue. Roughly, though: I encounter a "..." error when calling some Blazor-AOT-compiled published code, but don't encounter the bug when working with non-published or non-AOT artifacts.

The original finding of the error involved a very large payload, so I mistakenly assumed that we were hitting some memory limit due to the large payload. Since then, I've thinned the bug down to a thin separate repo, including a much smaller payload - so that's not the issue.

Within that repo is:

.NET SDK version is 6.0.300, running on Ubuntu, tested in Chrome and Firefox.

The relevant pieces are mostly within [webhost/index.html]() and [wasm/Program.fs]().

I'm not sure what party the issue is coming from. It could be any of: .NET/Blazor/mono, Ply, System.Text.Json, FSharp.SystemTextJson. My hunch is that it's either FSharp.SystemTextJson or Ply, but since taking any any of the pieces makes the error go away:

FWIW, this is not blocking me in any way - we decided to use a non-Ply task{} instead of the Ply one, and have gotten past the issue. I hope that this is the best repo to start here - I'm getting the error only in published AOT builds, so getting to the bottom of "why" there is a good place to start.

StachuDotNet commented 2 years ago

Actually, I'll just manage the issue here: https://github.com/StachuDotNet/ply-aot-serialization-bug-repro/issues/1

and other parties can link to it; easier to pass around this way.

pbiggar commented 2 years ago

Ill post my thoughts here to avoid dirtying up your pristine bug:

StachuDotNet commented 2 years ago

The separate repo is using the out-of-the-box blazor.webassembly.js, no webworker / custom blazorworker.

StachuDotNet commented 2 years ago

Feel free to post there - I'm just so damn curious what it is. That said, beyond posting it to aspnetcore tomorrow, I'll let it settle for a few days, as I'm tired of chasing it.