View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0032265 | Community | OCCT:Modeling Algorithms | public | 2021-03-30 08:26 | 2021-12-13 14:56 |
| Reporter | drazmyslovich | Assigned To | bugmaster | ||
| Priority | normal | Severity | feature | ||
| Status | closed | Resolution | won't fix | ||
| Product Version | 7.6.0 | ||||
| Target Version | 7.6.0 | ||||
| Summary | 0032265: Modeling Algorithms - some trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection | ||||
| Description | 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 | ||||
| Steps To Reproduce | N/A | ||||
| Tags | No tags attached. | ||||
| Test case number | |||||
|
|
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 |
|
|
I wonder what is the benefit of using reference instead of pointer in CSLib_Class2d.cxx. |
|
|
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. |
|
|
> 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; }
|
|
|
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. |
|
|
Branch CR0032265 has been updated forcibly by drazmyslovich. SHA-1: 7e2f13c0586286b402b379e6d5145a540e4586ac |
|
|
Please, check the latest commits |
|
|
I would discard the changes in CSLib_Class2d. |
|
|
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. |
|
|
Branch CR0032265 has been updated forcibly by drazmyslovich. SHA-1: 9d135c87b21455b5f2ef41638a2201f2ea645873 |
|
|
Done |
|
|
Branch CR0032265 has been updated forcibly by msv. SHA-1: b3100f20d79ed7d1b38ff3b242d33dc7633a87e4 |
|
|
Rebased on master, corrected the title of the commit message. Testing http://jenkins-test-12.nnov.opencascade.com/view/CR0032265-master-msv/view/COMPARE/. |
|
|
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? |
|
|
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. |
|
|
Dear bugmaster, please close this bug. |
|
|
Branch CR0032265 has been deleted by kgv. SHA-1: b3100f20d79ed7d1b38ff3b242d33dc7633a87e4 |
| 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 |
|
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 |
|
Note Added: 0099847 | |
| 2021-03-30 10:49 |
|
Note Added: 0099848 | |
| 2021-03-30 10:50 |
|
Assigned To | msv => drazmyslovich |
| 2021-03-30 10:50 |
|
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 |
|
Note Added: 0099952 | |
| 2021-04-01 15:21 |
|
Note Added: 0099953 | |
| 2021-04-01 15:22 |
|
Assigned To | msv => drazmyslovich |
| 2021-04-01 15:22 |
|
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 |
|
Note Added: 0100130 | |
| 2021-04-06 12:35 |
|
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 |
|
Note Added: 0100131 | |
| 2021-04-06 15:03 |
|
Assigned To | msv => drazmyslovich |
| 2021-04-06 15:03 |
|
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 |
|
Note Added: 0100143 | |
| 2021-04-06 19:56 |
|
Assigned To | msv => bugmaster |
| 2021-04-06 19:56 |
|
Status | acknowledged => feedback |
| 2021-04-06 19:56 |
|
Resolution | open => won't fix |
| 2021-04-07 16:01 | bugmaster | Status | feedback => closed |
| 2021-12-13 14:56 | git | Note Added: 0105847 |
