Ultimaker / Cura

3D printer / slicing GUI built on top of the Uranium framework
GNU Lesser General Public License v3.0
6.2k stars 2.08k forks source link

Big memory usage due to undo functionality #11010

Open jnaujok opened 2 years ago

jnaujok commented 2 years ago

Application Version

4.12.1

Platform

Linux Mint 20.2 Cinnamon

Printer

Creality CR-6 SE

Reproduction steps

  1. Load any 3D model.
  2. Slice
  3. Clear scene (Control-D)
  4. Load model
  5. Slice
  6. etc.
  7. Memory footprint never returns to original when clearing scene. It's like sliced model is left in memory.

Actual results

I sliced about 30 small models (Hexton Hills tiles) on a 16GB machine. Over this process, memory use went from initial usage of about 1GB of RAM to nearly 8GB of non-swappable RAM use. Machine nearly ran out of memory. Restarting Cura returns to the prior state.

Expected results

Clearing scene should remove all previously used memory. Memory use should not increase when model is cleared.

Checklist of files to include

Additional information & file uploads

No other items to add. This has happened ever since I went from 4.9 to 4.12 (Yes, I was behind on versions)

fvrmr commented 2 years ago

Hi @jnaujok thank you for your report. Are you using our AppImage? And could you share your log file? You can find your log file here: $HOME/.local/share/cura/<Cura version>/cura.log

jnaujok commented 2 years ago

Yes, I was using the AppImage of 4.12.1

Zip file of the logs is attached.

On Tue, Dec 7, 2021 at 4:54 AM Fenne @.***> wrote:

Hi @jnaujok https://github.com/jnaujok thank you for your report. Are you using our AppImage? And could you share your log file? You can find your log file here: $HOME/.local/share/cura//cura.log

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Ultimaker/Cura/issues/11010#issuecomment-987856051, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABU7Y5MM7BJJJR2X4JC37QDUPXYXZANCNFSM5JMH3R3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

fvrmr commented 2 years ago

I can't see your log file since you've replied via e-mail. You need to add them in a comment here on Github.

jnaujok commented 2 years ago

Yeah, I just saw that it didn't attach. Attaching here. curalogs.zip

thedustin commented 2 years ago

I noticed the same issue on macOS (but instead of "Clear scene" I used "Delete Selected"). I dived a bit into the code and found the RemoveSceneNodeOperation.py which is called when deleting models (see CuraActions::deleteSelection(), and see Uranium.UM.Qt.QtApplication::deleteAll()).

To be able to undo and redo this operation, the operation object stores the SceneNode (and parent) as property, which would explain the growing memory usage. But as I'm not that familiar with python, and even less with the code base, I'm not sure how this can be fixed/validated.

Ghostkeeper commented 2 years ago

Indeed, it's keeping the scene node (including the 3D model, which is like 99.99% of the data) in memory to be able to undo its deletion. As far as I know there is no stack limit to this either, at least not imposed by Cura.

The memory for the 3D mesh is freed when you undo the mesh loading and then do something else. Like a normal undo stack, that deletes the redo operations past its current position, including the mesh data.

However, one part should not happen:

It's like sliced model is left in memory.

The slice data is not kept in memory through the undo stack. It's not deleted via an operation. As a result, if you press undo you won't get your layer view back; you have to reslice.

Ghostkeeper commented 2 years ago

To solve the user's problem, we could add an undo limit.

We can't really change the way that data is stored on the undo stack. You could think of, e.g. storing the name of the file that was loaded so that it can be re-loaded when deleting a scene node is undone. But not all nodes are loaded from a file, e.g. meshes created by plug-ins like the support blocker, or downloaded from the internet with the Thingiverse or Digital Factory integration plug-ins. And it would also break the functionality of being able to undo a file reload.

fieldOfView commented 2 years ago

The memory for the 3D mesh is freed when you undo the mesh loading and then do something else.

I don't think so. Undoing the mesh loading sets the parent of the mesh to outside the visible scene. There is nothing to remove the mesh from that state except reparenting it to the visible scene via "redo", except maybe starting a new project.

Ghostkeeper commented 2 years ago

What would delete the memory is this line:

https://github.com/Ultimaker/Uranium/blob/66c8e3db276c6891154c101f9e556ee3fe062c2b/UM/Operations/OperationStack.py#L59

This deletes the reference to the operations on the stack. The operations keep a reference to the scene node. Expected behaviour is that this is the only reference to the scene nodes left, and so Python should garbage collect the mesh data. If this is not happening, something else must be referring to these scene nodes. We might need to debug that then.

It should not be referred to by the parent scene node because setParent unlinks it from the old parent before changing the parent. And the new parent is None.

fieldOfView commented 2 years ago

Ah yes, ofcourse, if the operation is the last reference to the mesh, and the operation is removed, then the mesh memory is indeed freed.

nallath commented 2 years ago

Setting the undo-redo limit to something like say 50 operations would probably already do the trick.