Page MenuHomePhabricator

Undo/Redo implementation for segmentation is eating RAM
Closed, ResolvedPublic

Description

There is a lot of memory allocated while segmenting a sufficiently large image.

Even more critical is the fact that the memory is not properly freed after close project is performed.

Event Timeline

Tobi,

did you have a way of telling that it is the Undo/Redo that consumes the memory?
I can reproduce the bug, but if i clean up the undo/redo the memory still isn't freed.

Yes. I've just removed the line in SegTool2D where the operations where added to the stack.

In an undo/redo operation the segmentation image is stored only as a pointer but the current slice is copied. This definitely increases memory. There are some lines in the constructor of the operation (mitkDiffSliceOperation - line 52) commented out that used a zip container for the copied slice. A first approach might be to compress the undo/redo image slices again. As the slices are only containing values of 0 and 1 a zip compression should be very effective.

The observed effect might be an accumulation of several effects,
simply loading and image and closing the project will also lead to memory increase (significant the first time, little the following ones.)

In BS-After-Session, the problem of measureing freed memory was adressed,
maybe the mem is freed properly but not marked as such in a transparent way for the operating system.

Main bug, is that there is no strategy to delete any Undo/Redo objects, ever.

This bug is relevant for the upcoming release.
Did you try to activate the zipping again? Is there an easy way to reduce the memory consumption?

I did not try activating compressiong, not sure how to do it.

Other than that, i don't think this will make it in the current release,
as there also might be a Undo/Redo redesign, to adress issues with the new interaction.

Current release is finished. Reseting target milestone...

Setting m_3DInterpolationEnabled(false) in the constructor of mitkSegTool2D.cpp solves the problem. However, then the 3D interpolation doesn't work anymore.

The memory leak lies between line 242 and 250. This part of code should only be processed for 3D interpolation slices:

if (m_3DInterpolationEnabled && contour->GetVtkPolyData()->GetNumberOfPoints() > 0 && image->GetDimension() == 3)

{
  unsigned int pos = this->AddContourmarker(positionEvent);
  us::ServiceReference<PlanePositionManagerService> serviceRef =
      us::GetModuleContext()->GetServiceReference<PlanePositionManagerService>();
  PlanePositionManagerService* service = us::GetModuleContext()->GetService(serviceRef);
  mitk::SurfaceInterpolationController::GetInstance()->AddNewContour( contour, service->GetPlanePosition(pos));
  contour->DisconnectPipeline();
}

The problem lies within the mitkSurfaceInterpolationControllers AddNewContour method.

"...
for (unsigned int i = 0; i < m_CurrentNumberOfReducedContours; i++)
{

m_NormalsFilter->SetInput(i, m_ReduceFilter->GetOutput(i));
m_InterpolateSurfaceFilter->SetInput(i, m_NormalsFilter->GetOutput(i));

}
..."

Needs further investigation.

The code adressed by Michael Brehler and Josef Goerres seems to have changed completely. Bug might have been fixed during this refactoring?
How can we check that? According to Christian Weber (whose reference is Sascha Zelzer), neither the MITK memory consumption indicator nor the system memory consumption indicator are to be trusted.

Wanted to see if multiple 3d interpolations lead to increased ram consumption in MITK (checked by mitk indicator at bottom right).
Couldn't do this due to a current bug which prevents multiple 3d segmentations.

Part of this bug has been resolved within T18688.

It seems that this memory leak is only present if the paint brush tools are used

[34bdb4]: Merge branch 'bug-16232-undo-redo-memory-leak-in-segmentation'

Merged commits:

2015-04-08 15:33:31 Andreas Fetzer [8204fb]
Cosmetic changes


2015-04-08 15:33:20 Andreas Fetzer [a49115]
Fixed memory leak which occurs if GetAffectedSlice is called but not WriteBackSlice afterwards

Fixed part of the problem, but the issue still exists. The undo/redo information is not compressed any more. We must reactivate this feature, which is not trivial since we are now storing the vtkImageData instead of the mitkImage.
I will investigate this.

This bug could not be fixed during the 2015.05 release.
Setting target_milestone to AfterNextRelease

The branch bug-16232-undo-redo-memory-leak-in-segmentation is already in the release 2015.05 .

Are there any plans to work on the bug for the bugfix release?

Yes. Currently the images, that are stored in the undo/redo stack are not compressed and hence they need more memory than they should do ;)
I will work on that in the next few days.

User fetzer has pushed new remote branch:

bug-16232-undo-ram-issue-release-base

[177a99]: Merge branch 'bug-16232-undo-ram-issue-release-base'

Merged commits:

2015-07-20 09:12:14 Andreas Fetzer [2e24e0]
Fixed heavy memory consumption for segmentation undo/redo

Reason was, that the undo/redo information was not compressed anymore.
Activated compression again + cleanup of the related classes


2015-07-20 09:05:52 Andreas Fetzer [b1d097]
Fixed warning