View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0031920 | Open CASCADE | OCCT:Application Framework | public | 2020-11-11 17:33 | 2021-05-22 13:54 |
| Reporter | nds | Assigned To | bugmaster | ||
| Priority | normal | Severity | feature | ||
| Status | closed | Resolution | fixed | ||
| Target Version | 7.6.0 | Fixed in Version | 7.6.0 | ||
| Summary | 0031920: Application Framework - speed up methods of getting label by entry and vice versa | ||||
| Description | It's proposed to implement some cache for Label, Entry methods of TDF_Tool. When these methods are called for the same values, it takes too much time in a custom application. If we have some cache, it lets save time. If cache used, it requires additional memory for it. By this reason, it would be nice to make such cache optional. Then, the application might decide which way is more preferable. It's proposed to implement it on OCCT side, not in the custom application because there are some internal places in OCCT, where this methods are also used, e.g. STEPCAFControl_Reader for TDF_Tool::Entry, XCAFPrs_DocumentExplorer, for TDF_Tool::Label. The proposal of container is: - tbb::concurrent_unordered_map<TCollection_AsciiString, TDF_Label, TCollection_AsciiStringHash, TCollection_AsciiStringEqual> m_ELCache; (if tbb is used) or - std::map<TCollection_AsciiString, TDF_Label> m_ELCache; | ||||
| Steps To Reproduce | Test case: bugs caf bug31920 | ||||
| Tags | No tags attached. | ||||
| Test case number | bugs/caf/bug31920 | ||||
|
|
Branch CR31920 has been created by vro. SHA-1: a0cd478719e7d29da73614a0b4cb11cd7f389cfd Detailed log of new commits: Author: vro Date: Thu Dec 31 12:52:03 2020 +0300 0031920: Application Framework - speed up methods of getting label by entry and vice versa A table for fast access to the labels by entry is implemented in OCAF document. A method TDF_Data::SetAccessByEntries(true) fills-in a table for fast access to the labels. New labels, created later will be added to the table automatically. The method TDF_Tool::Label() will search the entry in the table and then, if not found, will call the old code. Disabling of usage of the table (by calling of TDF_Data::SetAccessByEntries(false)) cleans the internal table of entries - labels. By default, the table is not used. This improvement is useful for large documents with a lot of labels, and if the application uses entries to get labels. The application should call TDF_Data::SetAccessByEntries(true) for a document and then, the method TDF_Tool::Label() called inside OCAF and XCAF will use the fast access to the labels and speed-up the application. Also, the method TDF_Tool::Entry() is improved (by MPV). Modified files: - TDF_Data.hxx and cxx: the new methods SetAccessByEntries(bool), IsAccessByEntries() and GetLabel(entry) are implemented. No need to use the method GetLabel() directly. It is called in TDF_Tool::Label(). - TDF_Label.cxx: adding of a newly created label to the table of entries - labels. - TDF_Tool.cxx: the method Entry() is accelerated (by MPV) and Label() is improved to call TDF_Data::GetLabel(). - DDF_DataCommands.cxx: a new draw-command is added SetAccessByEntry, which sets or unsets usage of the table for fast access to the labels. Usage of the draw-command is illustrated in a new test "bugs caf bug31920". Tests: - bugs caf bug31920: a new simple test to check TDF_Tool::Label() when fast access to the labels is on. For test purpose I set usage of the table by default to true (temporary) and re-ran all test grids - it works! |
|
|
Branch CR31920_1 has been created by vro. SHA-1: 8f2e87bddeffea13156ca6b0d7a1ae7619403844 Detailed log of new commits: Author: vro Date: Thu Dec 31 15:34:42 2020 +0300 0031920: Application Framework - speed up methods of getting label by entry and vice versa // A method TDF_Data::AddLabel() is set Standard_EXPORT. |
|
|
vro wrote: Dear MPV, couldyou review, please: http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31920_1-master-VRO/view/COMPARE/. [^] A test "perf NCollection A1" fails, I don't know a reason... but it seems my little changes can't introduce such a regression... The branch: OCCT - CR31920_1 Products - none. A test: bugs caf bug31920 - it tests a new draw-command SetAccessByEntry. |
|
|
Vlad, speeding up Application Framework is great. But could you please share here some measured numbers comparing performance of old/new implementations and some use case(s) where this difference is noticeable (probably, the latter could be shared by Natalia)? |
|
|
Hello Kirill! Certainly, I have some data on performance. But the end user is Natasha and only she could say whether it speeds-up the application or not... So, the test: - for TDF_Tool::Entry(), it was improved by Mikhail. For a big document (taranis.xbf, about 100Mb) I call TDF_Tool::Entry() for each label of the document 20 times. It was 1.5 seconds, now 0.08 seconds - about 20 times faster. - for TDF_Tool::Label(), a similar test: I collected entries for all labels of the document and call TDF_Tool::Label() for each label 20 times. It was 0.5 seconds, now 0.04 seconds - about 10 times faster. Mikhail has an idea to use an array instead of a map. If this improvement is not enough, we could use an array... but it is more complicated and I would like to use a simple solution before implementation of a complicated. May be it worth waiting for the tests from Natasha? I didn't ask her for testing yet... |
|
|
Dear Natalia, Could you check the improvements implemented are good for your demands and may be share with us some numbers of speedup on real use-cases. |
|
|
Dear Natasha, by default the acceleration of the method TDF_Tool::Entry() is inactive. In order to activate it call a method anylabel.Data()->SetAccessByEntries(true). Nothing more is needed, since then it should work faster, hope so... |
|
|
Dear colleagues, modification in TDF_Tool::Entry() gives a lot of performance improvement. With this modification, we needn't create a cache at all. It works better than using cached values. modification in TDF_Tool::Label() gives a bit worse performance/memory result than using mentioned container m_ELCache (tbb), but only on two files (about 1% in memory and 17% in performance) though on other files it provides similar metric values. Anyway, it's optional. We're switching if OFF in application. Thank you for help, could you please raise these modifications in OCCT repository? Best regards, Natalia |
|
|
+ Standard_Boolean aResult = myAccessByEntriesTable.IsBound(anEntry); + if (aResult) + aLabel = myAccessByEntriesTable(anEntry); Could be replaced by `Standard_Boolean aResult = myAccessByEntriesTable.Find (anEntry, aLabel );` Please check if it worth keeping the new code for `TDF_Tool::Label()` modifications considering feedback (it might be not a good idea keeping a code not used in any application). |
|
|
> modification in TDF_Tool::Entry() gives a lot of performance improvement. > With this modification, we needn't create a cache at all. > It works better than using cached values. Natalia, could these performance improvements be measured in some numbers / scenarios for tracking changes / OCCT release notes? |
|
|
Branch CR31920_2 has been created by vro. SHA-1: c4317117be3035c963f802d08a7aa9bf610df465 Detailed log of new commits: Author: vro Date: Wed May 5 11:53:39 2021 +0300 0031920: Application Framework - speed up methods of getting label by entry and vice versa A table for fast access to the labels by entry is implemented in OCAF document. A method TDF_Data::SetAccessByEntries(true) fills-in a table for fast access to the labels. New labels, created later will be added to the table automatically. The method TDF_Tool::Label() will search the entry in the table and then, if not found, will call the old code. Disabling of usage of the table (by calling of TDF_Data::SetAccessByEntries(false)) cleans the internal table of entries - labels. By default, the table is not used. This improvement is useful for large documents with a lot of labels, and if the application uses entries to get labels. The application should call TDF_Data::SetAccessByEntries(true) for a document and then, the method TDF_Tool::Label() called inside OCAF and XCAF will use the fast access to the labels and speed-up the application. Also, the method TDF_Tool::Entry() is improved (by MPV). Modified files: - TDF_Data.hxx and cxx: the new methods SetAccessByEntries(bool), IsAccessByEntries() and GetLabel(entry) are implemented. No need to use the method GetLabel() directly. It is called in TDF_Tool::Label(). - TDF_Label.cxx: adding of a newly created label to the table of entries - labels. - TDF_Tool.cxx: the method Entry() is accelerated (by MPV) and Label() is improved to call TDF_Data::GetLabel(). - DDF_DataCommands.cxx: a new draw-command is added SetAccessByEntry, which sets or unsets usage of the table for fast access to the labels. Usage of the draw-command is illustrated in a new test "bugs caf bug31920". Tests: - bugs caf bug31920: a new simple test to check TDF_Tool::Label() when fast access to the labels is on. Doc: - dox\upgrade\upgrade.md is extended for new information |
|
|
Dear Mikhail. The patch is ready for integration: http://jenkins-test-occt/view/CR31920_2-master-VRO/view/COMPARE/. [^] I applied a suggestion from Kirill. It accelerated the function TDF_Tool::Label() for about 20% in addition: Before: 0.261 seconds After: 0.194 seconds. In general, TDF_Tool::Label() is about 15 times faster. The method TDF_Tool::Entry() is about 20 times after. Natasha, could you test the application again, please? It seems usage of "native" accelerator of Open CASCADE should win now :-) . |
|
|
TDF_Data.hxx:106 + //! via entries. Internally, a table of entry - label icreated, I guess it should be "is created". Also there: + Standard_EXPORT Standard_Boolean IsAccessByEntries() const; I propose to put this method body definition into the header and make it "inline". It is used in many places where speed is important, so, this potentially could additionally speedup some use cases. |
|
|
Branch CR31920_3 has been created by vro. SHA-1: 0de2f90fe5a2d59a45c734843d256f3bf91548c0 Detailed log of new commits: Author: vro Date: Thu May 13 08:48:35 2021 +0300 0031920: Application Framework - speed up methods of getting label by entry and vice versa A table for fast access to the labels by entry is implemented in OCAF document. A method TDF_Data::SetAccessByEntries(true) fills-in a table for fast access to the labels. New labels, created later will be added to the table automatically. The method TDF_Tool::Label() will search the entry in the table and then, if not found, will call the old code. Disabling of usage of the table (by calling of TDF_Data::SetAccessByEntries(false)) cleans the internal table of entries - labels. By default, the table is not used. This improvement is useful for large documents with a lot of labels, and if the application uses entries to get labels. The application should call TDF_Data::SetAccessByEntries(true) for a document and then, the method TDF_Tool::Label() called inside OCAF and XCAF will use the fast access to the labels and speed-up the application. Also, the method TDF_Tool::Entry() is improved (by MPV). Modified files: - TDF_Data.hxx and cxx: the new methods SetAccessByEntries(bool), IsAccessByEntries() and GetLabel(entry) are implemented. No need to use the method GetLabel() directly. It is called in TDF_Tool::Label(). - TDF_Label.cxx: adding of a newly created label to the table of entries - labels. - TDF_Tool.cxx: the method Entry() is accelerated (by MPV) and Label() is improved to call TDF_Data::GetLabel(). - DDF_DataCommands.cxx: a new draw-command is added SetAccessByEntry, which sets or unsets usage of the table for fast access to the labels. Usage of the draw-command is illustrated in a new test "bugs caf bug31920". Tests: - bugs caf bug31920: a new simple test to check TDF_Tool::Label() when fast access to the labels is on. Doc: - dox\upgrade\upgrade.md is extended for new information |
|
|
Thanks for remarks, fixed: http://jenkins-test-occt/view/CR31920_3-master-VRO/view/COMPARE/. No advantage of the inline-method is measured, but probably on some other computers and using some other compilers it may indeed speed-up a little the functionality. |
|
|
+ Standard_EXPORT void AddLabel (const TDF_Label& aLabel); Just an idea - maybe "RegisterLabel() / FindRegisteredLabel()" would better describe the purpose of these methods? |
|
|
+Standard_Boolean TDF_Data::GetLabel(const TCollection_AsciiString& anEntry, TDF_Label& aLabel)
+{
+ aLabel = myAccessByEntriesTable.Find (anEntry);
+ return !aLabel.IsNull();
This is not the method I have proposed to use - this Find() will raise exception if value doesn't exist in map. There is another exception-free Find() method in the map interface taking result value by argument and returning TRUE on success:
Standard_Boolean Find (const TheKeyType& theKey,
TheItemType& theValue) const
|
|
|
Branch CR31920_4 has been created by vro. SHA-1: 36169b7639ee6615a9fa6ab45ee0123a85ecc4c0 Detailed log of new commits: Author: vro Date: Thu May 13 14:19:00 2021 +0300 0031920: Application Framework - speed up methods of getting label by entry and vice versa A table for fast access to the labels by entry is implemented in OCAF document. A method TDF_Data::SetAccessByEntries(true) fills-in a table for fast access to the labels. New labels, created later will be added to the table automatically. The method TDF_Tool::Label() will search the entry in the table and then, if not found, will call the old code. Disabling of usage of the table (by calling of TDF_Data::SetAccessByEntries(false)) cleans the internal table of entries - labels. By default, the table is not used. This improvement is useful for large documents with a lot of labels, and if the application uses entries to get labels. The application should call TDF_Data::SetAccessByEntries(true) for a document and then, the method TDF_Tool::Label() called inside OCAF and XCAF will use the fast access to the labels and speed-up the application. Also, the method TDF_Tool::Entry() is improved (by MPV). Modified files: - TDF_Data.hxx and cxx: the new methods SetAccessByEntries(bool), IsAccessByEntries() and GetLabel(entry) are implemented. No need to use the method GetLabel() directly. It is called in TDF_Tool::Label(). - TDF_Label.cxx: adding of a newly created label to the table of entries - labels. - TDF_Tool.cxx: the method Entry() is accelerated (by MPV) and Label() is improved to call TDF_Data::GetLabel(). - DDF_DataCommands.cxx: a new draw-command is added SetAccessByEntry, which sets or unsets usage of the table for fast access to the labels. Usage of the draw-command is illustrated in a new test "bugs caf bug31920". Tests: - bugs caf bug31920: a new simple test to check TDF_Tool::Label() when fast access to the labels is on. Doc: - dox\upgrade\upgrade.md is extended for new information |
|
|
Fixed: http://jenkins-test-occt/view/CR31920_4-master-VRO/view/COMPARE/ Indeed, AddLabel() is better to rename to RegisterLabel(). Also, GetLabel() became one-line method because of usage of another Find() method. It sped-up the functionality for about 10%! (from 0.059 sec to 0.052 sec) Thanks! |
|
|
For me it is fine now. OCCT branch: CR31920_4 Products branch: NOTHING |
|
|
Combination - OCCT branch : IR-2021-05-21 master SHA - 2aa0a6991da6f767c2a6a4c1d6d720fee35867ce a87b7ddc8cb44606b91e3f37113847c3f5f50fdc Products branch : IR-2021-05-21 SHA - e89cefb48fe77db69215a5124b453f69d9db404b 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: Debian80-64: OCCT Total CPU difference: 17862.650000000373 / 17879.470000000332 [-0.09%] Products Total CPU difference: 11535.410000000107 / 11557.880000000083 [-0.19%] Windows-64-VC14: OCCT Total CPU difference: 19361.25 / 19400.171875 [-0.20%] Products Total CPU difference: 12920.28125 / 12880.234375 [+0.31%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
|
Branch CR31920 has been deleted by mnt. SHA-1: a0cd478719e7d29da73614a0b4cb11cd7f389cfd |
|
|
Branch CR31920_1 has been deleted by mnt. SHA-1: 8f2e87bddeffea13156ca6b0d7a1ae7619403844 |
|
|
Branch CR31920_2 has been deleted by mnt. SHA-1: c4317117be3035c963f802d08a7aa9bf610df465 |
|
|
Branch CR31920_3 has been deleted by mnt. SHA-1: 0de2f90fe5a2d59a45c734843d256f3bf91548c0 |
|
|
Branch CR31920_4 has been deleted by mnt. SHA-1: 36169b7639ee6615a9fa6ab45ee0123a85ecc4c0 |
|
occt: master 604aa3f4 2021-05-13 11:19:00 Committer: bugmaster Details Diff |
0031920: Application Framework - speed up methods of getting label by entry and vice versa A table for fast access to the labels by entry is implemented in OCAF document. A method TDF_Data::SetAccessByEntries(true) fills-in a table for fast access to the labels. New labels, created later will be added to the table automatically. The method TDF_Tool::Label() will search the entry in the table and then, if not found, will call the old code. Disabling of usage of the table (by calling of TDF_Data::SetAccessByEntries(false)) cleans the internal table of entries - labels. By default, the table is not used. This improvement is useful for large documents with a lot of labels, and if the application uses entries to get labels. The application should call TDF_Data::SetAccessByEntries(true) for a document and then, the method TDF_Tool::Label() called inside OCAF and XCAF will use the fast access to the labels and speed-up the application. Also, the method TDF_Tool::Entry() is improved (by MPV). Modified files: - TDF_Data.hxx and cxx: the new methods SetAccessByEntries(bool), IsAccessByEntries() and GetLabel(entry) are implemented. No need to use the method GetLabel() directly. It is called in TDF_Tool::Label(). - TDF_Label.cxx: adding of a newly created label to the table of entries - labels. - TDF_Tool.cxx: the method Entry() is accelerated (by MPV) and Label() is improved to call TDF_Data::GetLabel(). - DDF_DataCommands.cxx: a new draw-command is added SetAccessByEntry, which sets or unsets usage of the table for fast access to the labels. Usage of the draw-command is illustrated in a new test "bugs caf bug31920". Tests: - bugs caf bug31920: a new simple test to check TDF_Tool::Label() when fast access to the labels is on. Doc: - dox\upgrade\upgrade.md is extended for new information |
Affected Issues 0031920 |
|
| mod - dox/upgrade/upgrade.md | Diff File | ||
| mod - src/DDF/DDF_DataCommands.cxx | Diff File | ||
| mod - src/TDF/TDF_Data.cxx | Diff File | ||
| mod - src/TDF/TDF_Data.hxx | Diff File | ||
| mod - src/TDF/TDF_Label.cxx | Diff File | ||
| mod - src/TDF/TDF_Tool.cxx | Diff File | ||
| add - tests/bugs/caf/bug31920 | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-11-11 17:33 | nds | New Issue | |
| 2020-11-11 17:33 | nds | Assigned To | => mpv |
| 2020-12-05 20:54 | kgv | Summary | Application Framework - speed up methods of getting label by entry and vise versa => Application Framework - speed up methods of getting label by entry and vice versa |
| 2020-12-31 12:51 | git | Note Added: 0097998 | |
| 2020-12-31 15:34 | git | Note Added: 0097999 | |
| 2021-01-01 16:33 | vro | Status | new => resolved |
| 2021-01-01 16:33 | vro | Steps to Reproduce Updated | |
| 2021-01-01 21:45 | kgv | Note Added: 0098004 | |
| 2021-01-01 21:45 | kgv | Steps to Reproduce Updated | |
| 2021-01-01 21:49 | kgv | Note Added: 0098005 | |
| 2021-01-01 21:50 | kgv | Note Edited: 0098005 | |
| 2021-01-02 09:23 | vro | Note Added: 0098006 | |
| 2021-01-02 11:06 |
|
Note Added: 0098007 | |
| 2021-01-02 11:06 |
|
Assigned To | mpv => nds |
| 2021-01-02 11:06 |
|
Status | resolved => feedback |
| 2021-01-02 12:19 | vro | Note Added: 0098010 | |
| 2021-04-30 10:14 | nds | Note Added: 0100682 | |
| 2021-04-30 10:14 | nds | Assigned To | nds => vro |
| 2021-04-30 10:14 | nds | Status | feedback => reviewed |
| 2021-04-30 11:42 | kgv | Note Added: 0100690 | |
| 2021-04-30 11:42 | kgv | Status | reviewed => feedback |
| 2021-04-30 11:42 | kgv | Note Edited: 0100690 | |
| 2021-04-30 11:44 | kgv | Note Added: 0100691 | |
| 2021-05-05 11:55 | git | Note Added: 0100777 | |
| 2021-05-05 14:46 | vro | Assigned To | vro => mpv |
| 2021-05-05 14:46 | vro | Status | feedback => resolved |
| 2021-05-05 14:46 | vro | Steps to Reproduce Updated | |
| 2021-05-05 14:47 | vro | Note Added: 0100781 | |
| 2021-05-12 09:16 |
|
Note Added: 0100909 | |
| 2021-05-12 09:16 |
|
Assigned To | mpv => vro |
| 2021-05-12 09:16 |
|
Status | resolved => assigned |
| 2021-05-13 08:48 | git | Note Added: 0100928 | |
| 2021-05-13 11:05 | vro | Note Added: 0100933 | |
| 2021-05-13 11:05 | vro | Assigned To | vro => mpv |
| 2021-05-13 11:05 | vro | Status | assigned => resolved |
| 2021-05-13 11:05 | vro | Steps to Reproduce Updated | |
| 2021-05-13 11:48 | kgv | Note Added: 0100939 | |
| 2021-05-13 11:48 | kgv | Note Edited: 0100939 | |
| 2021-05-13 11:54 | kgv | Note Added: 0100940 | |
| 2021-05-13 11:54 | kgv | Note Edited: 0100940 | |
| 2021-05-13 14:18 | git | Note Added: 0100947 | |
| 2021-05-13 17:13 | vro | Note Added: 0100950 | |
| 2021-05-13 17:13 | vro | Status | resolved => feedback |
| 2021-05-17 14:34 |
|
Note Added: 0101087 | |
| 2021-05-17 14:34 |
|
Assigned To | mpv => bugmaster |
| 2021-05-17 14:34 |
|
Status | feedback => reviewed |
| 2021-05-22 12:23 | bugmaster | Note Added: 0101247 | |
| 2021-05-22 12:23 | bugmaster | Status | reviewed => tested |
| 2021-05-22 12:26 | bugmaster | Test case number | => bugs/caf/bug31920 |
| 2021-05-22 12:38 | bugmaster | Changeset attached | => occt master 604aa3f4 |
| 2021-05-22 12:38 | bugmaster | Status | tested => verified |
| 2021-05-22 12:38 | bugmaster | Resolution | open => fixed |
| 2021-05-22 13:54 | git | Note Added: 0101267 | |
| 2021-05-22 13:54 | git | Note Added: 0101268 | |
| 2021-05-22 13:54 | git | Note Added: 0101269 | |
| 2021-05-22 13:54 | git | Note Added: 0101270 | |
| 2021-05-22 13:54 | git | Note Added: 0101271 |
