search

RMI-PACTA / workflow.data.preparation

The goal of `workflow.data.preparation` is to prepare all of the necessary data inputs for the Transition Monitor web application.
Other
2 stars 0 forks source link

Explore more appropriate memory management that doesn't involve calling `gc()` #141

Closed cjyetman closed 7 months ago

cjyetman commented 8 months ago 
          Approving as a "fine if we HAVE to" approval. And on the condition that we open a new issue to further explore a better way to get memory use within a reasonable limit.

There has to be a better way! But agree that this is an effective band-aid for now.

Originally posted by @jdhoffa in https://github.com/RMI-PACTA/workflow.data.preparation/pull/140#pullrequestreview-1884719033

cjyetman commented 8 months ago

for the record, @cjyetman does not believe that using gc() is necessarily a bad thing to do

jdhoffa commented 8 months ago

Maybe not!

My reaction to it may just be because I am not used to it or seeing that done in PROD elsewhere, not strictly a reaction to it fundamentally

AlexAxthelm commented 8 months ago

My reaction to it is that it's a pretty clear code smell. If you're running close enough to the edge that auto-collection isn't cutting it, then it's probably time to explore other options (this ticket!)

cjyetman commented 7 months ago

I think the idea here is that use of gc() implies a misuse of memory, e.g. making numerous, unnecessary large objects that need to be managed. The reality is that we have numerous large objects that need to be loaded, and we've already put a lot of effort into only loading them when needed, and getting rid of them once they're no longer needed, and now running gc() immediately after they've been removed to force R to clean up after them immediately, which imo is proper usage. Closing this ticket.