View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0032485Open CASCADEOCCT:Modeling Algorithmspublic2021-07-19 12:422021-09-04 16:27
ReporterasuravenAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version7.6.0Fixed in Version7.6.0 
Summary0032485: Modeling Algorithms - Add Clone() function for adapters
DescriptionAfter the analysis, it was decided not to add thread safety to adapters, but to ensure thread safety when using them. This requires creating copies of adapters for each thread, which is what the Clone () function should provide.
The function must be implemented in the childs of the following classes:
Geom2dAdaptor_Curve
GeomAdaptor_Curve
GeomAdaptor_Surface
Steps To Reproducenot required
TagsNo tags attached.
Test case numberNot required

Relationships

related to 0032449 closedbugmaster Modeling Algorithms - make curves adaptors classes thread safe 
related to 0032450 closedbugmaster Modeling Algorithms - change BRepLib_CheckCurveOnSurface & GeomLib_CheckCurveOnSurface interfaces to adaptors 

Activities

asuraven

2021-07-19 19:20

reporter   ~0102659

asuraven

2021-07-23 15:55

reporter   ~0102747

git

2021-07-26 12:40

administrator   ~0102813

Branch CR32485 has been created by asuraven.

SHA-1: 4837af7895a106c3f915f55a1091dc3c5df2214d


Detailed log of new commits:

Author: asuraven
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters

Author: asuraven
Date: Tue Jul 13 17:17:51 2021 +0300

    from patch

Author: asuraven
Date: Mon Jul 19 13:17:28 2021 +0300

    OSD_ThreadPool::Launcher instead OSD_Parallel::For

git

2021-07-26 12:41

administrator   ~0102814

Branch CR32485_1 has been created by asuraven.

SHA-1: fa5043119efa45c733eaca49afb5330164688c5c


Detailed log of new commits:

Author: asuraven
Date: Fri Jul 23 19:15:28 2021 +0300

    0032485: More ShallowCopy()

Author: asuraven
Date: Mon Jul 26 12:34:30 2021 +0300

    set isMultiThread = true

Author: asuraven
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters

Author: asuraven
Date: Tue Jul 13 17:17:51 2021 +0300

    from patch

git

2021-07-27 18:46

administrator   ~0102839

Branch CR32485_1 has been updated by asuraven.

SHA-1: cdb8d56465a517a6789229df1b65eb30a0be5a28


Detailed log of new commits:

Author: asuraven
Date: Tue Jul 27 18:45:16 2021 +0300

    0032485: More ShallowCopy()+

asuraven

2021-07-28 11:27

reporter   ~0102847

git

2021-07-28 17:30

administrator   ~0102851

Branch CR32485_2 has been created by asuraven.

SHA-1: ecf82553c1cdaaf3e1f861c88ac170b1da059296


Detailed log of new commits:

Author: asuraven
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters

git

2021-07-29 12:36

administrator   ~0102870

Branch CR32485_2 has been updated forcibly by asuraven.

SHA-1: 8efa0a5ef3df8e51fd9943b1ffe458281671f00e

git

2021-07-29 19:03

administrator   ~0102874

Branch CR32485_2 has been updated forcibly by asuraven.

SHA-1: 1a104a1e9842f2253e0acf847188da3408e3af8a

git

2021-07-30 14:08

administrator   ~0102890

Branch CR32485_2 has been updated forcibly by asuraven.

SHA-1: 9473a8f29c7acd41659d833222135c6b45cbb4cc

asuraven

2021-07-30 15:34

reporter   ~0102891

Please review the code in branch CR32485_2
yesterday's tests results (not available now):
http://jenkins-test-occt/view/CR32485_2-master-ASURAVEN/view/COMPARE/
has only one fail - 'bugs modalg_5 bug24012'
but now it has been fixed

kgv

2021-07-30 17:24

developer   ~0102899

Last edited: 2021-07-30 17:25

 
+Handle(Adaptor3d_Curve) Adaptor3d_Curve::ShallowCopy() const
+{
+  throw Standard_NotImplemented("Adaptor3d_Curve::ShallowCopy");
+}

I think that the stab implemented by base class in hierarchy of Adaptors is a legacy of old C++/CDL code.
Could you try declaring new interface (and try also updating old ones in these classes) as pure virtual methods?

Or there are real use cases, where sub-classes are supposed to not implement these methods?

msv

2021-08-02 19:42

developer   ~0102935

src/GeomAdaptor/GeomAdaptor_SurfaceOfLinearExtrusion.cxx
src/GeomAdaptor/GeomAdaptor_SurfaceOfRevolution.cxx
  aCopy->mySurfaceCache   = mySurfaceCache;

Cache must not be copied.

src/Geom2dEvaluator/Geom2dEvaluator_OffsetCurve.cxx
src/GeomEvaluator/GeomEvaluator_OffsetCurve.cxx
src/GeomEvaluator/GeomEvaluator_OffsetSurface.cxx
src/GeomEvaluator/GeomEvaluator_SurfaceOfExtrusion.cxx
src/GeomEvaluator/GeomEvaluator_SurfaceOfRevolution.cxx
You should select what constructor to call depending on the condition "myBaseAdaptor.IsNull()".

src/ProjLib/ProjLib_CompProjectedCurve.cxx
myTabInt is rewritten in the methods NbIntervals/Intervals, so it must not be shared.

src/BRepAdaptor/BRepAdaptor_Curve.cxx
  aCopy->myCurve = myCurve;

src/BRepAdaptor/BRepAdaptor_Surface.cxx
  aCopy->mySurf = mySurf;

In such way the cache of curve/surface adaptor will be shared, you should avoid it. It is better to create a shallow copy, and then save the value under its handle, as you are doing in BRepAdaptor_CompCurve.

src/BRepAdaptor/BRepAdaptor_Curve2d.cxx
  aCopy->myCurveCache = myCurveCache;

Cache must not be copied.

src/ChFiDS/ChFiDS_ElSpine.cxx
Do not change type of the field "curve". Instead do the same as in BRepAdaptor_Curve.

In the commit message, add a sentence that it will allow using copies of the same adapter in multi-thread calculations.

git

2021-08-16 12:50

administrator   ~0103236

Branch CR32485_3 has been created by asuraven.

SHA-1: 39e4658b5ae42546ab5a57d2d036aedf3290a657


Detailed log of new commits:

Author: asuraven
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters
    
    Provide ShallowCopy() functions for adapters & evaluators of curves, 2d curves and surfaces. This will allow using copies of the same adapter in multi-thread calculations.

asuraven

2021-08-16 13:33

reporter   ~0103238

Last edited: 2021-08-19 13:41

 Kirill, unfortunately, we cannot declare functions in the Adapter3d_Curve class as pure virtual because, in particular, the OffsetCurve() function is implemented only for the GeomAdaptor_Curve and BRepAdaptor_Curve child classes.

kgv

2021-08-16 14:00

developer   ~0103239

OffsetCurve() could be declared as non-virtual as exception or we may implement it in descendants.

git

2021-08-30 12:23

administrator   ~0103529

Branch CR32485_3 has been updated forcibly by asuraven.

SHA-1: a9523f1a08defa68bd3a9cae71af5985850ee23a

asuraven

2021-08-30 18:01

reporter   ~0103554

git

2021-08-31 18:36

administrator   ~0103591

Branch CR32485_3 has been updated forcibly by asuraven.

SHA-1: 73f116aa3dece71e80b3eefa7b6bc20bf4a48084

git

2021-08-31 19:18

administrator   ~0103593

Branch CR32485_3 has been updated forcibly by asuraven.

SHA-1: a9523f1a08defa68bd3a9cae71af5985850ee23a

asuraven

2021-08-31 19:25

reporter   ~0103594

Last edited: 2021-08-31 19:41

 > OffsetCurve() could be declared as non-virtual as exception or we may implement it in descendants.

Kirill, we have not implemented errors in Products after I tried switch virtual functions to pure.
http://jenkins-test-occt.nnov.opencascade.com/job/CR32485_3-master-ASURAVEN-Products-MacOS-opt-compile/1/parsed_console/

asuraven

2021-08-31 19:30

reporter   ~0103595

remarks fixed in branch CR32485_3 exclude switch to pure virtual functions as it causes problems in Products

kgv

2021-08-31 19:37

developer   ~0103597

Last edited: 2021-08-31 19:39

 > Kirill, we have not implemented errors in Products after I tried switch virtual functions to pure.
I'm a little confused what is the problem updating Products - I guess there would be a couple of classes there, not something requiring considerable efforts.

But lets Mikhail to decide.

msv

2021-08-31 20:12

developer   ~0103599

There are several adaptors in products having not implemented virtual functions.

HelixGeom_HelixCurve: ShallowCopy(), D3(), GetType()
AcisOther_AdaptorIntCur: ShallowCopy(), D3(), GetType(), Resolution()
AcisData_CurveLawAdaptor: ShallowCopy(), D3(), Resolution(), IsPeriodic(), Period()

There are different algorithms using adaptors, and each algorithm applies different requirements on the presence of virtual functions in adaptor. If we make all virtual functions "pure" then we shall need to implement them in all adaptors.
What are the benefits of this improvement? I see no one, only disadvantages, as some user applications may also suffer from this change.

kgv

2021-08-31 20:55

developer   ~0103602

Last edited: 2021-08-31 20:55

 > What are the benefits of this improvement?
The benefit is to clearly define which adaptor methods are mandatory to be implemented to be used within main OCCT algorithms.
So that mandatory ones like D0/D1 are declared as poor virtual methods, while optional could have default implementation throwing exception.

But if there are no methods in adaptors required to be useful, then OK, lets drop this idea.

msv

2021-08-31 21:24

developer   ~0103603

There are different OCCT algorithms, which of them to consider 'main'? GCPnts, Extrema, Approx, LProp, and many others, each of them use its own set of adaptor functions.

We could gather the list of functions common for all algorithms. But if we do so, we do not clearly define all mandatory functions needed for all algorithms.

If we make pure virtual all functions that are used in all algorithms, then if the user wants to implement an adaptor for some particular algorithm we force him to implement a set of functions that are not needed for that algorithm.

kgv

2021-08-31 22:05

developer   ~0103605

> There are different OCCT algorithms...
OK, got it.

asuraven

2021-09-01 12:37

reporter   ~0103616

tests results^
http://jenkins-test-occt.nnov.opencascade.com/view/CR32485_3-master-ASURAVEN/view/COMPARE/

msv

2021-09-01 12:39

developer   ~0103618

For integration:
occt - CR32485_3
products - none

smoskvin

2021-09-04 14:24

administrator   ~0103752

Combination -
OCCT branch : IR-2021-09-03
master SHA - f26ee38f2a309ffbf7de4eebbcef2c5a5c57d84e
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-09-03 SHA - 87cca1a8f3dd94387a936b9d49f5bd719c69cf4d
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17495.94000000042 / 17441.55000000031 [+0.31%]
Products
Total CPU difference: 11565.440000000113 / 11534.270000000102 [+0.27%]
Windows-64-VC14:
OCCT
Total CPU difference: 19307.140625 / 19200.421875 [+0.56%]
Products
Total CPU difference: 12917.859375 / 12874.53125 [+0.34%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2021-09-04 14:59

administrator   ~0103790

Branch CR32485 has been deleted by mnt.

SHA-1: 4837af7895a106c3f915f55a1091dc3c5df2214d

git

2021-09-04 14:59

administrator   ~0103791

Branch CR32485_1 has been deleted by mnt.

SHA-1: cdb8d56465a517a6789229df1b65eb30a0be5a28

git

2021-09-04 14:59

administrator   ~0103792

Branch CR32485_2 has been deleted by mnt.

SHA-1: 9473a8f29c7acd41659d833222135c6b45cbb4cc

git

2021-09-04 14:59

administrator   ~0103793

Branch CR32485_3 has been deleted by mnt.

SHA-1: a9523f1a08defa68bd3a9cae71af5985850ee23a

Related Changesets

occt: master 872a7e3a

2021-07-20 15:44:23

asuraven


Committer: bugmaster Details Diff		 0032485: Modeling Algorithms - Add Clone() function for adapters

Provide ShallowCopy() functions for adapters & evaluators of curves, 2d curves and surfaces. This will allow using copies of the same adapter in multi-thread calculations.		 Affected Issues
0032485
mod - src/Adaptor2d/Adaptor2d_Curve2d.cxx Diff File
mod - src/Adaptor2d/Adaptor2d_Curve2d.hxx Diff File
mod - src/Adaptor2d/Adaptor2d_Line2d.cxx Diff File
mod - src/Adaptor2d/Adaptor2d_Line2d.hxx Diff File
mod - src/Adaptor2d/Adaptor2d_OffsetCurve.cxx Diff File
mod - src/Adaptor2d/Adaptor2d_OffsetCurve.hxx Diff File
mod - src/Adaptor3d/Adaptor3d_Curve.cxx Diff File
mod - src/Adaptor3d/Adaptor3d_Curve.hxx Diff File
mod - src/Adaptor3d/Adaptor3d_CurveOnSurface.cxx Diff File
mod - src/Adaptor3d/Adaptor3d_CurveOnSurface.hxx Diff File
mod - src/Adaptor3d/Adaptor3d_IsoCurve.cxx Diff File
mod - src/Adaptor3d/Adaptor3d_IsoCurve.hxx Diff File
mod - src/Adaptor3d/Adaptor3d_Surface.cxx Diff File
mod - src/Adaptor3d/Adaptor3d_Surface.hxx Diff File
mod - src/BiTgte/BiTgte_CurveOnEdge.cxx Diff File
mod - src/BiTgte/BiTgte_CurveOnEdge.hxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_CompCurve.cxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_CompCurve.hxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_Curve.cxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_Curve.hxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_Curve2d.cxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_Curve2d.hxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_Surface.cxx Diff File
mod - src/BRepAdaptor/BRepAdaptor_Surface.hxx Diff File
mod - src/ChFiDS/ChFiDS_ElSpine.cxx Diff File
mod - src/ChFiDS/ChFiDS_ElSpine.hxx Diff File
mod - src/Geom2dAdaptor/Geom2dAdaptor_Curve.cxx Diff File
mod - src/Geom2dAdaptor/Geom2dAdaptor_Curve.hxx Diff File
mod - src/Geom2dEvaluator/Geom2dEvaluator_Curve.hxx Diff File
mod - src/Geom2dEvaluator/Geom2dEvaluator_OffsetCurve.cxx Diff File
mod - src/Geom2dEvaluator/Geom2dEvaluator_OffsetCurve.hxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_Curve.cxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_Curve.hxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_Surface.cxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_Surface.hxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_SurfaceOfLinearExtrusion.cxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_SurfaceOfLinearExtrusion.hxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_SurfaceOfRevolution.cxx Diff File
mod - src/GeomAdaptor/GeomAdaptor_SurfaceOfRevolution.hxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_Curve.hxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_OffsetCurve.cxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_OffsetCurve.hxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_OffsetSurface.cxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_OffsetSurface.hxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_Surface.hxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_SurfaceOfExtrusion.cxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_SurfaceOfExtrusion.hxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_SurfaceOfRevolution.cxx Diff File
mod - src/GeomEvaluator/GeomEvaluator_SurfaceOfRevolution.hxx Diff File
mod - src/GeomFill/GeomFill_SnglrFunc.cxx Diff File
mod - src/GeomFill/GeomFill_SnglrFunc.hxx Diff File
mod - src/ProjLib/ProjLib_CompProjectedCurve.cxx Diff File
mod - src/ProjLib/ProjLib_CompProjectedCurve.hxx Diff File
mod - src/ProjLib/ProjLib_ProjectedCurve.cxx Diff File
mod - src/ProjLib/ProjLib_ProjectedCurve.hxx Diff File
mod - src/ProjLib/ProjLib_ProjectOnPlane.cxx Diff File
mod - src/ProjLib/ProjLib_ProjectOnPlane.hxx Diff File

Issue History

Date Modified Username Field Change
2021-07-19 12:42 asuraven New Issue
2021-07-19 12:42 asuraven Assigned To => asuraven
2021-07-19 12:43 asuraven Relationship added related to 0032449
2021-07-19 12:43 asuraven Relationship added related to 0032450
2021-07-19 19:20 asuraven Note Added: 0102659
2021-07-20 18:44 asuraven Summary Modeling Algorithms - Add Clone() function for adaptors => Modeling Algorithms - Add Clone() function for adapters
2021-07-23 15:55 asuraven Note Added: 0102747
2021-07-26 12:40 git Note Added: 0102813
2021-07-26 12:41 git Note Added: 0102814
2021-07-27 18:46 git Note Added: 0102839
2021-07-28 11:27 asuraven Status new => assigned
2021-07-28 11:27 asuraven Note Added: 0102847
2021-07-28 17:30 git Note Added: 0102851
2021-07-29 12:36 git Note Added: 0102870
2021-07-29 19:03 git Note Added: 0102874
2021-07-30 14:08 git Note Added: 0102890
2021-07-30 15:34 asuraven Note Added: 0102891
2021-07-30 15:34 asuraven Assigned To asuraven => msv
2021-07-30 15:34 asuraven Status assigned => resolved
2021-07-30 17:24 kgv Note Added: 0102899
2021-07-30 17:25 kgv Note Edited: 0102899
2021-08-02 19:42 msv Note Added: 0102935
2021-08-02 19:42 msv Assigned To msv => asuraven
2021-08-02 19:42 msv Status resolved => assigned
2021-08-16 12:50 git Note Added: 0103236
2021-08-16 13:33 asuraven Note Added: 0103238
2021-08-16 14:00 kgv Note Added: 0103239
2021-08-19 13:41 asuraven Note Edited: 0103238
2021-08-30 12:23 git Note Added: 0103529
2021-08-30 18:01 asuraven Note Added: 0103554
2021-08-31 18:36 git Note Added: 0103591
2021-08-31 19:18 git Note Added: 0103593
2021-08-31 19:25 asuraven Note Added: 0103594
2021-08-31 19:30 asuraven Note Added: 0103595
2021-08-31 19:30 asuraven Assigned To asuraven => msv
2021-08-31 19:30 asuraven Status assigned => resolved
2021-08-31 19:37 kgv Note Added: 0103597
2021-08-31 19:38 kgv Note Edited: 0103597
2021-08-31 19:39 kgv Note Edited: 0103597
2021-08-31 19:41 asuraven Note Edited: 0103594
2021-08-31 20:12 msv Note Added: 0103599
2021-08-31 20:55 kgv Note Added: 0103602
2021-08-31 20:55 kgv Note Edited: 0103602
2021-08-31 21:24 msv Note Added: 0103603
2021-08-31 22:05 kgv Note Added: 0103605
2021-09-01 12:37 asuraven Note Added: 0103616
2021-09-01 12:39 msv Note Added: 0103618
2021-09-01 12:39 msv Assigned To msv => bugmaster
2021-09-01 12:39 msv Status resolved => reviewed
2021-09-04 14:24 smoskvin Note Added: 0103752
2021-09-04 14:24 smoskvin Status reviewed => tested
2021-09-04 14:36 bugmaster Changeset attached => occt master 872a7e3a
2021-09-04 14:36 bugmaster Status tested => verified
2021-09-04 14:36 bugmaster Resolution open => fixed
2021-09-04 14:59 git Note Added: 0103790
2021-09-04 14:59 git Note Added: 0103791
2021-09-04 14:59 git Note Added: 0103792
2021-09-04 14:59 git Note Added: 0103793
2021-09-04 16:27 smoskvin Test case number => Not required