View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0024044 | Community | OCCT:Foundation Classes | public | 2013-06-25 21:38 | 2019-03-12 06:55 |
| Reporter | Roman Lygin | Assigned To | bugmaster | ||
| Priority | normal | Severity | integration request | ||
| Status | closed | Resolution | fixed | ||
| Platform | A | OS | L | ||
| Product Version | 6.6.0 | ||||
| Target Version | 6.7.0 | Fixed in Version | 6.7.0 | ||
| Summary | 0024044: Performance improvements: Foundation Classes (math) | ||||
| Description | Continuation of series started in 0024042 *math - use of automatic allocation (on stack) to minimize heap allocation. Size constants are selected such to avoid stack overflow yet maximizing chances to favor stack allocation. | ||||
| Steps To Reproduce | N/A | ||||
| Tags | No tags attached. | ||||
| Test case number | Not needed | ||||
| related to | 0024042 | closed | apn | Community | Performance improvements: Foundation Classes |
| parent of | 0030552 | assigned | Community | Foundation Classes - Stack overflow due to math_SingleTab static array size | |
| related to | 0025746 | closed | bugmaster | Open CASCADE | Excessive memory use in math_Matrix |
| child of | 0024045 | closed | bugmaster | Community | Invalid test case for matrix-vector multiplication |
| Not all the children of this issue are yet resolved or closed. | |||||
|
|
The fix pushed to the repository |
|
|
Defining size of array independently in CDL and GXX is potentially dangerous. I suggest that definition of constants N, M, and MN in GXX should be done via sizeof() of relevant class fields. The same comment as to 0024042: if you have some numbers of performance increase this improvement gives in your case, please share. |
|
|
For performance gain - please refer to 0024042. Regarding sizeof(), do you see any better implementation than replacing each occurrence with something like (sizeof(Buf) / sizeof(Buf[0])). Though I share the desire to avoid risk of discrepancy this change would undermine code clarity, so I am not sure this is better than an explicit comment in the code. But I will change to whatever you prefer. |
|
|
> replacing each occurrence with something like (sizeof(Buf) / sizeof(Buf[0])) Yes, this is what I had in mind. Please check the amended version in CR24044_1 |
|
|
The amendment is fine. Thanks for making it and sorry for the delay - I thought you expected me to *make* the amendment not just to *review* it. |
|
|
Dear BugMaster, Branch CR24044_1 (and products from GIT master) was compiled on Linux and Windows platforms and tested. SHA-1: fdbd14f0acc8325642ec1fd0553de0611f7fd57f Number of compiler warnings: occt component : Linux: 2 (2 on master) Windows: 1767 (1767 on master) products component : Linux: 0 (0 on master) Windows: 288 (288 on master) Regressions: bugs moddata_2 bug524 http://occt-tests/CR24044-1-master-occt/Debian60-64/summary.html Improvements: No improvements Testing cases: Not needed Testing on Linux: Total MEMORY difference: 366693152 / 365661156 Total CPU difference: 43746.34000000115 / 44445.42000000096 Testing on Windows: Total MEMORY difference: 427509152 / 426010604 Total CPU difference: 43613.15625 / 31424.5 There are not differences in images found by testdiff. |
|
|
Dear BugMaster, The fix should be integrated along with 0024045 |
|
occt: master b28e7cc6 2013-06-25 17:42:25 Committer: bugmaster Details Diff |
0024044: Performance improvements: Foundation Classes (math) |
Affected Issues 0024044 |
|
| mod - src/math/math_DoubleTab.cdl | Diff File | ||
| mod - src/math/math_DoubleTab.gxx | Diff File | ||
| mod - src/math/math_SingleTab.cdl | Diff File | ||
| mod - src/math/math_SingleTab.gxx | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-06-25 21:38 | Roman Lygin | New Issue | |
| 2013-06-25 21:38 | Roman Lygin | Assigned To | => abv |
| 2013-06-25 21:43 | Roman Lygin | Note Added: 0024866 | |
| 2013-06-25 21:43 | Roman Lygin | Status | new => resolved |
| 2013-06-25 21:49 | Roman Lygin | Relationship added | child of 0024045 |
| 2013-06-25 22:00 | Roman Lygin | Relationship added | related to 0024042 |
| 2013-06-26 06:59 |
|
Note Added: 0024869 | |
| 2013-06-26 06:59 |
|
Assigned To | abv => Roman Lygin |
| 2013-06-26 06:59 |
|
Status | resolved => assigned |
| 2013-06-27 22:41 | Roman Lygin | Note Added: 0024907 | |
| 2013-06-27 22:41 | Roman Lygin | Assigned To | Roman Lygin => abv |
| 2013-06-27 22:41 | Roman Lygin | Status | assigned => feedback |
| 2013-07-12 07:13 |
|
Note Added: 0025041 | |
| 2013-07-12 07:13 |
|
Assigned To | abv => Roman Lygin |
| 2013-07-12 07:13 |
|
Status | feedback => resolved |
| 2013-07-17 00:19 | Roman Lygin | Note Added: 0025090 | |
| 2013-07-17 00:19 | Roman Lygin | Assigned To | Roman Lygin => abv |
| 2013-07-17 00:19 | Roman Lygin | Status | resolved => reviewed |
| 2013-07-18 16:21 | apn | Note Added: 0025114 | |
| 2013-07-18 16:21 | apn | Test case number | => Not needed |
| 2013-07-18 16:21 | apn | Assigned To | abv => Roman Lygin |
| 2013-07-18 16:21 | apn | Status | reviewed => assigned |
| 2013-07-18 16:21 | apn | Target Version | => 6.7.0 |
| 2013-08-15 15:04 |
|
Note Added: 0025320 | |
| 2013-08-15 15:06 |
|
Assigned To | Roman Lygin => bugmaster |
| 2013-08-15 15:06 |
|
Status | assigned => resolved |
| 2013-08-15 15:06 |
|
Status | resolved => reviewed |
| 2013-08-15 15:06 |
|
Status | reviewed => tested |
| 2013-08-17 12:50 | bugmaster | Changeset attached | => occt master b28e7cc6 |
| 2013-08-17 12:50 | bugmaster | Status | tested => verified |
| 2013-08-17 12:50 | bugmaster | Resolution | open => fixed |
| 2013-12-19 13:52 | bugmaster | Status | verified => closed |
| 2013-12-19 13:56 | bugmaster | Fixed in Version | => 6.7.0 |
| 2015-03-09 19:36 |
|
Relationship added | related to 0025746 |
| 2019-03-12 06:55 | kgv | Relationship added | parent of 0030552 |
