Make the kd-tree build really lazy and fix thread safety of builds#4675
Make the kd-tree build really lazy and fix thread safety of builds#4675sloriot merged 20 commits intoCGAL:masterfrom
Conversation
use c++11 memory model to fix the Double-Checked Locking
* use c++11 memory model to fix the Double-Checked Locking
It can be useful for a mesh with update vertex coordinates
AABB_tree/include/CGAL/AABB_tree.h
Outdated
| void clear_search_tree() | ||
| { | ||
| #ifdef CGAL_HAS_THREADS | ||
| if ( m_atomic_search_tree_constructed.load() ) |
There was a problem hiding this comment.
As you choose to use the full API of std::atomic, you can as well pass the right std::memory_order argument:
| if ( m_atomic_search_tree_constructed.load() ) | |
| if ( m_atomic_search_tree_constructed.load(std::memory_order_acquire) ) |
There was a problem hiding this comment.
I think it should be relaxed since the variable is guaranteed not to be modified once we are in this function (which I thought was the default but is seq actually...).
There was a problem hiding this comment.
That is right (relaxed). I had not seen in which function we were. Actually, in this function, the synchronization between threads and the performance do not really matter.
sloriot
force-pushed
the
AABB_tree-cpp11DCL
branch
from
1493553 to
0c98cd8
Compare
AABB_tree/include/CGAL/AABB_tree.h
Outdated
| delete m_p_search_tree; | ||
| m_p_search_tree = nullptr; | ||
| #ifdef CGAL_HAS_THREADS | ||
| m_atomic_search_tree_constructed.store(false); |
There was a problem hiding this comment.
| m_atomic_search_tree_constructed.store(false); | |
| m_atomic_search_tree_constructed.store(false, std::memory_order_release); |
|
ERROR: execution___of__point_inside_polyhedron_boundary_test in 1200.15 s : Timeout Also, (and pointed out by travis) : https://cgal.geometryfactory.com/CGAL/testsuite/results-5.1-Ic-132.shtml#Surface_mesh_simplification |
sloriot
force-pushed
the
AABB_tree-cpp11DCL
branch
from
7bb1f2c to
0bfc045
Compare
|
Successfully tested in https://cgal.geometryfactory.com/CGAL/testsuite/results-5.1-Ic-139.shtml. |
4 participants
Follow up of #4244 + document explicitely what is done.
The PR is on purpose for 5.1 only.