MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0028636Open CASCADE[OCCT] OCCT:Codingpublic2017-04-06 11:192017-04-14 14:28
Reporternbv 
Assigned Tobugmaster 
PrioritynormalSeveritytweak 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 7.1.0 
Target Version[OCCT] 7.2.0*Fixed in Version 
Summary0028636: Optimisation of gp_* classes in order to avoid unnecesary calling gp_Dir* constructors with normalisation
DescriptionMany constructors of gp_* classes contains fields as

gp_Dir v


and v is initialized by the value (1, 0). While such initialization, copy-constructor is used, which computes expression such as

sqrt(1*1+0*0)


and spent much time.

The main idea of optimization is to avoid using constructors with normalization for trivial cases.

P.S.

This problem has been observed while fixing performance degradation in the issue #28211. Though this was very little hot spot (about 1-3%) it was decided to correct such obvious issue.
Steps To ReproduceNew test case is not required.
TagsNo tags attached.
Test case numberNot needed
Attached Files

- Relationships

-  Notes
(0064981)
git (administrator)
2017-04-06 15:28

Branch CR28636 has been created by msv.

SHA-1: 75da7906240cfc6663a455a38912cb4885cc6898


Detailed log of new commits:

Author: msv
Date: Thu Apr 6 15:28:26 2017 +0300

    0028636: Optimisation of gp_* classes in order to avoid unnecesary calling gp_Dir* constructors with normalisation
    
    The improvement is concluded in replacement of calls to gp_Dir2d(1,0) with calls to gp_Dir2d(void), and calls to gp_Dir2d(0,1) to gp_Dir2d(gp::DY2d()). Thus we avoid sqrt() that is called from within gp_Dir2d(double,double). The same is with direction in 3D space.
(0064982)
msv (developer)
2017-04-06 15:29

Dear Eugeny, please review the fix.
(0064984)
abv (manager)
2017-04-06 15:42

Mikhail, please provide results of your performance measurements showing the positive effect of the change.
(0064988)
emv (developer)
2017-04-06 15:49

Reviewed.
(0064994)
msv (developer)
2017-04-06 16:15

Test case is to be created.
(0065056)
git (administrator)
2017-04-07 19:44

Branch CR28636_1 has been created by msv.

SHA-1: 7c022c420d2bb05933c258b00b07fcfefde77b36


Detailed log of new commits:

Author: msv
Date: Thu Apr 6 15:28:26 2017 +0300

    0028636: Optimisation of gp_* classes in order to avoid unnecesary calling gp_Dir* constructors with normalisation
    
    The improvement is concluded in replacement of calls to gp_Dir2d(1,0) with calls to gp_Dir2d(void). Thus we avoid sqrt() that is called from within gp_Dir2d(double,double). The same is with direction in 3D space.
    
    Test cases have been created to check the improvement.
(0065057)
git (administrator)
2017-04-07 20:01

Branch CR28636_1 has been updated forcibly by msv.

SHA-1: e57e1e343ba81e3975fb089716ebc61016988d89
(0065058)
msv (developer)
2017-04-07 20:08
edited on: 2017-04-10 10:13

I have created synthetic test case to call default constructor in a loop. The code is the following:

  Standard_Integer nbRep = 1000000;
  Standard_Real dummy = 0.;
  for (; nbRep > 0; nbRep--)
  {
    for (Standard_Integer n = 100; n > 0; n--)
    {
      gp_Ax2d anAx1, anAx2;
      anAx2.Mirror(gp_Pnt2d(1., 1.));
      dummy += anAx1.Direction().XY() * anAx2.Direction().XY();
    }
  }


The following is comparison of CPU time spent in the fix vs master:

0.46875/1.203125

Dear Kirill, please review.

(0065059)
kgv (developer)
2017-04-07 20:25
edited on: 2017-04-09 21:35

-inline gp_Ax3::gp_Ax3() : vydir(0.,1.,0.), vxdir(1.,0.,0.)
+inline gp_Ax3::gp_Ax3()
+  :
+  vydir(0.,1.,0.)
+  // vxdir(1.,0.,0.) use default ctor of gp_Dir, as it creates the same dir(1,0,0)


Both vydir and vxdir are gp_Dir, but patch modifies initialization only of vxdir - why?
Does initialization of vydir(0.,1.,0.) somehow avoid normalization while vxdir(1.,0.,0.) does not?

Patch looks incomplete if it is intended to improve performance.
There might be many other options for making gp_Dir/gp_Dir2d constructors for standard directions without normalization:
- Extra bool flag to prevent normalization.
  Can be useful also in context when normalization is performed in advance (especially taking into account gp_Dir throws exception on division by zero).
- Constructor with constexpr looks like a good example what constexpr is designed for.
- Constructor taking enum with standard directions.
- Template constructor/method as alternative to constexpr.

(0065071)
git (administrator)
2017-04-10 10:18

Branch CR28636_2 has been created by msv.

SHA-1: 9c1a3136a17e3647253a6ee625ed2279428c7031


Detailed log of new commits:

Author: msv
Date: Thu Apr 6 15:28:26 2017 +0300

    0028636: Optimisation of gp_* classes in order to avoid unnecesary calling gp_Dir* constructors with normalisation
    
    The improvement is concluded in replacement of calls to gp_Dir2d(1,0) with calls to gp_Dir2d(void). Thus we avoid sqrt() that is called from within gp_Dir2d(double,double). The same is with direction in 3D space.
(0065073)
msv (developer)
2017-04-10 10:20

Please review again.
(0065074)
kgv (developer)
2017-04-10 10:23

Please test the patch.
(0065127)
apv (tester)
2017-04-11 12:53

Dear BugMaster,

Branch CR28636_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 9c1a3136a17e3647253a6ee625ed2279428c7031

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1198

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 91799412 / 91395470 [+0.44%]
Total CPU difference: 19636.610000000222 / 19613.34000000024 [+0.12%]

Testing on Windows:
Total MEMORY difference: 57855660 / 57855806 [-0.00%]
Total CPU difference: 18299.525703898533 / 18063.40259029849 [+1.31%]

- Related Changesets
occt: master 0bd575a7
Timestamp: 2017-04-06 12:28:26
Author: msv
Committer: bugmaster
Details ] Diff ]
0028636: Optimisation of gp_* classes in order to avoid unnecesary calling gp_Dir* constructors with normalisation

The improvement is concluded in replacement of calls to gp_Dir2d(1,0) with calls to gp_Dir2d(void).
Thus we avoid sqrt() that is called from within gp_Dir2d(double,double). The same is with direction in 3D space.
mod - src/gp/gp_Ax3.lxx Diff ] File ]
mod - src/gp/gp_Ax2d.lxx Diff ] File ]
mod - src/gp/gp_Ax2.lxx Diff ] File ]
mod - src/gp/gp_Ax22d.lxx Diff ] File ]

- Issue History
Date Modified Username Field Change
2017-04-06 11:19 nbv New Issue
2017-04-06 11:19 nbv Assigned To => msv
2017-04-06 11:20 nbv Description Updated View Revisions
2017-04-06 11:21 nbv Relationship added related to 0028211
2017-04-06 12:32 msv Status new => assigned
2017-04-06 12:32 msv Product Version => 7.1.0
2017-04-06 15:28 git Note Added: 0064981
2017-04-06 15:29 msv Note Added: 0064982
2017-04-06 15:29 msv Assigned To msv => emv
2017-04-06 15:29 msv Status assigned => resolved
2017-04-06 15:42 abv Note Added: 0064984
2017-04-06 15:49 emv Note Added: 0064988
2017-04-06 15:49 emv Assigned To emv => bugmaster
2017-04-06 15:49 emv Status resolved => reviewed
2017-04-06 16:14 mkv Assigned To bugmaster => apv
2017-04-06 16:15 msv Note Added: 0064994
2017-04-06 16:15 msv Assigned To apv => msv
2017-04-06 16:15 msv Status reviewed => assigned
2017-04-07 19:44 git Note Added: 0065056
2017-04-07 20:01 git Note Added: 0065057
2017-04-07 20:08 msv Note Added: 0065058
2017-04-07 20:08 msv Assigned To msv => kgv
2017-04-07 20:08 msv Status assigned => resolved
2017-04-07 20:25 kgv Note Added: 0065059
2017-04-07 20:25 kgv Assigned To kgv => msv
2017-04-07 20:25 kgv Status resolved => assigned
2017-04-07 20:27 kgv Note Edited: 0065059 View Revisions
2017-04-07 20:31 kgv Note Edited: 0065059 View Revisions
2017-04-09 21:35 kgv Note Edited: 0065059 View Revisions
2017-04-10 10:06 msv Description Updated View Revisions
2017-04-10 10:13 msv Note Edited: 0065058 View Revisions
2017-04-10 10:18 git Note Added: 0065071
2017-04-10 10:20 msv Note Added: 0065073
2017-04-10 10:20 msv Assigned To msv => kgv
2017-04-10 10:20 msv Status assigned => resolved
2017-04-10 10:23 kgv Note Added: 0065074
2017-04-10 10:23 kgv Assigned To kgv => bugmaster
2017-04-10 10:23 kgv Severity minor => tweak
2017-04-10 10:23 kgv Status resolved => reviewed
2017-04-10 10:48 apv Assigned To bugmaster => apv
2017-04-11 12:06 apv Test case number => Not needed
2017-04-11 12:53 apv Note Added: 0065127
2017-04-11 12:53 apv Assigned To apv => bugmaster
2017-04-11 12:53 apv Status reviewed => tested
2017-04-14 14:28 bugmaster Changeset attached => occt master 0bd575a7
2017-04-14 14:28 bugmaster Status tested => verified
2017-04-14 14:28 bugmaster Resolution open => fixed


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker