View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0023850 | Open CASCADE | OCCT:Application Framework | public | 2013-03-22 12:58 | 2015-07-27 15:01 |
| Reporter | vro | Assigned To | vro | ||
| Priority | normal | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Platform | Windows | OS | VC++ 2010 | ||
| Product Version | 6.5.3 | ||||
| Target Version | 6.7.0 | Fixed in Version | 6.7.0 | ||
| Summary | 0023850: TDataStd_ByteArray is too slow on storage on disk | ||||
| Description | If a TDataStd_ByteArray contains several hundreds of thousands of values, it takes a minute to store the array on disk using any of OCAF storage mechanisms. | ||||
| Steps To Reproduce | #Create an OCAF document. Use XML as a test storage schema. NewDoc D XmlOcaf #Set a long byte array of many-many values. Here it is an array of 10 values (seems not enough for performance test). SetByteArray D 0:1 0 1 10 1 2 3 4 5 6 7 8 9 10 # Save the document on disk and measure time of storage. SaveAs D test.xml | ||||
| Tags | No tags attached. | ||||
| Test case number | caf bugs(006) D1 | ||||
|
|
Dear Szy, could you please review the changes? A file XmlMDataStd_ByteArrayDriver.cxx is modified in CR23850. |
|
|
I modified the same file once again. Could you please review the changes? |
|
|
Storage of a TDataStd_TreeNode is accelerated. For a document, where a TDataStd_TreeNode has 100 000 children, storage is about 8 times faster now! Don't review this improvement yet, please. I would like to continue... and rebase the changes at the end. |
|
|
Test23850.zip (1,575 bytes) |
|
|
Dear Mpv, Could you review the improvement, please? The improvement consists of increase of speed of storage on disk for several Ocaf attributes: TreeNode, IntegerArray, IntegerList, ... Also, I had to add several new draw-commands to manipulate with the Ocaf attributes: SetBooleanArray, GetBooleanArray, SetIntegerList, GetIntegerList, ... A draw-script Test23850 (attached) sets the attributes for a document, saves the document on disk in XML format and then reads it back and checks saved values. This script might be used for non-regression tests of this improvement, I suppose. The changes are in CR23850 branch. Any questions are welcome, Vro |
|
|
Vlad, could you add a test script to Git branch to make it directly usable (see http://files.opencascade.com/delivery/download/OCCT_Tests.pdf for how-to)? |
|
|
A test-script caf\bugs\D1 is added to control this improvement. |
|
|
Dear Vlad, in files XmlMDataStd_BooleanListDriver.cxx, XmlMDataStd_IntegerArrayDriver.cxx, XmlMDataStd_IntegerListDriver.cxx, XmlMDataStd_IntPackedMapDriver.cxx, XmlMDataStd_RealArrayDriver.cxx, XmlMDataStd_RealListDriver.cxx, XmlMDataStd_TreeNodeDriver.cxx there must be "+1" in allocated size of the arrays to avoid possible memory problems (zero symbol in the end of string may be located out of range or array). |
|
|
Done! + 1 is added to keep '\0', thanks. |
|
|
Using direct calls to Standard::Allocate and Standard::Free for pointers to char is bad practice, please try to avoid this (consider using C++ classes, e.g. TCollection_AsciiString can be used as safe string buffer). |
|
|
Do you mean to use: TCollection_AsciiString str("", length); instead of: Standard_Character* str = (Standard_Character*) Standard::Allocate(length + 1)? In this case we don't need to free the string, the C++ class (TCollection_AsciiString) will do it for us. Do you mean that? |
|
|
I thought about your idea and it seems to me usage of TCollection_AsciiString is not reasonable here because: 1) It will hide calling of Standard_Free() for the array of chars. For some drivers, for example for the BooleanArrayDriver the array is allocated following a condition (inside an "IF" statement), which makes not obvious allocation and release of the array. So, it decreases the source code visibility. 2) It slows-down the drivers because of filling-in the allocated array by a "filler" character. It slows-down not very much, but I would prefer not to spend time at all. 3) TCollection_AsciiString returns a constant reference to the allocated array of characters (the method ToCString()). In order to use this array in the method sprintf() it is neccesary to cast the object to Standard_Character* (without a const specifier). It looks like a "hacking" code... which doesn't add visibility. What do you think? |
|
|
I agree with Vlad: "Cut" method of two AsciiString-s here is too slow, usage of "ToCString" for sprintf here is awful. Perhaps, ideal solution here is creation of new container for array of characters (TCollection_Buffer?) for encapsulation of memory management and appending any type (char, int, real, etc) as string... |
|
|
Well, then consider using NCollection_Array1<char>. If you need optimization for small buffers, check PLib_LocalArray: this class can be made template (and moved to NCollection) to be useful for chars |
|
|
PLib_LocalArray is renamed to NCollection_LocalArray and became a template. It is used as a local array for Standard_Character in XML OCAF drivers, and as a local array of Standard_Real in PLib package. Also, I forgot to mention this, sorry, I extended a RealDriver to support 17 digits after a decimal point (previously it used 15). Now it allows to store and retrieve a double value with highest precision. The changes are in CR23850. |
|
|
It seems it is OK and corresponds to notes now. |
|
|
Dear Mkv, Integrate please the changes of CR23850. |
|
|
Please review it again. |
|
|
Information about improvement of performance. Speed of storage of an OCAF document in XML file format increased for about 10% (for an usual OCAF document with short arrays). For the documents using long arrays speed increased in about 10 times. |
|
|
Dear BugMaster, Branch CR23850 (and products from GIT master) was compiled on Linux and Windows platforms and tested. SHA-1: 5744855b06828092584808c4ae1015b0a7265c7e Number of compiler warnings: occt component : Linux: 2 (2 on master) Windows: 11 (11 on master) products component : Linux: 0 (0 on master) Windows: 64 (64 on master) Regressions: No regressions Improvements: No improvements Testing cases: caf bugs(006) D1 - OK. Testing on Linux: Total MEMORY difference: 365793192 / 365785356 Total CPU difference: 42342.64000000116 / 45769.360000001354 Testing on Windows: Total MEMORY difference: 419285528 / 420608076 Total CPU difference: 30196.25 / 30692.265625 There are not differences in images found by testdiff. |
|
|
Branch CR23850 has been created by mpv. SHA-1: 7b202085e63d83af2977f0543be5a5dcf56d227b Detailed log of new commits: Author: mkv Date: Wed May 22 12:10:37 2013 +0400 Small correction of test case for this fix Author: vro Date: Tue May 21 08:24:40 2013 +0400 PLib_LocalArray is renamed to NCollection_LocalArray and became a template. It is used as a local array for Standard_Character in XML OCAF drivers, and as a local array of Standard_Real in PLib package. Author: vro Date: Thu May 16 15:43:22 2013 +0400 Removed several spaces in source files. Author: vro Date: Thu May 16 08:08:53 2013 +0400 + 1 is added to keep '\0' Author: vro Date: Wed May 15 12:10:10 2013 +0400 A test-script for OCAF document successfully saved and opened from disk in XML file format. Author: vro Date: Wed May 15 09:49:28 2013 +0400 Several draw-commands are added to manipulate the basic Ocaf attributes: BooleanArray BooleanList IntegerList RealList Author: vro Date: Tue May 14 14:08:58 2013 +0400 Improvement of speed of storage of several Ocaf attributes in XML file format. Also, format of storage of a double value is extended to keep 17 digits after a decimal point (it was used only 15 digits before). Author: vro Date: Fri Apr 26 11:36:18 2013 +0400 Same improvement for storage of a TDataStd_TreeNode. Author: vro Date: Fri Apr 12 16:16:19 2013 +0400 A possible bug is corrected (size of an array is extended a little). Author: vro Date: Fri Mar 22 13:05:27 2013 +0400 Optimization of a byte-array for XML persistence (binary persistence is ok). |
|
|
Branch CR23850 has been deleted by kgv. SHA-1: 7b202085e63d83af2977f0543be5a5dcf56d227b |
|
occt: master f7b4312f 2013-05-23 08:09:09 Details Diff |
0023850: TDataStd_ByteArray is too slow on storage on disk Optimization of a byte-array for XML persistence (binary persistence is ok). A possible bug is corrected (size of an array is extended a little). Same improvement for storage of a TDataStd_TreeNode. Improvement of speed of storage of several Ocaf attributes in XML file format. Also, format of storage of a double value is extended to keep 17 digits after a decimal point (it was used only 15 digits before). Several draw-commands are added to manipulate the basic Ocaf attributes: BooleanArray BooleanList IntegerList RealList A test-script for OCAF document successfully saved and opened from disk in XML file format. + 1 is added to keep '\0' Removed several spaces in source files. PLib_LocalArray is renamed to NCollection_LocalArray and became a template. It is used as a local array for Standard_Character in XML OCAF drivers, and as a local array of Standard_Real in PLib package. Small correction of test case for this fix |
Affected Issues 0023850 |
|
| mod - src/BSplCLib/BSplCLib.cxx | Diff File | ||
| mod - src/BSplSLib/BSplSLib.cxx | Diff File | ||
| mod - src/DDataStd/DDataStd_BasicCommands.cxx | Diff File | ||
| mod - src/NCollection/FILES | Diff File | ||
| mod - src/PLib/FILES | Diff File | ||
| mod - src/PLib/PLib.cxx | Diff File | ||
| mod - src/PLib/PLib_HermitJacobi.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_BooleanArrayDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_BooleanListDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_ByteArrayDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_IntegerArrayDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_IntegerListDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_IntPackedMapDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_RealArrayDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_RealDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_RealListDriver.cxx | Diff File | ||
| mod - src/XmlMDataStd/XmlMDataStd_TreeNodeDriver.cxx | Diff File | ||
| add - tests/caf/bugs/D1 | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-03-22 12:58 | vro | New Issue | |
| 2013-03-22 12:58 | vro | Assigned To | => vro |
| 2013-03-22 13:06 | vro | Note Added: 0023857 | |
| 2013-03-22 13:06 | vro | Assigned To | vro => szy |
| 2013-03-22 13:06 | vro | Status | new => resolved |
| 2013-04-12 16:18 | vro | Note Added: 0024121 | |
| 2013-04-12 16:18 | vro | Assigned To | szy => abv |
| 2013-04-12 16:18 | vro | Status | resolved => assigned |
| 2013-04-26 11:39 | vro | Note Added: 0024289 | |
| 2013-04-26 11:39 | vro | Assigned To | abv => vro |
| 2013-05-15 09:40 | vro | File Added: Test23850.zip | |
| 2013-05-15 09:57 | vro | Note Added: 0024387 | |
| 2013-05-15 09:57 | vro | Assigned To | vro => mpv |
| 2013-05-15 09:57 | vro | Status | assigned => resolved |
| 2013-05-15 10:07 |
|
Note Added: 0024388 | |
| 2013-05-15 12:12 | vro | Note Added: 0024393 | |
| 2013-05-15 17:06 |
|
Note Added: 0024405 | |
| 2013-05-15 17:06 |
|
Assigned To | mpv => vro |
| 2013-05-15 17:06 |
|
Status | resolved => assigned |
| 2013-05-16 08:10 | vro | Note Added: 0024408 | |
| 2013-05-16 08:10 | vro | Assigned To | vro => mpv |
| 2013-05-16 08:10 | vro | Status | assigned => resolved |
| 2013-05-16 09:47 |
|
Note Added: 0024410 | |
| 2013-05-16 09:56 | vro | Note Added: 0024411 | |
| 2013-05-16 16:17 | vro | Note Added: 0024421 | |
| 2013-05-16 17:48 |
|
Note Added: 0024423 | |
| 2013-05-16 21:48 |
|
Note Added: 0024426 | |
| 2013-05-21 08:35 | vro | Note Added: 0024441 | |
| 2013-05-21 08:54 |
|
Note Added: 0024442 | |
| 2013-05-21 08:54 |
|
Status | resolved => reviewed |
| 2013-05-21 09:40 | vro | Note Added: 0024443 | |
| 2013-05-21 09:40 | vro | Assigned To | mpv => mkv |
| 2013-05-21 09:40 | vro | Status | reviewed => assigned |
| 2013-05-21 09:51 | vro | Note Added: 0024444 | |
| 2013-05-21 09:51 | vro | Assigned To | mkv => mpv |
| 2013-05-21 09:51 | vro | Status | assigned => resolved |
| 2013-05-21 09:54 |
|
Status | resolved => reviewed |
| 2013-05-21 10:33 |
|
Assigned To | mpv => mkv |
| 2013-05-21 12:14 | vro | Note Added: 0024447 | |
| 2013-05-22 17:52 |
|
Note Added: 0024473 | |
| 2013-05-22 17:53 |
|
Test case number | => caf bugs(006) D1 |
| 2013-05-22 17:53 |
|
Assigned To | mkv => bugmaster |
| 2013-05-22 17:53 |
|
Status | reviewed => tested |
| 2013-05-23 12:40 | bugmaster | Target Version | Unscheduled => 6.7.0 |
| 2013-05-24 13:06 | vro | Changeset attached | => occt master f7b4312f |
| 2013-05-24 13:06 | vro | Assigned To | bugmaster => vro |
| 2013-05-24 13:06 | vro | Status | tested => verified |
| 2013-05-24 13:06 | vro | Resolution | open => fixed |
| 2013-12-19 13:53 | bugmaster | Status | verified => closed |
| 2013-12-19 13:55 | bugmaster | Fixed in Version | => 6.7.0 |
| 2014-12-30 09:05 | git | Note Added: 0035861 | |
| 2015-07-27 15:01 | git | Note Added: 0043581 |
