MantisBT - Community
View Issue Details
0032265Community[OCCT] OCCT:Modeling Algorithmspublic2021-03-30 08:262021-04-07 16:01
drazmyslovich 
bugmaster 
normalfeature 
closedwon't fix 
[OCCT] 7.6.0* 
[OCCT] 7.6.0* 
0032265: Modeling Algorithms - some trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection
This 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
N/A
No tags attached.
related to 0032261assigned oan Mesh - some trivial improvements for mesher 
related to 0030020assigned msv Modeling Data - Incorrect bspline visualization 
related to 0030874closed apn Modeling Algorithms - GCPnts_TangentialDeflection inserts the points between nearby points 
Issue History
2021-03-30 08:26drazmyslovichNew Issue
2021-03-30 08:26drazmyslovichAssigned To => msv
2021-03-30 08:28gitNote Added: 0099839
2021-03-30 08:28drazmyslovichStatusnew => resolved
2021-03-30 08:28drazmyslovichSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=24887#r24887
2021-03-30 08:28drazmyslovichRelationship addedrelated to 0032261
2021-03-30 08:45kgvSeverityminor => feature
2021-03-30 08:45kgvSummarySome trivial code improvements => Modeling Algorithms - —čome trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection
2021-03-30 10:39msvSummaryModeling 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:44msvNote Added: 0099847
2021-03-30 10:49msvNote Added: 0099848
2021-03-30 10:50msvAssigned Tomsv => drazmyslovich
2021-03-30 10:50msvStatusresolved => assigned
2021-03-30 10:56kgvNote Added: 0099849
2021-03-30 10:57kgvNote Edited: 0099849bug_revision_view_page.php?bugnote_id=99849#r24891
2021-03-30 10:57kgvNote Edited: 0099849bug_revision_view_page.php?bugnote_id=99849#r24892
2021-03-30 10:58kgvNote Edited: 0099849bug_revision_view_page.php?bugnote_id=99849#r24893
2021-04-01 13:27drazmyslovichNote Added: 0099944
2021-04-01 14:58gitNote Added: 0099949
2021-04-01 15:00drazmyslovichNote Added: 0099950
2021-04-01 15:00drazmyslovichAssigned Todrazmyslovich => msv
2021-04-01 15:00drazmyslovichStatusassigned => resolved
2021-04-01 15:20msvNote Added: 0099952
2021-04-01 15:21msvNote Added: 0099953
2021-04-01 15:22msvAssigned Tomsv => drazmyslovich
2021-04-01 15:22msvStatusresolved => assigned
2021-04-06 11:20gitNote Added: 0100119
2021-04-06 11:21drazmyslovichNote Added: 0100120
2021-04-06 11:21drazmyslovichAssigned Todrazmyslovich => msv
2021-04-06 11:21drazmyslovichStatusassigned => resolved
2021-04-06 12:21gitNote Added: 0100129
2021-04-06 12:35msvNote Added: 0100130
2021-04-06 12:35msvNote Edited: 0100130bug_revision_view_page.php?bugnote_id=100130#r24996
2021-04-06 12:36kgvRelationship addedrelated to 0030020
2021-04-06 12:39kgvRelationship addedrelated to 0030874
2021-04-06 14:15msvNote Added: 0100131
2021-04-06 15:03msvAssigned Tomsv => drazmyslovich
2021-04-06 15:03msvStatusresolved => feedback
2021-04-06 19:52drazmyslovichNote Added: 0100142
2021-04-06 19:52drazmyslovichAssigned Todrazmyslovich => msv
2021-04-06 19:52drazmyslovichStatusfeedback => acknowledged
2021-04-06 19:56msvNote Added: 0100143
2021-04-06 19:56msvAssigned Tomsv => bugmaster
2021-04-06 19:56msvStatusacknowledged => feedback
2021-04-06 19:56msvResolutionopen => won't fix
2021-04-07 16:01bugmasterStatusfeedback => closed

Notes
(0099839)
git   
2021-03-30 08:28   
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
(0099847)
msv   
2021-03-30 10:44   
I wonder what is the benefit of using reference instead of pointer in CSLib_Class2d.cxx.
(0099848)
msv   
2021-03-30 10:49   
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.
(0099849)
kgv   
2021-03-30 10:56   
(edited on: 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; }


(0099944)
drazmyslovich   
2021-04-01 13:27   
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.
(0099949)
git   
2021-04-01 14:58   
Branch CR0032265 has been updated forcibly by drazmyslovich.

SHA-1: 7e2f13c0586286b402b379e6d5145a540e4586ac
(0099950)
drazmyslovich   
2021-04-01 15:00   
Please, check the latest commits
(0099952)
msv   
2021-04-01 15:20   
I would discard the changes in CSLib_Class2d.
(0099953)
msv   
2021-04-01 15:21   
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.
(0100119)
git   
2021-04-06 11:20   
Branch CR0032265 has been updated forcibly by drazmyslovich.

SHA-1: 9d135c87b21455b5f2ef41638a2201f2ea645873
(0100120)
drazmyslovich   
2021-04-06 11:21   
Done
(0100129)
git   
2021-04-06 12:21   
Branch CR0032265 has been updated forcibly by msv.

SHA-1: b3100f20d79ed7d1b38ff3b242d33dc7633a87e4
(0100130)
msv   
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/. [^]

(0100131)
msv   
2021-04-06 14:15   
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?
(0100142)
drazmyslovich   
2021-04-06 19:52   
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.
(0100143)
msv   
2021-04-06 19:56   
Dear bugmaster, please close this bug.