View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0024911 | Open CASCADE | OCCT:Foundation Classes | public | 2014-05-08 20:18 | 2014-11-11 12:52 |
| Reporter | Assigned To | apn | |||
| Priority | normal | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Product Version | 6.7.1 | ||||
| Target Version | 6.8.0 | Fixed in Version | 6.8.0 | ||
| Summary | 0024911: Avoid using virtual functions in NCollection classes | ||||
| Description | As discovered in context of 0024831, NCollection classes show considerable less performance than native STL collections when used with STL algorithms. One of possible reasons for that is that many simple methods of NCollection classes that could be expanded inline are defined as virtual, and hence are most likely instantiated as out-of-line. The background of this is design approach selected for implementation of NCollection classes: they inherit common base class, NCollection_BaseCollection, and their iterator classes inherit common base iterator. The idea likely was that it will be possible to deal with collections of arbitrary type using base interface. In particular, some collections provide method Assign() with BaseCollection as argument. However, this feature seems to be never or rarely used, and if needed, can be implemented more efficiently with templates. Thus it is proposed to eliminate BaseCollection class and convert all virtual methods to non-virtual, to allow their inline substitution. | ||||
| Tags | No tags attached. | ||||
| Test case number | Not needed | ||||
|
|
Fix pushed to CR24911, please review. Elimination of 'virtual' allows speed up of iteration by NCollection_Array by approximately 2 times, making it comparable with std::vector in operations like std::sort, and some speed up for Vector (see notes to 0024831). Alas, it does not help much to list and sequence... |
|
|
No remarks. |
|
|
Please do not test branch CR24911_1, test branch CR24240_1 instead (it includes both fixes) |
|
|
Fix pushed to CR24240_3 |
|
|
Tested along with 0024240 (branch CR24240_4) |
|
|
Is there a guide on, how to patch SMESH to work with this modification? I'm getting errors when i try to compile SMESH version 5.1.2.2 against OCCT IR-2014-06-05 |
|
|
Basically you need to: - delete all references to class NCollection_BaseCollection, and use of macro DEFINE_BASECOLLECTION - add "#include <NCollection_IncAllocator.hxx>" in the code that uses NCollection_IncAllocator class if you get compiler error on unknown type (it was previously included in BaseCollection) - if you have compiler error on use of method Assign() between two collections of different kind (e.g. sequence assigned to list), replace this by per-element copying If you have other kind of error, please post it here |
|
|
Thanks Andrey. I was able to modify the definition of the collections to get it compile again. (At least with anything besides the Borland compiler). However now i having out of bounds errors on arrays somewhere deep inside of MEFISTO2. I collect my notes regarding this at http://freecadweb.org/tracker/view.php?id=1574 Debugging this issue is to difficult for me. I don't understand all the French comments and i have no idea of the MEFISTO internals. I don't even know how to properly debug Fortran Code. Do you know if the Salome team has scheduled to make the change to the NCollections in their code base? |
|
|
Dear shoogen, >> Do you know if the Salome team has scheduled to make the change to the NCollections in their code base? please consider patch within branch CR25064 (should be in master soon). |
|
occt: master ddf2fe8e 2014-05-08 05:13:00
Committer: apn Details Diff |
0024911: Avoid using virtual functions in NCollection classes NCollection_BaseCollection class, relevant header files, and macro DEFINE_BASECOLLECTION removed. Hence methods Assign() from other compatible (via inheritance of BaseCollection) collections are not available any more, as well as base Iterator class. All methods of Iterator classes are made non-virtual, allowing their inline expansion for better performance. OCCT-specific operators new and delete added to collection classes and removed from iterator classes. |
Affected Issues 0024911 |
|
| mod - src/AIS/AIS_NDataMapOfTransientIteratorOfListTransient.hxx | Diff File | ||
| mod - src/AIS/AIS_NListTransient.hxx | Diff File | ||
| mod - src/BOPAlgo/BOPAlgo_BOP.cxx | Diff File | ||
| mod - src/BOPCol/BOPCol_NCVector.hxx | Diff File | ||
| mod - src/BOPDS/BOPDS_Iterator.cxx | Diff File | ||
| mod - src/BOPDS/BOPDS_IteratorSI.cxx | Diff File | ||
| mod - src/BOPDS/BOPDS_SubIterator.cxx | Diff File | ||
| mod - src/Font/Font_NListOfSystemFont.hxx | Diff File | ||
| mod - src/GeomInt/GeomInt_IntSS_1.cxx | Diff File | ||
| mod - src/Graphic3d/Graphic3d_NListOfHAsciiString.hxx | Diff File | ||
| mod - src/Message/Message_MsgFile.cxx | Diff File | ||
| mod - src/NCollection/FILES | Diff File | ||
| mod - src/NCollection/NCollection_Array1.hxx | Diff File | ||
| mod - src/NCollection/NCollection_Array2.hxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseAllocator.cxx | Diff File | ||
| rm - src/NCollection/NCollection_BaseCollection.hxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseList.cxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseList.hxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseMap.cxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseSequence.cxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseSequence.hxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseVector.cxx | Diff File | ||
| mod - src/NCollection/NCollection_BaseVector.hxx | Diff File | ||
| mod - src/NCollection/NCollection_Buffer.hxx | Diff File | ||
| mod - src/NCollection/NCollection_CellFilter.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DataMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineArray1.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineArray2.hxx | Diff File | ||
| rm - src/NCollection/NCollection_DefineBaseCollection.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineDataMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineDoubleMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineHArray1.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineHArray2.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineHSequence.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineIndexedDataMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineIndexedMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineSequence.hxx | Diff File | ||
| rm - src/NCollection/NCollection_DefineTListIterator.hxx | Diff File | ||
| rm - src/NCollection/NCollection_DefineTListNode.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DefineVector.hxx | Diff File | ||
| mod - src/NCollection/NCollection_DoubleMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_IndexedDataMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_IndexedMap.hxx | Diff File | ||
| mod - src/NCollection/NCollection_List.hxx | Diff File | ||
| mod - src/NCollection/NCollection_ListNode.hxx | Diff File | ||
| mod - src/NCollection/NCollection_LocalArray.hxx | Diff File | ||
| mod - src/NCollection/NCollection_Map.hxx | Diff File | ||
| mod - src/NCollection/NCollection_Sequence.hxx | Diff File | ||
| rm - src/NCollection/NCollection_StdBase.hxx | Diff File | ||
| mod - src/NCollection/NCollection_StlIterator.hxx | Diff File | ||
| mod - src/NCollection/NCollection_TListIterator.hxx | Diff File | ||
| mod - src/NCollection/NCollection_TListNode.hxx | Diff File | ||
| mod - src/NCollection/NCollection_UBTree.hxx | Diff File | ||
| mod - src/NCollection/NCollection_Vector.hxx | Diff File | ||
| mod - src/Prs3d/Prs3d_NListOfSequenceOfPnt.hxx | Diff File | ||
| mod - src/SelectMgr/SelectMgr_DataMapOfObjectOwners.hxx | Diff File | ||
| mod - src/TObj/TObj_Container.hxx | Diff File | ||
| mod - src/TObj/TObj_Model.hxx | Diff File | ||
| mod - src/TObj/TObj_SequenceOfIterator.hxx | Diff File | ||
| mod - src/TObj/TObj_SequenceOfObject.hxx | Diff File | ||
| mod - src/TObj/TObj_TNameContainer.cxx | Diff File | ||
| mod - src/Visual3d/Visual3d_NListOfLayerItem.hxx | Diff File | ||
| mod - src/Voxel/Voxel_TypeDef.hxx | Diff File | ||
| mod - src/VrmlData/VrmlData_Scene.hxx | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-05-08 20:18 |
|
New Issue | |
| 2014-05-08 20:18 |
|
Assigned To | => abv |
| 2014-05-08 20:20 |
|
Relationship added | child of 0024831 |
| 2014-05-12 18:08 |
|
Note Added: 0029287 | |
| 2014-05-12 18:08 |
|
Assigned To | abv => kgv |
| 2014-05-12 18:08 |
|
Status | new => resolved |
| 2014-05-13 08:38 | kgv | Note Added: 0029289 | |
| 2014-05-13 08:38 | kgv | Assigned To | kgv => bugmaster |
| 2014-05-13 08:38 | kgv | Status | resolved => reviewed |
| 2014-05-13 14:34 |
|
Note Added: 0029294 | |
| 2014-05-13 14:51 |
|
Note Edited: 0029294 | |
| 2014-05-14 16:35 | apn | Test case number | => Not needed |
| 2014-05-14 16:35 | apn | Assigned To | bugmaster => apn |
| 2014-05-21 19:04 |
|
Assigned To | apn => abv |
| 2014-05-21 19:04 |
|
Status | reviewed => assigned |
| 2014-05-28 16:39 |
|
Relationship added | related to 0024240 |
| 2014-05-28 16:43 |
|
Note Added: 0029582 | |
| 2014-05-28 16:43 |
|
Status | assigned => resolved |
| 2014-06-04 07:33 |
|
Status | resolved => reviewed |
| 2014-06-04 07:34 |
|
Note Added: 0029663 | |
| 2014-06-04 07:34 |
|
Status | reviewed => tested |
| 2014-06-06 10:53 | shoogen | Note Added: 0029726 | |
| 2014-06-06 11:07 |
|
Note Added: 0029727 | |
| 2014-06-06 12:04 | apn | Changeset attached | => occt master ddf2fe8e |
| 2014-06-06 12:04 | apn | Assigned To | abv => apn |
| 2014-06-06 12:04 | apn | Status | tested => verified |
| 2014-06-06 12:04 | apn | Resolution | open => fixed |
| 2014-06-27 16:55 | shoogen | Note Added: 0029907 | |
| 2014-06-27 17:21 | shoogen | Note Edited: 0029907 | |
| 2014-06-27 17:44 | shoogen | Note Edited: 0029907 | |
| 2014-07-17 15:12 | kgv | Note Added: 0030231 | |
| 2014-11-11 12:46 |
|
Fixed in Version | => 6.8.0 |
| 2014-11-11 12:52 |
|
Status | verified => closed |
