View Issue Details

IDProjectCategoryView StatusLast Update
0024911Open CASCADEOCCT:Foundation Classespublic2014-11-11 12:52
ReporterabvAssigned Toapn  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version6.7.1 
Target Version6.8.0Fixed in Version6.8.0 
Summary0024911: Avoid using virtual functions in NCollection classes
DescriptionAs 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.
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0024240 closedapn Open CASCADE Separation of header files for QANCollection into hxx and cxx 
child of 0024831 closedapn Open CASCADE Make iterators of NCollection classes STL-compatible 

Activities

abv

2014-05-12 18:08

manager   ~0029287

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...

kgv

2014-05-13 08:38

developer   ~0029289

No remarks.

abv

2014-05-13 14:34

manager   ~0029294

Last edited: 2014-05-13 14:51

Please do not test branch CR24911_1, test branch CR24240_1 instead (it includes both fixes)

abv

2014-05-28 16:43

manager   ~0029582

Fix pushed to CR24240_3

abv

2014-06-04 07:34

manager   ~0029663

Tested along with 0024240 (branch CR24240_4)

shoogen

2014-06-06 10:53

reporter   ~0029726

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

abv

2014-06-06 11:07

manager   ~0029727

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

shoogen

2014-06-27 16:55

reporter   ~0029907

Last edited: 2014-06-27 17:44

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?

kgv

2014-07-17 15:12

developer   ~0030231

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).

Related Changesets

occt: master ddf2fe8e

2014-05-08 05:13:00

abv


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

Issue History

Date Modified Username Field Change
2014-05-08 20:18 abv New Issue
2014-05-08 20:18 abv Assigned To => abv
2014-05-08 20:20 abv Relationship added child of 0024831
2014-05-12 18:08 abv Note Added: 0029287
2014-05-12 18:08 abv Assigned To abv => kgv
2014-05-12 18:08 abv 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 abv Note Added: 0029294
2014-05-13 14:51 abv 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 abv Assigned To apn => abv
2014-05-21 19:04 abv Status reviewed => assigned
2014-05-28 16:39 abv Relationship added related to 0024240
2014-05-28 16:43 abv Note Added: 0029582
2014-05-28 16:43 abv Status assigned => resolved
2014-06-04 07:33 abv Status resolved => reviewed
2014-06-04 07:34 abv Note Added: 0029663
2014-06-04 07:34 abv Status reviewed => tested
2014-06-06 10:53 shoogen Note Added: 0029726
2014-06-06 11:07 abv 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 aiv Fixed in Version => 6.8.0
2014-11-11 12:52 aiv Status verified => closed