CGAL users discussion list

["Laurent Rineau (CGAL/GeometryFactory)" <>]

Dear Bernhard,

After almost a year, I am in the process of integrating your contribution 
into 
CGAL.

I am a few question, below, for the authors of the patch. How could I contact 
them directly?

Le Wednesday 21 January 2015 12:06:03 Bernhard Kornberger a écrit :
> Hello!
> 
> 
> Last year researches from the Distributed and Parallel Systems Group
> of the University of Innsbruck have parallelized one of my programs.
> To this end they needed to patch CGAL to achieve thread safety for
> the Exact_predicates_exact_constructions_kernel. Now I'd like to
> contribute this patch and possibly start a discussion about it:
> 
> http://www.geom.at/_downloads/patch.diff
> 
> The patch is not my work, so I paste their description here:



Hi,

I am integrating a modified version of your patch in CGAL. That is the pull-
request 648 of CGAL/cgal on GitHub. You can see the patch here:

https://github.com/CGAL/cgal/pull/648/files

You can also pull my branch `Filtered_kernel-Epeck_reentrance-GF` from 
https://github.com/lrineau/cgal


> ### Copy&Paste start ###
> 
> The handler classes implement reference counting mechanisms. A simple
> atomic increment
> operation was added to the counter increment to avoid concurrent
> alteration. To provide portability,
> the boost::atomic tools were used, instead of using compiler dependent
> built/in functions.
> The current C++11 standard could have been used as well, std::atomic
> provides with the same functionality and portability.
> The modified files are: CGAL/Handle_for.h and CGAL/Handle.h

For that part, I have decided to use the C++11 atomics when available, then 
fallback to Boost.Atomic, and only fallback to the non-documented 
`boost::detail::atomic_count` when none of the other solutions are available.

> In CGAL/Lazy.h the version of the approx() method that is not constant
> and do returns a reference to the inner
> object was dropped. We want to avoid anyone from modify the state of the
> object so we can enforce thread
> safety. It seems that this method is not used during compilation and can
> be safely deleted.

I agree.

> The Lazy_rep object keeps two objects that hold the approximate value:
> at, that holds the
> original approximate value, and at_exact that holds the exact value that
> is recalculated inside
> update_exact. In this way, a thread that calls approx and ta thread that
> calls update_exact operate on
> different objects and we can ensure thread safety.
> Instead of utilizing a mutex to check whenever the approximate value has
> being updated, the
> check is done done via an atomic counter. This counter is initialized
> with value 0 to represent that
> the value is an approximation. The last statement of the update_exact
> operation updates the value to
> represent the fact that the value is no longer an approximation.

I do not understand your patch for `approx()`. I do not wee the atomic 
variable that protects the value `this->at` from being access by a reader 
while a writer thread is updating it from `exact()`.

I have added a lock of the mutex in `approx()` as well. Ideally, a shared 
mutex should be used: locked "shared" in `approx()`, by reader threads, and 
only locked with strict ownership in `exact()`, by a writer thread.

-- 
Laurent Rineau, PhD
R&D Engineer at GeometryFactory           http://www.geometryfactory.com/
Release Manager of the CGAL Project       http://www.cgal.org/