Page MenuHome

memory leak - modifiers on curves
Closed, ResolvedPublic

Description

System Information
Operating system: Windows-10-10.0.17134 64 Bits
Graphics card: GeForce GTX 1070/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 430.64

Blender Version
Broken: version: 2.80 (sub 74), branch: blender2.7, commit date: 2019-06-07 23:24, hash: rB749d53effd58
Worked: May 26 build works fine. May 31 is broken. I don't have builds in-between.

Short description of error
Adding any modifier to a curve/nurbs and tweaking anything in the modifier or in edit mode will increase memory usage.

Exact steps for others to reproduce the error
Open blend file, press "G" to move and look at the memory.


Or
Add a curve to a new scene, it doesn't matter if it's a bezier, path or NURBS
Add any modifier to this curve
Tweak anything

Event Timeline

Way created this task.Jun 8 2019, 12:50 PM
Way updated the task description. (Show Details)Jun 8 2019, 12:58 PM
Way updated the task description. (Show Details)Jun 8 2019, 1:02 PM
Way updated the task description. (Show Details)Jun 8 2019, 1:15 PM
Way renamed this task from memory leak - screw modifier on curves to memory leak - modifiers on curves.

Can confirm. On my older 14 May Build works ok.

Can reproduce on latest master. Wiggling one end of the default bezier curve, with a subsurf modifier (view subdivs=1) it will leak about 4mb a second. Add some bevel geometry, some extrusions and up the view subsurf level, and it can start haemorrhaging 100's mb a second.

Operating system: Linux-4.15.0-48-generic-x86_64-with-debian-stretch-sid 64 Bits

Bastien Montagne (mont29) claimed this task.
Bastien Montagne (mont29) triaged this task as Confirmed, High priority.

Will have a look at that one, unless @Sergey Sharybin (sergey) beats me to it.

System Information
Operating system: Windows-7 64 Bits
Graphics card: 'GeForce GTX 470/PCIe/SSE2NVIDIA Corporation 4.5.0 NVIDIA 391.35'

Blender Version
Broken: Blender-2.80.0-git.cc600de6695a-windows64 May 31- blender-2.80.0-git.749d53effd58-windows64 June 7
Worked:blender-2.80.0-git.d525c76003b3-windows64 May 26th

I can confirm that this bug also affects text objects. Attached is an example file of some text with a wave modifier. Just open it and hit play. It leaks 1 GB approximately every 270 frames.

Let me know if I should open a separate bug report

Way added a comment.EditedJun 9 2019, 8:36 AM

Can reproduce using text objects as well.
Easy steps to reproduce :

Create text object
Add a screw modifier
Tweak anything in the screw modifier or go into edit mode and start typing

If i turn off all modifiers in viewport memory won't leak. When i turn them back on it will happen again.

Text are basically using same code as curves here, so no, no need to open a new report, that is exact same issue. :)

Using chained array modifiers increase memory usage by a lot. Maybe this could give some hint on where relies the problem.

This is the output running in --debug mode:

CDMVert len: 260 00000240542221A8
CustomData->layers len: 520 0000024054AC0B78
CDMEdge len: 144 0000024054684B78
CustomData->layers len: 520 0000024054ABDD58
CustomData->layers len: 520 0000024054ABE278
CustomData->layers len: 520 0000024054ABECB8
BKE_mesh_recalc_tessellation len: 0 000002405446E768
BKE_mesh_recalc_tessellation len: 0 000002405446DDE8
CustomData->layers len: 520 0000024054ABE508
mesh_batch_cache_init len: 608 0000024053467C08
mesh_batch_cache_init len: 8 0000024053F24EF8
mesh_batch_cache_init len: 8 0000024053F25A38
GPUBatch len: 152 000002405267B678
GPUBatch len: 152 000002405267B9D8
GPUBatch len: 152 000002405267B318
GPUIndexBuf len: 32 000002405439C368
GPUVertBuf len: 416 0000024054AAB948
GPUIndexBuf len: 32 000002405439DEE8
GPUVertBuf len: 416 0000024054AAD708

This group is repited (more or less equal) as many times as changes in the modifier.

@Antonio Vazquez (antoniov) this is rather confusing since it mixes direct and indirect leaks. ASAN report is much more interesting as it distinguishes between both, and gives a proper complete backtrace. the root of the issue is that final mesh generated from modifiers evaluation (and stored into ob->runtime.mesh_eval) is never freed for cuve objects, for some reasons… Still need to check what happens in depsgraph on that matter.

Direct leak of 1584 byte(s) in 1 object(s) allocated from:
    #0 0x7f0ae779c518 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe9518)
    #1 0x55695c2b4694 in MEM_lockfree_callocN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:267
    #2 0x55695b4315f5 in BKE_libblock_alloc_notest /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library.c:1196
    #3 0x55695b4317ce in BKE_libblock_alloc /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library.c:1212
    #4 0x55695b4325bf in BKE_id_new_nomain /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library.c:1400
    #5 0x55695b511c04 in BKE_mesh_from_bmesh_for_eval_nomain /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/mesh.c:755
    #6 0x55695e4467f9 in triangulate_mesh /home/i74700deb64/blender/__work__/src/source/blender/modifiers/intern/MOD_triangulate.c:68
    #7 0x55695e447288 in applyModifier /home/i74700deb64/blender/__work__/src/source/blender/modifiers/intern/MOD_triangulate.c:110
    #8 0x55695b3068a0 in curve_calc_modifiers_post /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/displist.c:1098
    #9 0x55695b312fa2 in do_makeDispListCurveTypes /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/displist.c:1780
    #10 0x55695b313567 in BKE_displist_make_curveTypes /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/displist.c:1811
    #11 0x55695b6b6033 in BKE_object_handle_data_update /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/object_update.c:207
    #12 0x55695b6b87ba in BKE_object_eval_uber_data /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/object_update.c:350
    #13 0x55695c0c2214 in void std::__invoke_impl<void, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(std::__invoke_other, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/8/bits/invoke.h:60
    #14 0x55695c0bd5b4 in std::__invoke_result<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>::type std::__invoke<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/8/bits/invoke.h:95
    #15 0x55695c0b7798 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::__call<void, Depsgraph*&&, 0ul, 1ul, 2ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) /usr/include/c++/8/functional:400
    #16 0x55695c0af519 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/8/functional:484
    #17 0x55695c0a44e7 in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/8/bits/std_function.h:297
    #18 0x55695c1123eb in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/8/bits/std_function.h:687
    #19 0x55695c10f684 in deg_task_run_func /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:86
    #20 0x55695c01ec9f in handle_local_queue /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:420
    #21 0x55695c01ec9f in task_scheduler_thread_run /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:457
    #22 0x7f0ae5359fa2 in start_thread /build/glibc-vjB4T1/glibc-2.28/nptl/pthread_create.c:486

@Sergey Sharybin (sergey) this simple patch seems to fix it (just tagging the`mesh_eval` of curve as owned, when we generate one, which makes total sense to me), would rather have your green light on that one though:

diff --git a/source/blender/blenkernel/intern/displist.c b/source/blender/blenkernel/intern/displist.c
index b67335235a4..cf6f1fdeec1 100644
--- a/source/blender/blenkernel/intern/displist.c
+++ b/source/blender/blenkernel/intern/displist.c
@@ -1159,6 +1159,10 @@ static void curve_calc_modifiers_post(Depsgraph *depsgraph,
       (*r_final) = NULL;
     }
   }
+  else if (modified != NULL) {
+    /* Prety stupid to generate that whole mesh if it's unused, yet we have to free it. */
+    BKE_id_free(NULL, modified);
+  }
 }
 
 static void displist_surf_indices(DispList *dl)
@@ -1808,6 +1812,11 @@ void BKE_displist_make_curveTypes(
   do_makeDispListCurveTypes(
       depsgraph, scene, ob, dispbase, for_render, for_orco, &ob->runtime.mesh_eval);
 
+  if (ob->runtime.mesh_eval != NULL) {
+    ob->runtime.mesh_eval->id.tag |= LIB_TAG_COPIED_ON_WRITE_EVAL_RESULT;
+    ob->runtime.is_mesh_eval_owned = true;
+  }
+
   boundbox_displist_object(ob);
 }

Note that the first part of this patch is to address another potential memleak, which I found by reading the code (am not sure in practice it could actually happen with current code paths using curve_calc_modifiers_post(), though). But I have the feeling that func could use a bit more love, looks like at some point we had an optimization ensuring we would not generate a temp modified mesh in case r_final was not provided, but now we seem to always generate one, even if we only have deforming modifiers… At least with that minimal patch things should be secure, if not sane.

@Bastien Montagne (mont29), makes total sense to me ,good catch! Don't have code handy, but if you verified it properly marks things for curves/texts/surfaces please go ahead and commit.

@Sergey Sharybin (sergey) thanks, stealing back then. Yes, am pretty sure this fully covers case reported here, for all curve types.

There are several other issues in another user though (mesh_new_from_curve_type_object()/curve_to_mesh_eval_ensure() couple, used by 'convert to mesh' operator), but those are for other reports (like T65295), will check on those later.