BKE_pbvh_bmesh_update_topology is an expensive function that is executed on the main thread. the speed of the function varies based on the size of the tree. this is the main reason why blender will hang occasional during sculpting using dynamic topology; This only happens when topology gets up to around a million triangles. I've multi-threaded the search operation which does help speed up dynamic topology by quite a substantial amount. this by no means fixes the hanging issue with dynamic topology but does help the issue. the main way to resolve this issue is to handle sculpting actions on a separate thread away from the main UI thread. wm_handlers_do is where the main issue steams from and is located in wm_event_system.c.
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
I guess @Antonis Ryakiotakis (psy-fi) and @Sergey Sharybin (sergey) should be the ones to summon here?
| source/blender/blenkernel/intern/pbvh_bmesh.c | ||
|---|---|---|
| 588 | No C++ style comments in the C-code. | |
| 711 | I got some reservation about moving this out of the for-loop declaration. | |
| 718–734 | Why correcting the indentation? | |
| 720 | There's no reason to put the brace on a new line. | |
| 796–798 | Don't do unnecessary changes like this as you're not touching the body of the condition. | |
| 825 | Unnecessary as well. | |
Comment Actions
I'm not sure what's the actual root of slowness here but the patch does not seem to be correct at all for me. It wraps most expensive operation into a critical section which could give a bit of speedup on a systems with few threads because some intermediate loop will be parallelized (so you'll have loop arguments increased in threads), but systems with like >=16 threads will just waste all the time waiting for a critical section.
Also the "proper" solution from the original message is not correct as well. You can not update topology in a separate from main thread. Viewport does need topology to draw the object, so i'm not sure how you think you can separate threads here.
I'm not sure about really proper solution here, but having thread-safe lock-free structures for mempool, heaps, hashes could help.
| source/blender/blenkernel/intern/pbvh_bmesh.c | ||
|---|---|---|
| 676 | I don't see how it'll help. The body of the cycle traverses leaf edges and calls long_edge_queue_face_add() for them. That function only calls long_edge_queue_edge_add for each loop which only calls edge_queue_insert() once. And as you can see, the whole edge_queue_insert() was wrapped into omp critical section. Sure you'll have a bit of speedup because of some intermediate loops being traversed in threads, but most of the time you'll be just wasting your time in spinlocks around the critical section. This could actually make overall time of long_edge_queue_create() much higher. | |
| 718 | Same as above, throwing pragma omp here is not gonna to give measurable speedup. | |
Comment Actions
This is absolutely wrong. You can't multithread topology operations, mesh elements of shared vertices will run into concurrency issues when accessing elements of other nodes. I remember original code did that initially and it was reverted after some weird behaviour. In any case it's a no go from me.
Comment Actions
Hrmm..I should not be so negative sounding here..dyntopo freezing is indeed a source of trouble and patches are appreciated of course, but in this case this won't help I'm afraid...