-
Notifications
You must be signed in to change notification settings - Fork 45
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
CPU Overload When Using Many Functions #4437
Comments
I experienced similar slowness. So I think it’s unrelated to imports. Can you confirm, @jgomez720? |
I can confirm. I removed all |
We tried reducing the KCL a bit. The part doesn't render in the scene when using an import. The same thing with everything inlined in a single file works, but it's not consistent. I think some kind of timeout is being hit. If I reduce the imported functions to only one, and gradually add them, it works. For a brief second, this error shows up in the editor and then disappears. This is what the error looks like when doing break on uncaught exceptions. Seems to be inside WASM, so the back trace isn't too helpful.
|
I haven't tracked down the actual problem yet, but running that sample with debug symbols show the panic is happening here: modeling-app/src/wasm-lib/src/wasm.rs Line 136 in 70b8541
out_4.txt only had the first 4 exports, out_8.txt had 8, etc.It seems like each function might be getting a copy of the AST at the time of parsing included in its memory |
ProgramMemory::default() drops the output file to 89KB. Not sure what the best way to capture the current state without completely copying the entire AST is
|
Yes, thank you for digging into this and getting more details. I knew it was bad, but I didn't know how bad. As a band-aid, I had the idea of clearing closed over program memory in closures before crossing back to TS from WASM, just before the line you linked in But I want to make sure the linked issue #4857 is fixed first. Even if there's ever a panic on the WASM side, the TS side should handle it gracefully. |
See: CPU still gets high load and is slow, but the file no longer fails. |
Not sure if it would be easier to add
|
That is a clever approach that would have a much smaller diff. It might also make it easier to avoid serializing closure memory recursively, which my branch doesn't yet do. I would consider adding the skip to my branch. I've thought about it, and I think we'd be better off with a separate type like ExecOutcome in my branch. This will force us to opt-in to make new fields available to the TS side, rather than exposing everything by default. I'm hoping it may also allow us to reduce the number of derives needed for Rust types. As it is, everything in ExecState needs to implement all the traits that the TS side needs, even though only a few things are used on the TS side. We'd also like to reduce what we expose to TS, so that we can change things on the Rust side easier. The hope is to not expose ProgramMemory in the future. But we're still working on the alternative. I closed my PR because it has a problem. See comments there. |
Yeah, I was mainly thinking it would be nice not to have to worry about clearing all the recursive memory manually. Just not sending that stuff back sounds like a better alternative if it isn't used on the TS side |
About the cloning of ProgramMemory that you linked...
The reason for the clone of ProgramMemory is because we implement it with mutable I wanted to mention another related thing that was talked about: The above issue would help here (by reducing serialization and reducing data crossing the WASM boundary) and make progress towards other goals. But without addressing the cloning, it's unclear if it would solve the CPU overload problem. |
Description
When I was trying to import many functions, my computer had a CPU overload. App clicking became unresponsive.
edit: does not have to do with import, it's just using many functions.
Version
v0.26.3
Recordings/Screenshots
Screenshare.-.2024-11-08.12_09_20.AM.mp4
KCL
whatever.kcl
nas1351.kcl
The text was updated successfully, but these errors were encountered: