MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0027491Community[OCCT] OCCT:Modeling Datapublic2016-05-11 18:272021-06-16 19:15
ReporterBenjaminBihler 
Assigned Tobugmaster 
PrioritynormalSeveritymajor 
StatusclosedResolutiondocumentation updated 
PlatformWindowsOSVC++ 2015OS Version64 bit
Product Version[OCCT] 7.0.0 
Target Version[OCCT] 7.1.0Fixed in Version[OCCT] 7.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
Attached Files

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

-  Notes
(0054263)
abv (manager)
2016-05-21 11:05

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.
(0054264)
abv (manager)
2016-05-21 11:09

Besides, you can get the same behavior as in OCCT 6.x if you protect access to your adaptor instance by mutex.
(0054315)
abv (manager)
2016-05-23 20:14

It can be possible to make adapters thread safe if improvement suggested in 0027074 is implemented.
(0054322)
BenjaminBihler (developer)
2016-05-24 10:13

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.
(0054590)
BenjaminBihler (developer)
2016-06-01 11:45

Any feedback regarding the documentation?
(0054595)
msv (developer)
2016-06-01 12:45

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
(0054598)
git (administrator)
2016-06-01 14:27

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.
(0054599)
BenjaminBihler (developer)
2016-06-01 14:30

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.
(0054600)
msv (developer)
2016-06-01 15:17

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.
(0054605)
mkv (tester)
2016-06-01 18:03

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
(0054606)
mkv (tester)
2016-06-01 18:03

Dear BugMaster,
Branch CR27491 is TESTED.
(0054663)
git (administrator)
2016-06-03 13:12

Branch CR27491 has been deleted by inv.

SHA-1: 55cabfc29489227320f765ec66037d42e09a1afa

- Related Changesets
occt: master 34e4e9f2
Timestamp: 2016-06-01 11:24:23
Author: 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.
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 View Revisions
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


Copyright © 2000 - 2021 MantisBT Team
Powered by Mantis Bugtracker