Reducing memory leaks in runtime-integration-tests #10793
Conversation
Making sure that resources are properly closed when test suits are done. Previously memory usage in Java tests was steadily rising until ~8GB. Now, we only see jumps in the last tests but the baseline sticks to ~3GB. We are not done yet.
hubertp
requested review from
4e6,
JaroslavTulach and
Akirathan
as code owners
|
With latest changes:
|
hubertp
added
the
CI: No changelog needed
| @@ -28,14 +28,15 @@ public static void prepareCtx() { | |||
| ctx = ContextUtils.createDefaultContext(out); | |||
| } | |||
|
|
|||
| @Before | |||
| @After | |||
| public void resetOut() { | |||
There was a problem hiding this comment.
Reset @Before is desirable to clean any debris from previous executions.
There was a problem hiding this comment.
It also meant that we weren't cleaning up tests after runs. Double-edged sword.
There was a problem hiding this comment.
You may want to do both. @Before and @After then.
| public static void disposeCtx() { | ||
| ctx.close(); | ||
| } | ||
| public class SignatureTest extends ContextTest { |
There was a problem hiding this comment.
Extending ContextTest is a nice trick. See also @Rule, btw.
There was a problem hiding this comment.
On many places in our tests, we use @Rule TemporaryFolder. See https://github.com/enso-org/enso/blob/develop/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/privateaccess/PrivateCheckDisabledTest.java#L15. Implementing ContextTest as Rule, we wouldn't probably need to subclass it.
There was a problem hiding this comment.
Sadly sometimes sometimes we use default context, sometimes more specialized one. Sometimes we cleanup on every test case, sometimes when the whole class is done. A lot of variation tbh.
|
Seems like some of our leaks come from TCK tests. This is after I disabled them: |
|
Updated description to reflect on the achieved/potential improvements. |
Can't read IR from an already closed context
hubertp
force-pushed
the
wip/hubert/tests-memory-leaks
branch
from
7f67066 to
9eb3f2f
Compare
Pull Request Description
Inspired by the revert done in #10778, started looking into apparent memory leaks in
runtime-integration-tests, written in Java.Initial state:
After:
Important Notes
Some remaining issues:
Leaving this for now, as it will probably need to be taken care by initial authors of those tests, if possible. Plus this PR leaves tests in a much better state than before.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.