View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0028635 | Open CASCADE | OCCT:Coding | public | 2017-04-06 10:21 | 2017-05-29 16:11 |
Reporter | Assigned To | bugmaster | |||
Priority | high | Severity | minor | ||
Status | closed | Resolution | won't fix | ||
Product Version | 7.1.0 | ||||
Target Version | 7.2.0 | ||||
Summary | 0028635: Optimization of method "load" for Geom2dAdaptor* and GeomAdaptor* classes | ||||
Description | Current version of load(...) methods uses many code fragment such asif(...) { ... } else if(...) { ... } ... else { ... } Processing of such structure requires much time. It is recommended for speeding up performance to use of map of pointers to respective function and to call necessary function contains in this map (without "if-else" structure). P.S. This problem has been observed while fixing performance degradation in the issue #28211. | ||||
Steps To Reproduce | New test case is not required. | ||||
Tags | No tags attached. | ||||
Test case number | |||||
|
Branch CR28635 has been created by nbv. SHA-1: d3de0bb31b88b6343e2472e97c46dc08c4ace6b7 Detailed log of new commits: Author: nbv Date: Thu Apr 6 12:08:47 2017 +0300 0028635: Optimization of method "load" for Geom2dAdaptor* and GeomAdaptor* classes Following classes have been optimized: Geom2dAdaptor_Curve, GeomAdaptor_Curve, GeomAdaptor_Surface. |
|
Dear Mikhail, Please review CR28635 branch. |
|
Branch CR28635 has been updated forcibly by nbv. SHA-1: d1e7d9cdb4c3f9be9a22cc5762538ef34bd5c2b9 |
|
Commit message has been updated. |
|
- Make one-line methods inline. - Move typedef and method ProcessType out from fields area. - Revise the work with mutex (change while to double if). - Use method Seek() of the map. |
|
Branch CR28635_1 has been created by nbv. SHA-1: 35e6ec3c1e2d9957bbe8d17d814fe3e0cb1d3915 Detailed log of new commits: Author: nbv Date: Thu Apr 6 12:08:47 2017 +0300 0028635: Optimization of method "load" for Geom2dAdaptor* and GeomAdaptor* classes Following classes have been optimized: Geom2dAdaptor_Curve, GeomAdaptor_Curve, GeomAdaptor_Surface. The main idea of the optimization is replacement structures as "if - else if - ... - else" with direct execution of necessary method. |
|
Branch CR28635_1 has been updated forcibly by nbv. SHA-1: f4189951b51101087497939fbfa6005fcd4fee62 |
|
Dear Mikhail, Please review the current state of CR28635_1 branch. My results of optimization (average mean is taken from set of five elements): OCC28635_1 (GeomAdaptor_Curve): MASTER: 1.541666667 seconds FIX: 1.453125 seconds DIFF: -5.743% OCC28635_2 (Geom2dAdaptor_Curve): MASTER: 4.317708333 seconds FIX: 4.166666667 seconds DIFF: -3.498% OCC28635_3 (GeomAdaptor_Surface): MASTER: 2.729166667 seconds FIX: 2.619791667 seconds DIFF: -4.008% |
|
In the test function I see the mistake: you call in the loop the method Load with the same curve. In this case the updated inside Load() code will not be run. Also, please correct other things as we agreed. |
|
Branch CR28635_2 has been created by nbv. SHA-1: 7d687bc65636455ad96f382107318992e4fd6fb1 Detailed log of new commits: Author: nbv Date: Thu Apr 6 12:08:47 2017 +0300 0028635: Optimization of method "load" for Geom2dAdaptor* and GeomAdaptor* classes Following classes have been optimized: Geom2dAdaptor_Curve, GeomAdaptor_Curve, GeomAdaptor_Surface. The main idea of the optimization is replacement structures as "if - else if - ... - else" with direct execution of necessary method. |
|
New results of optimization on CR28635_2 (average mean is taken from set of five elements): OCC28635_1 (GeomAdaptor_Curve): MASTER: 1.604166667 seconds FIX: 1.588541667 seconds DIFF: -0.974% OCC28635_2 (Geom2dAdaptor_Curve): MASTER: 1.526041667 seconds FIX: 1.380208333 seconds DIFF: -9.556% OCC28635_3 (GeomAdaptor_Surface): MASTER: 3.28125 seconds FIX: 3.213541667 seconds DIFF: -2.063% |
|
Dear Mikhail, Please review CR28635_2 branch and decide about necessity of the integration (basing on the message 0028635:0066575) |
|
+ //! Coordinates call process* methods (from "load" method) + //! according to the type of given curve. + Geom2dAdaptor_Curve::WhichTypePtr ProcessType(const Handle(Standard_Type)& theType); Adding a method converting Dynamic type into enumeration looks a little bit awkward... Maybe it is better adding a virtual method to Geom_Curve and other interface classes directly returning enum? |
|
It seems to me this optimization did not meet our hopes. The gain got by this improvement is too little to take into account. So, I propose to close this bug without any integrations. |
|
Branch CR28635 has been deleted by kgv. SHA-1: d1e7d9cdb4c3f9be9a22cc5762538ef34bd5c2b9 |
|
Branch CR28635_1 has been deleted by kgv. SHA-1: f4189951b51101087497939fbfa6005fcd4fee62 |
|
Branch CR28635_2 has been deleted by kgv. SHA-1: 7d687bc65636455ad96f382107318992e4fd6fb1 |
Date Modified | Username | Field | Change |
---|---|---|---|
2017-04-06 10:21 |
|
New Issue | |
2017-04-06 10:21 |
|
Assigned To | => msv |
2017-04-06 11:10 |
|
Assigned To | msv => nbv |
2017-04-06 11:10 |
|
Status | new => assigned |
2017-04-06 11:10 |
|
Product Version | => 7.1.0 |
2017-04-06 11:10 |
|
Priority | normal => high |
2017-04-06 11:53 |
|
Summary | Optimisation of method "load" for Geom2dAdaptor* and GeomAdaptor* classes => Optimization of method "load" for Geom2dAdaptor* and GeomAdaptor* classes |
2017-04-06 14:14 | git | Note Added: 0064979 | |
2017-04-06 14:15 |
|
Note Added: 0064980 | |
2017-04-06 14:15 |
|
Assigned To | nbv => msv |
2017-04-06 14:15 |
|
Status | assigned => resolved |
2017-04-06 16:04 | git | Note Added: 0064992 | |
2017-04-06 16:04 |
|
Note Added: 0064993 | |
2017-04-06 16:40 |
|
Note Added: 0064996 | |
2017-04-06 16:40 |
|
Assigned To | msv => nbv |
2017-04-06 16:40 |
|
Status | resolved => assigned |
2017-04-06 17:40 | git | Note Added: 0065004 | |
2017-04-07 11:14 | git | Note Added: 0065009 | |
2017-04-07 11:22 |
|
Note Added: 0065010 | |
2017-04-07 11:22 |
|
Assigned To | nbv => msv |
2017-04-07 11:22 |
|
Status | assigned => resolved |
2017-04-07 17:53 |
|
Note Added: 0065046 | |
2017-04-07 17:53 |
|
Status | resolved => assigned |
2017-04-07 17:53 |
|
Assigned To | msv => nbv |
2017-05-25 13:07 | git | Note Added: 0066573 | |
2017-05-25 13:12 |
|
Note Added: 0066575 | |
2017-05-25 13:14 |
|
Note Added: 0066577 | |
2017-05-25 13:14 |
|
Assigned To | nbv => msv |
2017-05-25 13:14 |
|
Status | assigned => feedback |
2017-05-25 14:25 | kgv | Note Added: 0066585 | |
2017-05-25 14:26 | kgv | Note Edited: 0066585 | |
2017-05-25 14:26 | kgv | Note Edited: 0066585 | |
2017-05-25 14:59 |
|
Note Added: 0066591 | |
2017-05-25 14:59 |
|
Assigned To | msv => bugmaster |
2017-05-25 14:59 |
|
Resolution | open => won't fix |
2017-05-29 10:27 | bugmaster | Status | feedback => closed |
2017-05-29 16:11 | git | Note Added: 0066787 | |
2017-05-29 16:11 | git | Note Added: 0066788 | |
2017-05-29 16:11 | git | Note Added: 0066789 |