View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0029830 | Open CASCADE | OCCT:Data Exchange | public | 2018-06-01 09:39 | 2018-07-21 20:02 |
Reporter | Assigned To | bugmaster | |||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Product Version | 7.2.0 | ||||
Target Version | 7.4.0 | Fixed in Version | 7.4.0 | ||
Summary | 0029830: Data Exchange, STEPCAFControl_Reader poor performance - quadratic dependence | ||||
Description | This class performs very slow while reading models with big number of parts. It has quadratic dependence of time on the number of parts during setting colors to shapes. It iterates on all read shapes, and calls the method SetColor of XCAFDoc_ColorTool, which in its turn searches for the shape in the document by brute force iterations. It is needed to use the mechanism of searching shapes by the data map (the method SearchUsingMap from XCAFDoc_ShapeTool), how it is done in the class IGESCAFControl_Reader. The sample files to reproduce the issue are very big and cannot be loaded here. They can be found in the local network storage //master/projects/HybridProduct/data_nobackup/. | ||||
Steps To Reproduce | chrono h reset start ReadStep d ALL_AREA01.step chrono h stop show #CPU user time: 131.40625 seconds chrono h reset start ReadStep d ALL.step chrono h stop show #CPU user time: 1338.671875 seconds | ||||
Tags | No tags attached. | ||||
Test case number | perf/de/bug29830_1, perf/de/bug29830_2, perf/de/bug29830_3 | ||||
|
Branch CR29830 has been created by msv. SHA-1: fb04c5f4b4941b14df79f2724cebd989c90c2161 Detailed log of new commits: Author: msv Date: Fri Jun 1 14:38:39 2018 +0300 0029830: STEPCAFControl_Reader poor performance - quadratic dependence The issue has been solved by using the call to SearchUsingMap() in order to quickly find the label of the shape. |
|
The fix allows speeding up the reading as follows: ALL_AREA01.step: 59 vs 131 s ALL.step: 325 vs 1338 s |
|
Please review the fix. |
|
Test results http://jenkins-test-11.nnov.opencascade.com:8080/view/CR29830-master-msv/view/COMPARE/. |
|
No remarks, please proceed |
|
Combination - OCCT branch : CR29830 SHA - fb04c5f4b4941b14df79f2724cebd989c90c2161 Products branch : master SHA - 69b1ea63c0a20d2e45aff1ec3b661e6b0ce68eac was compiled on Linux, MacOS and Windows platforms and tested in optimize mode. Number of compiler warnings: No new/fixed warnings Regressions/Differences/Improvements: No regressions/differences CPU differences: Debian70-64: OCCT Total CPU difference: 17074.939999999857 / 17011.039999999866 [+0.38%] Products Total CPU difference: 7413.420000000049 / 7351.93000000003 [+0.84%] Windows-64-VC10: OCCT Total CPU difference: 16749.12536549862 / 16821.806231398525 [-0.43%] Products Total CPU difference: 8081.772205899864 / 8072.068943699895 [+0.12%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
Please ask test case from MSV |
|
Branch CR29830 has been updated by msv. SHA-1: 7500ee44537d4e68b75d21fe3168d7e3dbfc9111 Detailed log of new commits: Author: msv Date: Wed Jun 6 12:19:35 2018 +0300 Make it possible to use ReadStep and WriteStep to read/write from the session without accessing the disk file. |
|
Branch CR29830 has been updated forcibly by msv. SHA-1: 243f45cfe9b50124722c096af8b46ca207654089 |
|
Branch CR29830 has been updated forcibly by msv. SHA-1: 6f17ba381b3675d53c140ba14b42bee2f8531df8 |
|
Branch CR29830 has been updated forcibly by msv. SHA-1: bc5a0072905ebec8d7930d6dfccfb9d1db1d4b66 |
|
Branch CR29830 has been updated forcibly by msv. SHA-1: ea171d917a43b2c3500ed27421c9acc9eafcbb57 |
|
Branch CR29830_1 has been created by msv. SHA-1: 92ad3ea161efe1b64a2de662515a53256cdecb25 Detailed log of new commits: Author: msv Date: Fri Jun 1 14:38:39 2018 +0300 0029830: STEPCAFControl_Reader poor performance - quadratic dependence Various performance improvements have been made in the algorithms of STEP read/write: - Using the call to XCAFDoc_ShapeTool::SearchUsingMap() in STEPCAFControl_Reader::ReadColors() in order to quickly find the label of the shape. - Place the label of a component to the map in order to avoid iteration on all components to find a label of a component shape. It is done both in XCAFDoc_ShapeTool and in STEPCAFControl_Reader. - Using the map ShapeLabelMap in the method STEPCAFControl_Reader::FindInstance() to avoid iteration on components. - Move invariant FindEntities() out of the loop in the method getStyledItem in STEPCAFControl/STEPCAFControl_Writer.cxx. - Small fixes connected with elimination of excess copying of handles, calls of handle DownCasts and so on. - Add pointer to the end of binders chain to the Transfer_Binder class in order to speed up adding a binder to the chain. Remove stack overflow during destruction of STEP model with long chains of Transfer_Binder. Make it possible to use the Draw commands ReadStep and WriteStep to read/write from the session without accessing the disk file (use '.' for the file name). Add performance test cases for STEP reading/writing. |
|
Dear Pavel, please review CR29830_1. The tests are being computed now in CR29830-master-msv. |
|
I have found the new warning on Debian with this patch: Transfer_ProcessForFinder.hxx:250, GNU C Compiler 4 (gcc), Priority: Normal dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] |
|
Branch CR29830_1 has been updated by msv. SHA-1: 842140f9ad8c60fc5469dc5bad3f0857f675cec1 Detailed log of new commits: Author: msv Date: Sat Jun 9 18:47:11 2018 +0300 #Correct warning |
|
Tests have been restarted. |
|
Branch CR29830_1 has been updated forcibly by msv. SHA-1: c77684b867567a1f3b655ccd2ced28c9d2fde8bb |
|
Branch CR29830_1 has been updated forcibly by msv. SHA-1: 0419c5d0afec08783099a72363e9e607d643a96e |
|
Tests have been passed OK. Please review the patch. |
|
I have some doubts regarding change in STEPCAFControl_Reader::FindInstance() -- have you checked that everything works fine for STEP files with external references? How much change in STEPControl_ActorRead::TransferEntity() improves performance? Using C casts is less safe than using downcasts... |
|
The associations of shapes from external references to labels are present in the same map ShapeLabelMap. They are put there in the method AddShape. I have checked reading the following STEP models with ext refs kindly provided by SKL: s1-c5-214 s1-tu-203 s1_pe_214 Everything was OK, no parts have been lost comparing to master. Regarding C casts in STEPControl_ActorRead::TransferEntity(), I found this hotspot using Intel Amplifier. It was about 15% (if I correct remember) of the function time. Changing dynamic cast to static cast eliminated this hotspot. Using C casts is safe if we have just checked the type with IsKind() method. I see no sense in using dynamic cast after checking type (inside which the same thing is checked). |
|
Branch CR29830_1 has been updated by abv. SHA-1: af0fb02d9b6cc5458251100d899109abf1a9b634 Detailed log of new commits: Author: abv Date: Tue Jul 17 12:05:14 2018 +0300 # minor refactoring of the code |
|
I have pushed minor correction aimed to avoid C casts in STEPControl_ActorRead::TransferEntity(), please have a look. The performance seems to be practically the same, as tested on the tree different scenarios (all times are minimal of multiple executions, new time / base time): 1. Elapsed time of "testg perf de bug29* -parallel 0": 32.5 / 31.78 2. CPU time of reading ALL_AREA01.step: 56.0 / 56.52 3. CPU time of reading ALL.step: 298 / 295 |
|
OK, but please wrap too long line. |
|
Branch CR29830_2 has been created by abv. SHA-1: 46da28b4d4313ad490238bad3d81a14861e33e85 Detailed log of new commits: Author: msv Date: Fri Jun 1 14:38:39 2018 +0300 0029830: STEPCAFControl_Reader poor performance - quadratic dependence Various performance improvements in the algorithms of STEP read/write: - Using the call to XCAFDoc_ShapeTool::SearchUsingMap() in STEPCAFControl_Reader::ReadColors() in order to quickly find the label of the shape. - Place the label of a component to the map in order to avoid iteration on all components to find a label of a component shape. It is done both in XCAFDoc_ShapeTool and in STEPCAFControl_Reader. - Using the map ShapeLabelMap in the method STEPCAFControl_Reader::FindInstance() to avoid iteration on components. - Move invariant FindEntities() out of the loop in the method getStyledItem in STEPCAFControl/STEPCAFControl_Writer.cxx. - Small fixes connected with elimination of excess copying of handles, calls of handle DownCasts and so on. - Add pointer to the end of binders chain to the Transfer_Binder class in order to speed up adding a binder to the chain. Remove stack overflow during destruction of STEP model with long chains of Transfer_Binder. Make it possible to use the Draw commands ReadStep and WriteStep to read/write from the session without accessing the disk file (use '.' for the file name). Add performance test cases for STEP reading/writing. |
|
Branch CR29830_2 has been updated by abv. SHA-1: 48bd3744148d7e0c578fc4a76b7d7183425be218 Detailed log of new commits: Author: abv Date: Wed Jul 18 20:41:31 2018 +0300 # fix regression |
|
Branch CR29830_2 has been updated forcibly by abv. SHA-1: 9ffc9b8dd1d5142afd7380115dffb7248e375dd9 |
|
Reviewed with minor amendments, please integrate from branch CR29830_2. See results of tests in Jenkins job CR29830-master-msv. |
|
Combination - OCCT branch : CR29830_2 SHA - 9ffc9b8dd1d5142afd7380115dffb7248e375dd9 Products branch : master SHA - edc03a9d94e87d0141f8dbfdc1c42f755e9765d9 was compiled on Linux, MacOS and Windows platforms and tested in optimize mode. Number of compiler warnings: No new/fixed warnings Regressions/Differences/Improvements: No regressions/differences CPU differences: Debian70-64: OCCT Total CPU difference: 17243.200000000084 / 17080.800000000007 [+0.95%] Products Total CPU difference: 7474.120000000019 / 7461.820000000036 [+0.16%] Windows-64-VC10: OCCT Total CPU difference: 16860.556879798518 / 16984.608874998572 [-0.73%] Products Total CPU difference: 8218.975085399885 / 8248.209672799876 [-0.35%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
Branch CR29830_2 has been deleted by inv. SHA-1: 9ffc9b8dd1d5142afd7380115dffb7248e375dd9 |
|
Branch CR29830_1 has been deleted by inv. SHA-1: af0fb02d9b6cc5458251100d899109abf1a9b634 |
|
Branch CR29830 has been deleted by inv. SHA-1: ea171d917a43b2c3500ed27421c9acc9eafcbb57 |
occt: master 63cdf48e 2018-06-01 11:38:39
Committer: bugmaster Details Diff |
0029830: STEPCAFControl_Reader poor performance - quadratic dependence Various performance improvements in STEP read/write algorithms: - Search for the label of a shape or component shape is improved using map mechanism instead of brute force iteration. - Invariant FindEntities() is moved out of the loop in the method getStyledItem in STEPCAFControl/STEPCAFControl_Writer.cxx. - A pointer to the end of binders chain is added in Transfer_Binder class to speed up adding a binder to the chain. - Small fixes are added to eliminate excess copying of handles, calls of handle DownCasts and so on. Stack overflow is removed during destruction of STEP model with long chains of Transfer_Binder. It is possible to use the Draw commands ReadStep and WriteStep to read/write from the session without accessing the disk file (use '.' for the file name). Performance test cases for STEP reading/writing have been added. |
Affected Issues 0029830 |
|
mod - src/STEPCAFControl/STEPCAFControl_Reader.cxx | Diff File | ||
mod - src/STEPCAFControl/STEPCAFControl_Reader.hxx | Diff File | ||
mod - src/STEPCAFControl/STEPCAFControl_Writer.cxx | Diff File | ||
mod - src/STEPControl/STEPControl_ActorRead.cxx | Diff File | ||
mod - src/StepVisual/StepVisual_StyledItem.cxx | Diff File | ||
mod - src/StepVisual/StepVisual_StyledItem.hxx | Diff File | ||
mod - src/Transfer/Transfer_Binder.cxx | Diff File | ||
mod - src/Transfer/Transfer_Binder.hxx | Diff File | ||
mod - src/Transfer/Transfer_SimpleBinderOfTransient.cxx | Diff File | ||
mod - src/XCAFDoc/XCAFDoc_ShapeTool.cxx | Diff File | ||
mod - src/XCAFDoc/XCAFDoc_ShapeTool.hxx | Diff File | ||
mod - src/XDEDRAW/XDEDRAW_Common.cxx | Diff File | ||
mod - tests/perf/de/begin | Diff File | ||
add - tests/perf/de/bug29830_1 | Diff File | ||
add - tests/perf/de/bug29830_2 | Diff File | ||
add - tests/perf/de/bug29830_3 | Diff File | ||
add - tests/perf/de/bug29830_dir/script | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2018-06-01 09:39 |
|
New Issue | |
2018-06-01 09:39 |
|
Assigned To | => msv |
2018-06-01 14:38 | git | Note Added: 0076522 | |
2018-06-01 14:42 |
|
Status | new => assigned |
2018-06-01 15:01 |
|
Description Updated | |
2018-06-01 15:01 |
|
Steps to Reproduce Updated | |
2018-06-01 15:02 | kgv | Summary | STEPCAFControl_Reader poor performance - quadratic dependence => Data Exchange, STEPCAFControl_Reader poor performance - quadratic dependence |
2018-06-01 15:02 | kgv | Steps to Reproduce Updated | |
2018-06-01 15:03 |
|
Note Added: 0076523 | |
2018-06-01 15:04 |
|
Note Edited: 0076523 | |
2018-06-01 16:26 |
|
Note Added: 0076526 | |
2018-06-01 16:26 |
|
Assigned To | msv => abv |
2018-06-01 16:26 |
|
Status | assigned => resolved |
2018-06-01 16:27 |
|
Note Added: 0076527 | |
2018-06-04 12:25 |
|
Note Added: 0076582 | |
2018-06-04 12:25 |
|
Assigned To | abv => bugmaster |
2018-06-04 12:25 |
|
Status | resolved => reviewed |
2018-06-04 15:00 | bugmaster | Test case number | => Not needed |
2018-06-04 15:04 | bugmaster | Note Added: 0076591 | |
2018-06-04 15:04 | bugmaster | Status | reviewed => tested |
2018-06-04 15:05 |
|
Note Added: 0076592 | |
2018-06-05 15:55 |
|
Assigned To | bugmaster => msv |
2018-06-05 15:55 |
|
Status | tested => assigned |
2018-06-06 12:20 | git | Note Added: 0076633 | |
2018-06-06 12:27 | git | Note Added: 0076634 | |
2018-06-06 12:30 | git | Note Added: 0076635 | |
2018-06-07 11:20 | git | Note Added: 0076655 | |
2018-06-07 17:07 | git | Note Added: 0076665 | |
2018-06-09 15:01 |
|
Relationship added | related to 0023974 |
2018-06-09 16:06 |
|
Relationship added | related to 0023979 |
2018-06-09 16:47 | git | Note Added: 0076705 | |
2018-06-09 16:51 |
|
Note Added: 0076706 | |
2018-06-09 16:51 |
|
Assigned To | msv => pdn |
2018-06-09 16:51 |
|
Status | assigned => resolved |
2018-06-09 18:39 |
|
Note Added: 0076713 | |
2018-06-09 18:48 | git | Note Added: 0076714 | |
2018-06-09 18:49 |
|
Note Added: 0076715 | |
2018-06-09 19:57 | git | Note Added: 0076716 | |
2018-06-09 20:21 | git | Note Added: 0076717 | |
2018-06-13 09:55 |
|
Note Added: 0076730 | |
2018-06-13 10:27 |
|
Assigned To | pdn => abv |
2018-07-12 19:45 |
|
Note Added: 0077560 | |
2018-07-13 15:18 |
|
Note Added: 0077574 | |
2018-07-13 15:19 |
|
Note Edited: 0077574 | |
2018-07-17 13:50 | git | Note Added: 0077729 | |
2018-07-17 13:54 |
|
Note Added: 0077730 | |
2018-07-17 15:19 |
|
Note Added: 0077735 | |
2018-07-17 21:58 | git | Note Added: 0077759 | |
2018-07-18 20:42 | git | Note Added: 0077794 | |
2018-07-18 20:47 | git | Note Added: 0077795 | |
2018-07-18 23:02 |
|
Note Added: 0077796 | |
2018-07-18 23:02 |
|
Assigned To | abv => bugmaster |
2018-07-18 23:02 |
|
Status | resolved => reviewed |
2018-07-19 10:31 | bugmaster | Test case number | Not needed => perf/de/bug29830_1, perf/de/bug29830_2, perf/de/bug29830_3 |
2018-07-19 10:57 | bugmaster | Note Added: 0077808 | |
2018-07-19 10:57 | bugmaster | Status | reviewed => tested |
2018-07-21 18:39 | bugmaster | Changeset attached | => occt master 63cdf48e |
2018-07-21 18:39 | bugmaster | Status | tested => verified |
2018-07-21 18:39 | bugmaster | Resolution | open => fixed |
2018-07-21 20:02 | git | Note Added: 0077878 | |
2018-07-21 20:02 | git | Note Added: 0077883 | |
2018-07-21 20:02 | git | Note Added: 0077893 |