Skip to content

RMS-8495: Komprise Memory leak #618

Merged
scribe merged 5 commits intoSpectraLogic:masterfrom
scribe:MemoryLeak
Sep 15, 2023
Merged

RMS-8495: Komprise Memory leak #618
scribe merged 5 commits intoSpectraLogic:masterfrom
scribe:MemoryLeak

Conversation

@scribe
Copy link
Contributor

@scribe scribe commented Sep 11, 2023

Some work with a profiler detected a handful of issues:

  • A whole bunch of inner cases were marked static so the outer class can be GC'd
  • Replace Arrays.asList(new String[] {} with Collections.emptyList() to avoid unnecessary allocations
  • Replace anonymous new Function<…> with static classes to allow the GC to run correctly
  • Replace single item Arrays.asList with List.of which should bound allocation
  • Remove tons of unused import statements which allowed removal of a sub-project dependency that wasn't actually used.
  • Replace Charset.forName("UTF-8") which requires a parser with StandardCharsets.UTF_8 which does not.

All of these together have left the profiler graphs in significantly better shape.

Edit: I ran tests on my laptop but I really want to see the tests run in Jenkins to give me better feelings.

@scribe
Copy link
Contributor Author

scribe commented Sep 11, 2023

Looks like I have a merge issue merging with master. Time to clean it up.

@david-limbach
Copy link

@scribe Seems like 99% of the changes are converting:
private class
to
private static class

Is this really necessary?

@scribe
Copy link
Contributor Author

scribe commented Sep 11, 2023

@scribe Seems like 99% of the changes are converting: private class to private static class

Is this really necessary?

If an inner class is not static it needs to be GC'd before the outer class can be marked for GC. What I'm worried about is something we return the lives a long time that keeps the outer class from going away.

@scribe scribe merged commit bf93957 into SpectraLogic:master Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants