| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| & resolveReexports reexportRefs | ||
| setVolatileStateSTM ref (IdeVolatileState (AstData asts) (map reResolved results) rebuildCache) | ||
| pure (force results) | ||
| pure results |
There was a problem hiding this comment.
I have limited haskell XP so may be misunderstanding but I suspect that by allowing results to be lazy, the IDE doesn't have to calculate all of the results as the state may be recomputed before much or any of an old result is used.
Whereas, when it is forced, it is possible for many computations to be queued up on a large project and the results value can use up so much memory that the IDE starts to grind to a halt.
Sorry, something went wrong.
|
I think the PR is sound, CI currently fails presumably because of the Azure debacle |
Sorry, something went wrong.
|
We'll need #4548 merged before this can pass |
Sorry, something went wrong.
…o lazy-state-result-purescript#4545
|
As #4548 is merged, is this ok to merge now? |
Sorry, something went wrong.
There was a problem hiding this comment.
This looks good to me - could someone else take a look as well?
Sorry, something went wrong.
|
This patch appears to be reverting 2606727 from #3006, which means at least three things:
|
Sorry, something went wrong.
|
@kritzcreek I know that commit was 7y ago, but do you have any recollection about this? |
Sorry, something went wrong.
|
Oh my, I had to think for a bit...
Actually meant off-thread, which is about a worker thread that does the populateVolatileState logic in here in the background. I think at the time when measuring I figured out that leaving the value lazy meant the computation ended up being done on the thread that accepts and responds to requests. Which meant the first requests after a state reload would take a very long time. (That's also why most of the resolution logic is wrapped in STM) I'm not sure if making things lazy here is going to help with peak memory usage, it might just delay it until you start sending some requests against the updated IdeState. Sorry, it's been a long time so I'm very fuzzy on the details... |
Sorry, something went wrong.
|
fwiw, this change brings down what normally is 30 GBs of peak memory usage down to 10 GBs alongside bringing down memory pressure/time-to-diagnostics quite a bit on our codebase. We also use this with a custom IDE command that filters which externs files get loaded. One side effect is that diagnostics are not immediately available until the first edit, but I'm not entirely sure if that's coming from this PR or the custom IDE command. This LGTM aside from removing the redundant Generic/NFData instances. |
Sorry, something went wrong.
|
Please don't take my comment as a blocker to merging. I haven't written Haskell in over 5(?) years, so I'm hardly an authority on the matter :D Just wanted to dump what context I could remember in case it was helpful. |
Sorry, something went wrong.
I assume by diagnostics you mean type info etc. It would certainly seem worth checking this with a clean build because that sounds like a blocker to me (and presumably not at all necessary or deep) |
Sorry, something went wrong.
|
This looks very easy to revert. If it was merged, it'd get released as a beta, right? |
Sorry, something went wrong.
Good point. I've removed them.
Also removed. I guess it would also be possible to only call force when logLevel == DEBUG |
Sorry, something went wrong.
|
This looks ready to merge to me; any final objections? |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Description of the change
To address #4545.
This change fixes the memory issues in our project and makes the IDE reasonable performant but I don't know if there are other ramifications.
Checklist: