Delete principal from array after usage for GC

[heystewart]

On a large number of files, knip is crashing with out-of-memory. This is caused by the fact that all of the source file text and programs are being held for the entirety of the processing.

There is an explicit factory.deletePrincipal being done after every principal is processed, in the !isIsolateWorkspaces && isSkipLibs && !isWatch case, but the principals array is still holding onto the principal, so the memory is not released.

As we iterate through the principals, I clear the entry if it is deleted. I chose to set the principals entry to undefined, rather than manipulating the list, for code simplicity.

[webpro]

Thanks for the PR, @heystewart! The whole idea of isolated workspaces is to release this memory so i'm a bit ashamed this isn't happening. Oh well.

To minimize the changes, maybe we could add something like principals.length = 0 once to clear out the array?

[heystewart]

To minimize the changes, maybe we could add something like principals.length = 0 once to clear out the array?

No that would not be sufficient. in my use case it is failing partway through the iteration over the principals.
And doing .length = 0 at the end still leaves the factory.deletePrincipal(principal); code useless.

Most of the new code is due to the typesafety since the array needs to now be undefined...

[webpro]

Yes, during iteration we need to remove the current principal from the array (only in isolated-workspaces mode), but I don't really get why we have to remove the array itself?

[heystewart]

Yes, during iteration we need to remove the current principal from the array (only in isolated-workspaces mode), but I don't really get why we have to remove the array itself?

Ah, you mean specifically the principals = undefined; line instead of principals.length = 0;. This is more of a common practice I tend to use, especially in a function like this where a lot of work is being done after. If I am actively 'destroying' the contents of this array, I unset it so that further usage is prevented. I can use your preference, no problem.

[heystewart]

PTALA @webpro ; I'm not sure if this appeases your concerns.

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/knip@897

commit: d6a744d

[webpro]

Thanks a lot for bearing with me @heystewart! Planning to release this tomorrow.

[webpro]

🚀 This pull request is included in v5.42.0. See Release 5.42.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.