Archived issue #0027491
Modeling Data - document thread-safety behavior of GeomAdaptor_Curve
Description
I use OpenMP to do parallel computations on a curve like this (pseudo-code):
const Adaptor3d_Curve curve = GeomAdaptor_Curve(anotherCurve);
#pragma omp parallel for
for (int step = 0; step <= steps; ++step)
{
computeSomething(curve);
}
where the method computeSomething takes a const reference to the curve and evaluates it with curve.D1().
I can reproduce it here, that the results of D1 differ strongly depending on whether I do the D1 call inside "#pragma omp critical" or not. This tells me that D1 is not thread-safe (both on Windows 64 and on Debian Linux 64). But the behaviour was different with OCE 6.8.0 (yes, the community edition, if have not used the official version at that time - but most probably the behaviour was the same with OCCT 6.8.0).
Is GeomAdaptor_Curve intended to be thread-safe? Would it help you in this case, if I tried to create a minimum example showing the problem? Or is GeomAdaptor_Curve not thread-safe by design?
const Adaptor3d_Curve curve = GeomAdaptor_Curve(anotherCurve);
#pragma omp parallel for
for (int step = 0; step <= steps; ++step)
{
computeSomething(curve);
}
where the method computeSomething takes a const reference to the curve and evaluates it with curve.D1().
I can reproduce it here, that the results of D1 differ strongly depending on whether I do the D1 call inside "#pragma omp critical" or not. This tells me that D1 is not thread-safe (both on Windows 64 and on Debian Linux 64). But the behaviour was different with OCE 6.8.0 (yes, the community edition, if have not used the official version at that time - but most probably the behaviour was the same with OCCT 6.8.0).
Is GeomAdaptor_Curve intended to be thread-safe? Would it help you in this case, if I tried to create a minimum example showing the problem? Or is GeomAdaptor_Curve not thread-safe by design?
Steps to reproduce
Not required, only the documentation has been enhanced.
Public activity
12 archived notes
Participants are labeled by their role within this record.
Adaptor classes are not thread safe by design.
The major reason is that polynomial coefficients of b-splines used for their evaluation are cached for better performance. When you evaluate the same b-spline from parallel threads, they will contend for that cache. Prior to OCCT 7.0, thread safety of evaluation of b-spline curves and surfaces was achieved by mutex stored in curve or surface class, which actually blocked parallel execution when several threads evaluate the same b-spline. In OCCT 7.0, this cache has been moved to adaptor classes. It is assumed that each thread should keep its own instance of the adaptor.
The major reason is that polynomial coefficients of b-splines used for their evaluation are cached for better performance. When you evaluate the same b-spline from parallel threads, they will contend for that cache. Prior to OCCT 7.0, thread safety of evaluation of b-spline curves and surfaces was achieved by mutex stored in curve or surface class, which actually blocked parallel execution when several threads evaluate the same b-spline. In OCCT 7.0, this cache has been moved to adaptor classes. It is assumed that each thread should keep its own instance of the adaptor.
Besides, you can get the same behavior as in OCCT 6.x if you protect access to your adaptor instance by mutex.
It can be possible to make adapters thread safe if improvement suggested in #0027074 is implemented.
Andrey, thank you for the explanation. Now I understand it and I can live with that. Yet I have the feeling that this should be documented! I didn't find a note about the non-thread-safety in Adaptor3d_Curve.hxx.
Should I add a note and your explanation to all Adaptor*.hxx parent classes?
Also the 7.0.0 upgrade notes give "Possibility to evaluate the same B-Spline concurrently in parallel threads without data races and mutex locks" as a new feature. This is perfectly right, but it should be mentioned that evaluations of the same adaptor are not at all thread-safe.
Should I add a note and your explanation to all Adaptor*.hxx parent classes?
Also the 7.0.0 upgrade notes give "Possibility to evaluate the same B-Spline concurrently in parallel threads without data races and mutex locks" as a new feature. This is perfectly right, but it should be mentioned that evaluations of the same adaptor are not at all thread-safe.
Any feedback regarding the documentation?
Dear Benjamin, sorry for long time response.
Yes, you are encouraged to update documentation of adaptor classes. Please make a new branch with your contribution.
Mikhail
Yes, you are encouraged to update documentation of adaptor classes. Please make a new branch with your contribution.
Mikhail
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Benjamin Bihler
Date: Wed Jun 1 13:24:23 2016 +0200
0027491: GeomAdaptor_Curve Is Not Thread-Safe (Anymore?)
Added hint about BSpline cache and missing thread-safety to adaptor classes.
[revision removed]
Detailed log of new commits:
Author: Benjamin Bihler
Date: Wed Jun 1 13:24:23 2016 +0200
0027491: GeomAdaptor_Curve Is Not Thread-Safe (Anymore?)
Added hint about BSpline cache and missing thread-safety to adaptor classes.
Mikhail, I have added the note to Geom2dAdaptor_Curve, GeomAdaptor_Curve, GeomAdaptor_Surface and their parent classes, since IMHO they are the only affected classes.
Indeed, there are a lot of descendants of Adaptor classes, and all of them are affected, since they can encapsulate corresponding GeomAdaptor class, see for example BRepAdaptor_Curve.
However, I consider it is enough to document this issue only in those classes you have changed already.
Thank you.
Dear Commenter 1, there is no need in testing, only documentation has been updated.
However, I consider it is enough to document this issue only in those classes you have changed already.
Thank you.
Dear Commenter 1, there is no need in testing, only documentation has been updated.
Dear Commenter 1,
Branch CR27491 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
Number of compiler warnings:
occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)
products component :
Linux: 72 (72 on master)
Windows: 4 (4 on master)
MacOS : 1137
Branch CR27491 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
Number of compiler warnings:
occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)
products component :
Linux: 72 (72 on master)
Windows: 4 (4 on master)
MacOS : 1137
Dear Commenter 1,
Branch CR27491 is TESTED.
Branch CR27491 is TESTED.
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Related records