MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0028325Community[OCCT] OCCT:Modeling Algorithmspublic2016-12-29 20:252017-12-05 17:09
ReporterIstvan Csanady 
Assigned Tomsv 
PrioritynormalSeveritytweak 
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.4.0*Fixed in Version 
Summary0028325: Unnecessary copying of surface
DescriptionProjLib_ComputeApproxOnPolarSurface unnecessarily copies a surface object, that can lead to significant performance loss in some cases. Please find the attached patch file.
Steps To ReproduceNot needed
TagsNo tags attached.
Test case number
Attached Filesdiff file icon surfacecopy.diff (1,896 bytes) 2016-12-29 20:25
log file icon diffcpu-2017-01-24T1522.log (5,399 bytes) 2017-01-30 10:24

- Relationships

-  Notes
(0063172)
git (administrator)
2017-01-30 10:13

Branch CR28325 has been created by msv.

SHA-1: d2c3d328efbd79697827514c36c064c07ed8cbe8


Detailed log of new commits:

Author: msv
Date: Tue Jan 24 12:29:02 2017 +0300

    0028325: Unnecessary copying of surface
    
    Giving up creation of new adaptor of the surface without real need. Using the available adaptor instead.
(0063173)
msv (developer)
2017-01-30 10:16

Actually, the surface is not copied in the patched code. Just new GeomAdaptor_Surface is created with reduced domain from the surface of another adaptor defined in the upper scope.
(0063174)
msv (developer)
2017-01-30 10:23

I have tested the patch. It brings no regressions.
Here are the results of performance comparison. The patch code figures are on the left, master are on the right. I am giving here the figures only for the tests having difference of more than 10%. The more detailed log with difference 5% and higher is attached.

CPU boolean bcommon_2d: 7.859375 / 9.140625 [-14.02%]
CPU boolean bopcommon_complex C6: 12.5625 / 14.890625 [-15.63%]
CPU boolean boptuc_complex A4: 10.921875 / 12.640625 [-13.60%]
CPU boolean boptuc_complex A6: 12.65625 / 15.734375 [-19.56%]
CPU bugs mesh bug25378_3_3: 26.609375 / 30.671875 [-13.25%]
CPU bugs mesh bug27384_1: 35.078125 / 39.4375 [-11.05%]
CPU bugs modalg_2 bug22109_2: 1.96875 / 2.75 [-28.41%]
CPU bugs modalg_2 bug22109_4: 1.90625 / 2.703125 [-29.48%]
CPU bugs modalg_2 bug22109_5: 1.671875 / 2.53125 [-33.95%]
CPU bugs modalg_6 bug26567: 503.40625 / 566.25 [-11.10%]
CPU bugs moddata_1 bug20391: 2.390625 / 3.59375 [-33.48%]
CPU bugs step bug365_2: 11.96875 / 13.953125 [-14.22%]
CPU heal surface_to_revolution_standard: 17.203125 / 19.453125 [-11.57%]
CPU mesh advanced_incmesh_parallel B5: 19.78125 / 24.171875 [-18.16%]
CPU offset with_intersect_20: 12.296875 / 13.703125 [-10.26%]
CPU perf fclasses: 5.53125 / 6.40625 [-13.66%]
CPU perf moddata bug623: 4.03125 / 5.953125 [-32.28%]
CPU thrusection specific: 3.765625 / 4.578125 [-17.75%]
CPU v3d edge: 4.421875 / 4.984375 [-11.29%]

CPU bugs modalg_6 bug26188: 23.546875 / 7.09375 [+231.94%]
CPU bugs modalg_6 bug26196: 22.109375 / 6.015625 [+267.53%]
CPU de step_3 A6: 22.609375 / 11.015625 [+105.25%]
CPU de step_3 A7: 22.8125 / 10.96875 [+107.98%]
CPU de step_3 F2: 656.15625 / 181.671875 [+261.18%]
CPU de step_3: 1572.90625 / 1093.390625 [+43.86%]
CPU perf modalg bug27085_2: 5.5625 / 4.390625 [+26.69%]
CPU thrusection not_solids: 6.8125 / 5.703125 [+19.45%]
CPU v3d glsl: 6.859375 / 6.21875 [+10.30%]
CPU v3d raytrace bug26617: 4.171875 / 2.6875 [+55.23%]
CPU v3d raytrace sample_cube_clamp: 8.46875 / 6.484375 [+30.60%]

Total CPU difference: 21008.796875 / 21027.125 [-0.09%]
(0063176)
msv (developer)
2017-01-30 10:58

We can see that the patch brings as performance improvements (-9% in average for the full log) as degradations (+64% in average).

I have analyzed what happens and found the causes of both improvement and degradation.

Let's consider the test:
CPU perf moddata bug623: 4.03125 / 5.953125 [-32.28%]

In this case the master version spends much time to rebuild bspline cache. In the patched version the cache is not rebuilt, because the same adaptor is used. This is the cause of performance improvement.

Now let's consider the test:
CPU bugs modalg_6 bug26188: 23.546875 / 7.09375 [+231.94%]

In this case in the master version the most number of times when a point is projected on the surface, the local search is used (the class Extrema_GenLocateExtPS). And in the patched version significant number of times local search is failed and global search is used (Extrema_ExtPS). Below are some figures from Intel amplifier. Number of spent seconds and approximate number of calls is shown:
patched:
Extrema_GenLocateExtPS 2.8s 188000
Extrema_ExtPS           11s 13000
ref:
Extrema_GenLocateExtPS   3s 249000
Extrema_ExtPS          0.2s 0
(0063177)
msv (developer)
2017-01-30 11:03

Summarizing and taking into account that total CPU difference is very small (-0.09%), I can say that the patch (as is) is not worth to be taken.

However, I think we could do something to improve. If we could create a copy of adaptor with preserving bspline cache and with new parameters range it could speed up execution without performance degradation.
(0063178)
msv (developer)
2017-01-30 11:23

Dear Istvan, could you provide your test case? We would like to analyze it before taking the final decision.
(0063194)
Istvan Csanady (developer)
2017-01-30 15:05

I believe the test files were related to the issue 0028274 and I used the same or similar geometries to test it.
(0063279)
msv (developer)
2017-02-01 10:59

Using the test files attached to 0028274, I did not found any change in execution time between master version and this patch.

So, I would keep idea expressed in the post above (0028325:0063177).

- Issue History
Date Modified Username Field Change
2016-12-29 20:25 Istvan Csanady New Issue
2016-12-29 20:25 Istvan Csanady Assigned To => msv
2016-12-29 20:25 Istvan Csanady File Added: surfacecopy.diff
2017-01-24 12:25 msv Summary Unecessary copying of surface => Unnecessary copying of surface
2017-01-30 10:12 msv Status new => assigned
2017-01-30 10:13 git Note Added: 0063172
2017-01-30 10:14 msv Status assigned => resolved
2017-01-30 10:14 msv Steps to Reproduce Updated View Revisions
2017-01-30 10:16 msv Note Added: 0063173
2017-01-30 10:23 msv Note Added: 0063174
2017-01-30 10:24 msv File Added: diffcpu-2017-01-24T1522.log
2017-01-30 10:58 msv Note Added: 0063176
2017-01-30 11:03 msv Note Added: 0063177
2017-01-30 11:23 msv Note Added: 0063178
2017-01-30 11:23 msv Assigned To msv => Istvan Csanady
2017-01-30 11:23 msv Status resolved => feedback
2017-01-30 15:05 Istvan Csanady Note Added: 0063194
2017-02-01 10:59 msv Note Added: 0063279
2017-02-01 11:01 msv Assigned To Istvan Csanady => msv
2017-02-01 11:01 msv Status feedback => assigned
2017-07-21 11:22 msv Target Version 7.2.0 => 7.3.0
2017-12-05 17:09 msv Target Version 7.3.0 => 7.4.0*


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker