View Issue Details

IDProjectCategoryView StatusLast Update
0032265CommunityOCCT:Modeling Algorithmspublic2021-12-13 14:56
Reporterdrazmyslovich Assigned Tobugmaster  
PrioritynormalSeverityfeature 
Status closedResolutionwon't fix 
Product Version7.6.0 
Target Version7.6.0 
Summary0032265: Modeling Algorithms - some trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection
DescriptionThis issue is a part of changes, which were initially submitted into 0032261.

Hi,

in course of investigating diverse meshing problems with different models I have produced several trivial code improvements in the context of meshing algorithm, which I would like to share here.

The improvements are the following:
6. CSLib_Class2d even thought the implementation of NCollection_Handle::get is quite trivial, I saw it several times in the profiler taking some 5% of meshing time (maybe this was just due to debug mode and some address sanitizing - I am not sure)
7. GCPnts_TangentialDeflection::PerformCurve consider near points not only by parameter value, but also by 3D location

Regards,
Dima
Steps To ReproduceN/A
TagsNo tags attached.
Test case number

Relationships

related to 0032261 closedbugmaster Mesh - some trivial improvements for mesher 
related to 0030020 assignedmsv Modeling Data - Incorrect bspline visualization 
related to 0030874 closedapn Modeling Algorithms - GCPnts_TangentialDeflection inserts the points between nearby points 

Activities

git

2021-03-30 08:28

administrator   ~0099839

Branch CR0032265 has been created by drazmyslovich.

SHA-1: 73f8ed308631d67bc0e1c808f716b46dafcf8d33


Detailed log of new commits:

Author: Dzmitry Razmyslovich
Date: Tue Mar 30 07:27:47 2021 +0200

    0032265: Some trivial code improvements for CSLib_Class2d and GCPnts_TangentialDeflection

msv

2021-03-30 10:44

developer   ~0099847

I wonder what is the benefit of using reference instead of pointer in CSLib_Class2d.cxx.

msv

2021-03-30 10:49

developer   ~0099848

I guess the changes in GCPnts_TangentialDeflection.pxx solve some real bug in your application. Is it possible to attach a shape and create a draw script to reproduce it?
Also, it is desirable to add a detailed info about changes in the commit message. What for the changes, what problem they solve.

kgv

2021-03-30 10:56

developer   ~0099849

Last edited: 2021-03-30 10:58

> 6. CSLib_Class2d even thought the implementation of NCollection_Handle::get is quite trivial,
> I saw it several times in the profiler taking some 5% of meshing time (maybe this was just due to debug mode and some address sanitizing - I am not sure)

I think that profiler statistics was misleading - I highly doubt that operator->() may involve any extra CPU cycles in Release builds.
It would be useful to have some real performance estimations of entire function logic before/after patch to ensure that it is justified.
And, of course, profiling of Debug builds is usually not helpful (it is better using ReleaseWithDebugInfo builds for profiling).
  T* get () { return ((Ptr*)opencascade::handle<Standard_Transient>::get())->myPtr; }


drazmyslovich

2021-04-01 13:27

developer   ~0099944

The changes in GCPnts_TangentialDeflection.pxx is an early bailout, which just saves some CPU cycles, as the later conditions are going to fail anyway.

Regarding the changes in CSLib_Class2d - I surely agree, that there should be no impact of get method.

I will split the changes into 2 commits, so that they can be picked by one.

git

2021-04-01 14:58

administrator   ~0099949

Branch CR0032265 has been updated forcibly by drazmyslovich.

SHA-1: 7e2f13c0586286b402b379e6d5145a540e4586ac

drazmyslovich

2021-04-01 15:00

developer   ~0099950

Please, check the latest commits

msv

2021-04-01 15:20

developer   ~0099952

I would discard the changes in CSLib_Class2d.

msv

2021-04-01 15:21

developer   ~0099953

Also, please do the commit message as follows:
1. The first line is a copy from bug summary.
2. 2nd line is empty
3. Add description of changes here.

git

2021-04-06 11:20

administrator   ~0100119

Branch CR0032265 has been updated forcibly by drazmyslovich.

SHA-1: 9d135c87b21455b5f2ef41638a2201f2ea645873

drazmyslovich

2021-04-06 11:21

developer   ~0100120

Done

git

2021-04-06 12:21

administrator   ~0100129

Branch CR0032265 has been updated forcibly by msv.

SHA-1: b3100f20d79ed7d1b38ff3b242d33dc7633a87e4

msv

2021-04-06 12:35

developer   ~0100130

Last edited: 2021-04-06 12:35

Rebased on master, corrected the title of the commit message.
Testing http://jenkins-test-12.nnov.opencascade.com/view/CR0032265-master-msv/view/COMPARE/.

msv

2021-04-06 14:15

developer   ~0100131

Dear Dmitry,
There was already discussion about checking 3D points in this code. It was in the bug 0030874. There we agreed to check points only by parameter value.
The current patch introduces the risk of skipping loops in a curve if the curve has self-intersection. We cannot pass such regression in the code.
Moreover, the benefit of this patch is not obvious. Could you give some numbers to compare performance?

drazmyslovich

2021-04-06 19:52

developer   ~0100142

Dear Mikhail,

the performance impact of this change is not remarkable.

I am sorry, that I have spent your time on investigating again the already discussed topic. Somehow I forgot, that I have already submitted this patch.

Let's just close this issue.

msv

2021-04-06 19:56

developer   ~0100143

Dear bugmaster, please close this bug.

git

2021-12-13 14:56

administrator   ~0105847

Branch CR0032265 has been deleted by kgv.

SHA-1: b3100f20d79ed7d1b38ff3b242d33dc7633a87e4

Issue History

Date Modified Username Field Change
2021-03-30 08:26 drazmyslovich New Issue
2021-03-30 08:26 drazmyslovich Assigned To => msv
2021-03-30 08:28 git Note Added: 0099839
2021-03-30 08:28 drazmyslovich Status new => resolved
2021-03-30 08:28 drazmyslovich Steps to Reproduce Updated
2021-03-30 08:28 drazmyslovich Relationship added related to 0032261
2021-03-30 08:45 kgv Severity minor => feature
2021-03-30 08:45 kgv Summary Some trivial code improvements => Modeling Algorithms - ыome trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection
2021-03-30 10:39 msv Summary Modeling Algorithms - ыome trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection => Modeling Algorithms - some trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection
2021-03-30 10:44 msv Note Added: 0099847
2021-03-30 10:49 msv Note Added: 0099848
2021-03-30 10:50 msv Assigned To msv => drazmyslovich
2021-03-30 10:50 msv Status resolved => assigned
2021-03-30 10:56 kgv Note Added: 0099849
2021-03-30 10:57 kgv Note Edited: 0099849
2021-03-30 10:57 kgv Note Edited: 0099849
2021-03-30 10:58 kgv Note Edited: 0099849
2021-04-01 13:27 drazmyslovich Note Added: 0099944
2021-04-01 14:58 git Note Added: 0099949
2021-04-01 15:00 drazmyslovich Note Added: 0099950
2021-04-01 15:00 drazmyslovich Assigned To drazmyslovich => msv
2021-04-01 15:00 drazmyslovich Status assigned => resolved
2021-04-01 15:20 msv Note Added: 0099952
2021-04-01 15:21 msv Note Added: 0099953
2021-04-01 15:22 msv Assigned To msv => drazmyslovich
2021-04-01 15:22 msv Status resolved => assigned
2021-04-06 11:20 git Note Added: 0100119
2021-04-06 11:21 drazmyslovich Note Added: 0100120
2021-04-06 11:21 drazmyslovich Assigned To drazmyslovich => msv
2021-04-06 11:21 drazmyslovich Status assigned => resolved
2021-04-06 12:21 git Note Added: 0100129
2021-04-06 12:35 msv Note Added: 0100130
2021-04-06 12:35 msv Note Edited: 0100130
2021-04-06 12:36 kgv Relationship added related to 0030020
2021-04-06 12:39 kgv Relationship added related to 0030874
2021-04-06 14:15 msv Note Added: 0100131
2021-04-06 15:03 msv Assigned To msv => drazmyslovich
2021-04-06 15:03 msv Status resolved => feedback
2021-04-06 19:52 drazmyslovich Note Added: 0100142
2021-04-06 19:52 drazmyslovich Assigned To drazmyslovich => msv
2021-04-06 19:52 drazmyslovich Status feedback => acknowledged
2021-04-06 19:56 msv Note Added: 0100143
2021-04-06 19:56 msv Assigned To msv => bugmaster
2021-04-06 19:56 msv Status acknowledged => feedback
2021-04-06 19:56 msv Resolution open => won't fix
2021-04-07 16:01 bugmaster Status feedback => closed
2021-12-13 14:56 git Note Added: 0105847