insightsengineering / teal.modules.hermes

RNA-seq analysis modules to add to a teal application
https://insightsengineering.github.io/teal.modules.hermes/
Other
7 stars 1 forks source link

`R CMD check` consumes a large amount of memory #361

Closed cicdguy closed 8 months ago

cicdguy commented 8 months ago

What happened?

Running R CMD check on this package consumes a large amount of memory, almost to a point where total amount of available memory gets saturated.

See memory consumption graph when R CMD check --ignore-vignettes teal.modules.hermes_0.1.6.9001.tar.gz is run on a R 4.3.1 + BioC 3.18 container:

image

sessionInfo()

I can supply the renv.lock for reference. Cannot post it here due to the character limit for issue bodies imposed by GitHub (maximum is 65536 characters).

Relevant log output

The actual check runs fine when run in isolation.

The real issue happens when there are other R CMD checks or other processes that run alongside this one. The resulting issue is a memory leak and the following error in the log:


  In callback(...) :
    Chromote has received a Inspector.targetCrashed event. This means that the ChromoteSession has probably crashed.

Code of Conduct

Contribution Guidelines

Security Policy

danielinteractive commented 8 months ago

Thanks @cicdguy , how about other teal.modules.* packages? Do we see a similarly increasing, almost linear over time consumption of memory?

For now let's try to put https://rstudio.github.io/shinytest2/reference/AppDriver.html#method-AppDriver-stop into each shiny test.

cicdguy commented 8 months ago

Other teal.modules.* do not have this issue, for some reason. Sure, we can give it a shot and we'll let you know how it goes.

BFalquet commented 8 months ago

The 2 first test files and the 11th (which could correspond to the plateau in mid-curve) are the only test file where we are not performing snapshot tests. Maybe there is a glitch there.

danielinteractive commented 8 months ago

Hm interesting ... let's see after we add the stop commands what happens

danielinteractive commented 8 months ago

If this does not help, we could try

cicdguy commented 8 months ago

Agreed with the approach above. If possible we can try profiling the tests in isolation using a memory profiler (eg profmem) and see where the issue could be.

danielinteractive commented 8 months ago

@cicdguy is it better now?

cicdguy commented 8 months ago

@walkowif - do you mind testing it on the integration test suite?

walkowif commented 8 months ago

This is the current memory consumption of tests running only for teal.modules.hermes: process-memory

Here's the test log.

cicdguy commented 8 months ago

Nice! That looks much better. Looks like app$stop did the trick. Thanks @danielinteractive and @BFalquet!

danielinteractive commented 8 months ago

good to know and learn about it! thanks @cicdguy and @walkowif !