r-lib / rlang

Low-level API for programming with R
https://rlang.r-lib.org
Other
498 stars 136 forks source link

Large increase in memory due to commit 35e879084 #1626

Closed kyleam closed 1 year ago

kyleam commented 1 year ago

I've been investigating a large increase in memory consumption when running rcmdcheck() on the styler source tarball (more background below). I bisected the increase to rlang 35e879084 (Bump deprecations, 2023-02-24). That was on a custom Ubuntu (Bionic) EC2 instance with R 4.1.3, but I've set up a repo that demonstrates the issue with a rstudio/r-base:4.3.0-focal-based Docker image.

Here's a plot of the system memory used with different versions of rlang:

mem

The three cases:

Any ideas why 35e879084 is leading to such a large increase in memory and runtime?

More context

As part of building https://mpn.metworx.com/ snapshots, we run package checks on all of the packages. The current system is using R 4.1.3 and executes on workers running a custom EC2 image based on Ubuntu Bionic. For the latest snapshot, the styler check was failing with

> test_check("styler") # checks multiple files, in parallel
Starting 2 test processes
Killed

This check was running out of memory on 16 GB RAM workers. I bisected that memory exhaustion to 35e879084.

However, with the rstudio/r-base:4.1.3-bionic image, I see a memory increase similar to the plots above (which are for rstudio/r-base:4.3.0-focal). So, I don't know why the check is exhausting 16 GB of memory in our custom environment while just increasing the memory by a little under a GB here. My hope is that resolving this smaller increase will also address the memory exhaustion we're hitting into. In any case, an increase as large as the one above that's reproducible on top the rstudio/r-base:4.3.0-focal image seemed worth reporting.

kyleam commented 1 year ago

Any ideas why 35e8790 is leading to such a large increase in memory and runtime?

Given 35e8790 is focused on deprecations, I suppose a likely culprit is lots of warnings generated during the check.

Glancing at styler's unreleased recent commits, I spot 99d502fc (Replace usage of deprecated rlang::with_handlers(), 2023-04-01, r-lib/styler#1103). I'll check if that commit helps decrease the memory usage.

kyleam commented 1 year ago

Glancing at styler's unreleased recent commits, I spot 99d502fc (Replace usage of deprecated rlang::with_handlers(), 2023-04-01, r-lib/styler#1103). I'll check if that commit helps decrease the memory usage.

styler's current main (8e9ac8) looks at least as good as the low memory case above (v1.0.6-145-gd3dc2d84b). Reverting styler's 99d502fc on top of main restores a good portion of the increased memory use.

mem

So I suppose there might not be much to do on rlang's end (unless there's something obvious that could be done to avoid these warnings having such a large memory footprint).