View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0027491CommunityOCCT:Modeling Datapublic2016-05-11 18:272021-06-16 19:15
ReporterBenjaminBihler Assigned Tobugmaster  
PrioritynormalSeveritymajor 
Status closedResolutiondocumentation updated 
PlatformWindowsOSVC++ 2015 
Product Version7.0.0 
Target Version7.1.0Fixed in Version7.1.0 
Summary0027491: Modeling Data - document thread-safety behavior of GeomAdaptor_Curve
DescriptionI 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?
Steps To ReproduceNot required, only the documentation has been enhanced.
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0027074 feedbackabv Community Support of multi-span cache in Geom[2d]Adaptor 
related to 0032449 closedbugmaster Open CASCADE Modeling Algorithms - make curves adaptors classes thread safe 

Activities

abv

2016-05-21 11:05

manager   ~0054263

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.

abv

2016-05-21 11:09

manager   ~0054264

Besides, you can get the same behavior as in OCCT 6.x if you protect access to your adaptor instance by mutex.

abv

2016-05-23 20:14

manager   ~0054315

It can be possible to make adapters thread safe if improvement suggested in 0027074 is implemented.

BenjaminBihler

2016-05-24 10:13

developer   ~0054322

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.

BenjaminBihler

2016-06-01 11:45

developer   ~0054590

Any feedback regarding the documentation?

msv

2016-06-01 12:45

developer   ~0054595

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

git

2016-06-01 14:27

administrator   ~0054598

Branch CR27491 has been created by BenjaminBihler.

SHA-1: 55cabfc29489227320f765ec66037d42e09a1afa


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.

BenjaminBihler

2016-06-01 14:30

developer   ~0054599

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.

msv

2016-06-01 15:17

developer   ~0054600

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 bugmaster, there is no need in testing, only documentation has been updated.

mkv

2016-06-01 18:03

tester   ~0054605

Dear BugMaster,
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.
SHA-1: 55cabfc29489227320f765ec66037d42e09a1afa

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

mkv

2016-06-01 18:03

tester   ~0054606

Dear BugMaster,
Branch CR27491 is TESTED.

git

2016-06-03 13:12

administrator   ~0054663

Branch CR27491 has been deleted by inv.

SHA-1: 55cabfc29489227320f765ec66037d42e09a1afa

Related Changesets

occt: master 34e4e9f2

2016-06-01 11:24:23

BenjaminBihler


Committer: bugmaster Details Diff		 0027491: GeomAdaptor_Curve Is Not Thread-Safe (Anymore?)

Added hint about BSpline cache and missing thread-safety to adaptor classes.		 Affected Issues
0027491
mod - src/Adaptor2d/Adaptor2d_Curve2d.hxx Diff File
mod - src/Adaptor3d/Adaptor3d_Curve.hxx Diff File
mod - src/Adaptor3d/Adaptor3d_Surface.hxx Diff File
mod - src/Geom2dAdaptor/Geom2dAdaptor_Curve.hxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_Curve.hxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_Surface.hxx Diff File

Issue History

Date Modified Username Field Change
2016-05-11 18:27 BenjaminBihler New Issue
2016-05-11 18:27 BenjaminBihler Assigned To => abv
2016-05-21 11:05 abv Note Added: 0054263
2016-05-21 11:05 abv Assigned To abv => BenjaminBihler
2016-05-21 11:05 abv Status new => feedback
2016-05-21 11:09 abv Note Added: 0054264
2016-05-23 19:26 abv Relationship added related to 0027074
2016-05-23 20:14 abv Note Added: 0054315
2016-05-24 10:13 BenjaminBihler Note Added: 0054322
2016-05-25 10:20 BenjaminBihler Assigned To BenjaminBihler => abv
2016-06-01 11:45 BenjaminBihler Note Added: 0054590
2016-06-01 12:45 msv Note Added: 0054595
2016-06-01 14:27 git Note Added: 0054598
2016-06-01 14:30 BenjaminBihler Note Added: 0054599
2016-06-01 14:30 BenjaminBihler Status feedback => resolved
2016-06-01 14:30 BenjaminBihler Steps to Reproduce Updated
2016-06-01 14:49 kgv Assigned To abv => msv
2016-06-01 14:49 kgv Category OCCT:Foundation Classes => OCCT:Modeling Data
2016-06-01 14:51 kgv Summary GeomAdaptor_Curve Is Not Thread-Safe (Anymore?) => Modeling Data - document thread-safety behavior of GeomAdaptor_Curve
2016-06-01 15:17 msv Note Added: 0054600
2016-06-01 15:17 msv Assigned To msv => bugmaster
2016-06-01 15:17 msv Status resolved => reviewed
2016-06-01 16:21 mkv Assigned To bugmaster => mkv
2016-06-01 18:03 mkv Note Added: 0054605
2016-06-01 18:03 mkv Note Added: 0054606
2016-06-01 18:03 mkv Assigned To mkv => bugmaster
2016-06-01 18:03 mkv Status reviewed => tested
2016-06-01 18:03 mkv Test case number => Not needed
2016-06-03 12:50 bugmaster Changeset attached => occt master 34e4e9f2
2016-06-03 12:50 bugmaster Status tested => verified
2016-06-03 12:50 bugmaster Resolution open => fixed
2016-06-03 13:12 git Note Added: 0054663
2016-11-02 16:33 msv Resolution fixed => documentation updated
2016-11-03 13:07 ysn Assigned To bugmaster => duv
2016-11-03 13:08 ysn Assigned To duv => ysn
2016-11-03 14:50 ysn Assigned To ysn => bugmaster
2016-12-09 16:30 aiv Status verified => closed
2016-12-09 16:39 aiv Fixed in Version => 7.1.0
2021-06-16 19:15 msv Relationship added related to 0032449