Page MenuHome

Fix for race condition introduced in recent geom sync multithreading (https://developer.blender.org/D9258)
ClosedPublic

Authored by Olivier Maury (omaury) on Oct 23 2020, 12:19 AM.

Diff Detail

Repository
rB Blender

Event Timeline

Olivier Maury (omaury) requested review of this revision.Oct 23 2020, 12:19 AM
Olivier Maury (omaury) created this revision.

Looks good, does this pass the regression tests?

I don't know how to run them, was counting on you. Btw, this should prob be considered as a temp fix since the underlying more fundamental fix would be to formally declare the sync_hair tasks as dependent on the result (future) of the relevant geom subtasks (tessellation in this case).

I would encourage you to redo the performance tests you did for the original diff to see how this fix affects the final numbers.

I'll go ahead and commit this fix, even if this is not ideal for performance. The most important thing right now is to fix the crash quickly.

Once we have a dedicated hair object this will also resolve itself, so it's not that a big deal. Still may be worth re-running the benchmarks and seeing if there is a better solution (like task dependencies).

This revision is now accepted and ready to land.Oct 23 2020, 5:43 PM

I'll go ahead and commit this fix, even if this is not ideal for performance. The most important thing right now is to fix the crash quickly.

Once we have a dedicated hair object this will also resolve itself, so it's not that a big deal. Still may be worth re-running the benchmarks and seeing if there is a better solution (like task dependencies).

I'll re-run my benchmarks on our lab when I get a chance. Thanks @Brecht Van Lommel (brecht) !