MantisBT - Open CASCADE
View Issue Details
0024788Open CASCADE[OCCT] OCCT:Foundation Classespublic2014-04-02 12:462016-12-09 16:40
pdn 
apn 
normalmajor 
closedfixed 
 
[OCCT] 7.1.0[OCCT] 7.1.0 
Not needed
0024788: Foundation Classes - remove Dico_Dictionary
Dico_Dictionary and whole package implements functionality on binding generic data with the text strings. The strings are put into search tree and tree node is created for each character.

Such organization is not efficient for millions of strings. It is proposed to change Dico_Dictionaty to NCollection with the appropriate hasher.

The following actions are proposed:
1) To implement NCollection of strings
2) To use this collection in STEP interface
3) To check performance
4) To identify all the usages of Dico and use collection of strings instead.
N/A
No tags attached.
Issue History
2014-04-02 12:46pdnNew Issue
2014-04-02 12:46pdnAssigned To => abv
2014-09-12 14:31abvTarget Version => 7.0.0
2015-12-14 22:39abvTarget Version7.0.0 => 7.1.0
2016-09-23 14:06abvAssigned Toabv => ski
2016-09-23 14:06abvStatusnew => assigned
2016-10-25 12:07gitNote Added: 0059065
2016-10-26 10:44gitNote Added: 0059146
2016-10-26 10:46gitNote Added: 0059147
2016-10-26 10:55gitNote Added: 0059150
2016-10-26 11:19kgvSummaryRemove Dico_Dictionary from OCC => Foundation Classes - remove Dico_Dictionary
2016-10-26 11:27kgvNote Added: 0059153
2016-10-26 11:27kgvNote Edited: 0059153bug_revision_view_page.php?bugnote_id=59153#r15063
2016-11-03 13:08gitNote Added: 0059842
2016-11-03 13:13skiNote Added: 0059845
2016-11-03 13:13skiAssigned Toski => abv
2016-11-03 13:13skiStatusassigned => resolved
2016-11-03 13:13skiSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=15192#r15192
2016-11-03 18:44abvNote Added: 0059887
2016-11-03 18:44abvAssigned Toabv => bugmaster
2016-11-03 18:44abvStatusresolved => reviewed
2016-11-03 18:52mkvAssigned Tobugmaster => mkv
2016-11-03 19:00pdnNote Added: 0059890
2016-11-03 19:11kgvNote Added: 0059891
2016-11-03 19:19gitNote Added: 0059894
2016-11-07 13:40mkvNote Added: 0059926
2016-11-07 13:40mkvNote Added: 0059927
2016-11-07 13:41mkvNote Added: 0059928
2016-11-07 13:41mkvAssigned Tomkv => bugmaster
2016-11-07 13:41mkvStatusreviewed => tested
2016-11-07 13:41mkvTest case number => Not needed
2016-11-09 17:43apnChangeset attached => occt master 997e128f
2016-11-09 17:43apnAssigned Tobugmaster => apn
2016-11-09 17:43apnStatustested => verified
2016-11-09 17:43apnResolutionopen => fixed
2016-12-07 11:30gitNote Added: 0061229
2016-12-09 16:29aivStatusverified => closed
2016-12-09 16:40aivFixed in Version => 7.1.0

Notes
(0059065)
git   
2016-10-25 12:07   
Branch CR24788 has been created by ski.

SHA-1: 801deab8b229d77fe3672408e8ea7798e7e8b8bd


Detailed log of new commits:

Author: ski
Date: Wed Oct 12 18:02:55 2016 +0300

    0024788: Remove Dico_Dictionary from OCC
    
    Class Dico_Dictionary was replaced by NCollection_DataMap.
(0059146)
git   
2016-10-26 10:44   
Branch CR24788 has been updated forcibly by ski.

SHA-1: 953ead8d78080e76dd901695f36aff8e8f350f4a
(0059147)
git   
2016-10-26 10:46   
Branch CR24788 has been updated forcibly by ski.

SHA-1: a27d420276d6557e8d085b1b9957f1869f4a2750
(0059150)
git   
2016-10-26 10:55   
Branch CR24788 has been updated by ski.

SHA-1: 5ae9d05dbc8c377c2c811bba71f0fa870dee8ab3


Detailed log of new commits:

Author: ski
Date: Wed Oct 12 18:02:55 2016 +0300

    Changed argument name in template class NCollection_Datamap to eliminate redefinitions of static members in dependent code.

(0059153)
kgv   
2016-10-26 11:27   
-  if (!thenames->GetItem(name.ToCString(),id)) {
+  if (!thenames.IsBound(name.ToCString())) {
     sout << " -- Item Unknown in File : " << name
       << " lineno " << thenl << " param." << nm << 
endl;
     id = 0;
+  } else {
+    id = thenames.Find(name.ToCString());

Hint for this and other places: the replacement for GetItem() should be
> if (!thenames.Find (name.ToCString(), id)) {
And it seems that ToCString() is redundant here (name is already TCollection_AsciiString).

(0059842)
git   
2016-11-03 13:08   
Branch CR24788 has been updated forcibly by ski.

SHA-1: af5dc262ee96e7673b07a6adfc1b64cbf62a20f6
(0059845)
ski   
2016-11-03 13:13   
Dear abv,

OCCT branch CR24788 and PRODUCT branch CR24788 were created.

Package Dico was removed and replaced by NCollection_DataMap/NCollection_IndexedDataMap.

Documentation was updated.

Please, review.
(0059887)
abv   
2016-11-03 18:44   
Looks reasonable, please test
(0059890)
pdn   
2016-11-03 19:00   
Minor remark, for STEP RWStepAP214_ReadWriteModule may be it would be good to put expected size of the map to avoid dynamic map extending.
(0059891)
kgv   
2016-11-03 19:11   
Patch contains many occurrencies of double-lookups from the map that can be optimized.

+        if (aMapCountResult.IsBound(mess))
+          aMapCountResult.ChangeFind(mess)++;

[many similar occurrences]
if (Standard_Integer* aCounterPtr = aMapCountResult.ChangeSeek (mess))
  *aCounterPtr += 1;

+        Handle(TColStd_HSequenceOfInteger) alist;
+        if (aMapList.IsBound(mess))
+          alist = aMapList.ChangeFind(mess);
+        else {
+          alist = new TColStd_HSequenceOfInteger();
+          aMapList.Bind(mess, alist);

if (!aMapList.Find (mess, alist))
{
  alist = new TColStd_HSequenceOfInteger();
  aMapList.Bind(mess, alist);
}
(0059894)
git   
2016-11-03 19:19   
Branch CR24788 has been updated forcibly by mkv.

SHA-1: 3526c9cb1afa9d1ae20ebb1125bfdabb90c7e8d9
(0059926)
mkv   
2016-11-07 13:40   
Dear BugMaster,
Branch CR4788 was rebased on current master of occt git-repository.
SHA-1: 3526c9cb1afa9d1ae20ebb1125bfdabb90c7e8d9
Branch CR4788 was rebased on current master of products git-repository.
SHA-1: 78ca277b016c95542c2190f307b1078fffd28eb8
(0059927)
mkv   
2016-11-07 13:40   
Dear BugMaster,
Branch CR4788 from occt git-repository (and CR4788 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: 3526c9cb1afa9d1ae20ebb1125bfdabb90c7e8d9
SHA-1: 78ca277b016c95542c2190f307b1078fffd28eb8

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

products component :
Linux: 63 (63 on master)
Windows: 0 (0 on master)
MacOS : 1150

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 88976422 / 91224905 [-2.46%]
Total CPU difference: 19318.38999999983 / 19514.21000000008 [-1.00%]
products component :
Total MEMORY difference: 29945323 / 30588723 [-2.10%]
Total CPU difference: 5269.259999999971 / 5278.629999999967 [-0.18%]

Testing on Windows:
occt component :
Total MEMORY difference: 55618074 / 57451773 [-3.19%]
Total CPU difference: 18317.4658188986 / 17672.82288659874 [+3.65%]
products component :
Total MEMORY difference: 20800908 / 21473188 [-3.13%]
Total CPU difference: 5226.953905899959 / 5107.691141399945 [+2.33%]

There are no differences in images found by testdiff.
(0059928)
mkv   
2016-11-07 13:41   
Dear BugMaster,
Branch CR4788 is TESTED.
(0061229)
git   
2016-12-07 11:30   
Branch CR24788 has been deleted by kgv.

SHA-1: 3526c9cb1afa9d1ae20ebb1125bfdabb90c7e8d9