MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0032265Community[OCCT] OCCT:Modeling Algorithmspublic2021-03-30 08:262021-04-07 16:01
Reporterdrazmyslovich 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusclosedResolutionwon't fix 
PlatformOSOS Version
Product Version[OCCT] 7.6.0* 
Target Version[OCCT] 7.6.0*Fixed in Version 
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
Attached Files

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

-  Notes
(0099839)
git (administrator)
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 (developer)
2021-03-30 10:44

I wonder what is the benefit of using reference instead of pointer in CSLib_Class2d.cxx.
(0099848)
msv (developer)
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 (developer)
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 (developer)
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 (administrator)
2021-04-01 14:58

Branch CR0032265 has been updated forcibly by drazmyslovich.

SHA-1: 7e2f13c0586286b402b379e6d5145a540e4586ac
(0099950)
drazmyslovich (developer)
2021-04-01 15:00

Please, check the latest commits
(0099952)
msv (developer)
2021-04-01 15:20

I would discard the changes in CSLib_Class2d.
(0099953)
msv (developer)
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 (administrator)
2021-04-06 11:20

Branch CR0032265 has been updated forcibly by drazmyslovich.

SHA-1: 9d135c87b21455b5f2ef41638a2201f2ea645873
(0100120)
drazmyslovich (developer)
2021-04-06 11:21

Done
(0100129)
git (administrator)
2021-04-06 12:21

Branch CR0032265 has been updated forcibly by msv.

SHA-1: b3100f20d79ed7d1b38ff3b242d33dc7633a87e4
(0100130)
msv (developer)
2021-04-06 12:35
edited on: 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 (developer)
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 (developer)
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 (developer)
2021-04-06 19:56

Dear bugmaster, please close this bug.

- 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 View Revisions
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 View Revisions
2021-03-30 10:57 kgv Note Edited: 0099849 View Revisions
2021-03-30 10:58 kgv Note Edited: 0099849 View Revisions
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 View Revisions
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


Copyright © 2000 - 2021 MantisBT Team
Powered by Mantis Bugtracker