MantisBT - Open CASCADE
View Issue Details
0030964Open CASCADE[OCCT] OCCT:Data Exchangepublic2019-09-13 14:552019-11-08 17:20
kgv 
osa 
normalfeature 
resolvedopen 
 
[OCCT] 7.5.0* 
0030964: Data Exchange - use Standard_ReadLineBuffer within OBJ reader
Fast prototype shows that using Standard_ReadLineBuffer may give 3x time performance boost on reading of large OBJ files (~ 1 GiB):
  struct RWObj_ReaderFile : public Standard_ReadLineBuffer
  {
...
    bool ReadLine()
    {
      size_t aLineLen = 0;
      int64_t aReadBytes = 0;
      Line = Standard_ReadLineBuffer::ReadLine (File, aLineLen, aReadBytes);
      Position += aReadBytes;
      return Line != NULL;


To complete the patch, Standard_ReadLineBuffer or its sub-class should be extended by handling of special multi-line case with \ at the end of the line.

Not required
No tags attached.
Issue History
2019-09-13 14:55kgvNew Issue
2019-09-13 14:55kgvAssigned To => kgv
2019-09-13 14:55kgvAssigned Tokgv => user897
2019-09-13 14:55kgvStatusnew => assigned
2019-09-13 14:56kgvRelationship addedchild of 0030830
2019-09-13 14:56kgvRelationship addedchild of 0029296
2019-09-13 15:02kgvDescription Updatedbug_revision_view_page.php?rev_id=21825#r21825
2019-09-16 15:59gitNote Added: 0087144
2019-09-24 14:30gitNote Added: 0087478
2019-09-24 14:54kgvNote Added: 0087482
2019-10-02 17:49gitNote Added: 0087702
2019-10-02 17:55gitNote Added: 0087713
2019-10-02 17:57user897Note Added: 0087714
2019-10-02 17:57user897Assigned Touser897 => osa
2019-10-02 17:57user897Statusassigned => resolved
2019-10-02 17:57user897Steps to Reproduce Updatedbug_revision_view_page.php?rev_id=21933#r21933
2019-10-02 18:13kgvNote Added: 0087715
2019-10-02 18:15kgvNote Added: 0087716
2019-10-02 18:17kgvNote Added: 0087717
2019-10-04 15:45gitNote Added: 0087816
2019-10-04 15:55gitNote Added: 0087819
2019-10-04 16:14user897Note Added: 0087822
2019-10-04 16:16user897Note Added: 0087824
2019-10-06 09:28abvDescription Updatedbug_revision_view_page.php?rev_id=21972#r21972
2019-10-06 09:29abvNote Added: 0087883
2019-10-06 09:42kgvNote Added: 0087885
2019-10-06 09:43kgvNote Edited: 0087885bug_revision_view_page.php?bugnote_id=87885#r21974
2019-10-06 09:43kgvNote Edited: 0087885bug_revision_view_page.php?bugnote_id=87885#r21975
2019-10-06 09:44kgvNote Edited: 0087885bug_revision_view_page.php?bugnote_id=87885#r21976
2019-10-17 15:29gitNote Added: 0088259
2019-10-17 15:30gitNote Added: 0088260
2019-10-22 14:16osaNote Added: 0088419
2019-10-22 14:16osaAssigned Toosa => user897
2019-10-22 14:16osaStatusresolved => assigned
2019-10-28 12:15gitNote Added: 0088630
2019-10-28 12:17user897Note Added: 0088631
2019-10-28 22:55kgvNote Added: 0088641
2019-10-29 12:35user897Note Added: 0088645
2019-10-29 12:35user897Assigned Touser897 => osa
2019-10-29 12:35user897Statusassigned => resolved
2019-11-05 14:43gitNote Added: 0088728
2019-11-05 14:50user897Note Added: 0088729
2019-11-05 14:59kgvNote Added: 0088730
2019-11-07 10:48gitNote Added: 0088769
2019-11-07 12:05osaAssigned Toosa => mzernova
2019-11-07 12:05osaStatusresolved => assigned
2019-11-08 11:27gitNote Added: 0088786
2019-11-08 11:28gitNote Added: 0088787
2019-11-08 11:29mzernovaAssigned Tomzernova => osa
2019-11-08 11:29mzernovaStatusassigned => resolved
2019-11-08 17:20mzernovaNote Added: 0088811

Notes
(0087144)
git   
2019-09-16 15:59   
Branch CR30964 has been created by mzernova.

SHA-1: 5a7808160e8d767d50348786e73ac79d45bfabe3


No new revisions were added by this update.
(0087478)
git   
2019-09-24 14:30   
Branch CR30964 has been updated by mzernova.

SHA-1: dd692d8b7b2129b5480d911ec45431dd0c8777d3


Detailed log of new commits:

Author: mzernova
Date: Tue Sep 17 16:05:05 2019 +0300

    0030964: Data Exchange - use Standard_ReadLineBuffer within OBJ reader
    
    The sub-class of Standard_ReadLineBuffer has been created. It supports a processing of the special multi-line case with \ at the end of the line.

(0087482)
kgv   
2019-09-24 14:54   
+//! Auxiliary tool for buffered reading of lines from input stream.
+class Standard_ReadLineBufferMultiline: Standard_ReadLineBuffer
+{

It would be more straightforward adding Boolean flag to Standard_ReadLineBuffer rather than introducing new global class.
(0087702)
git   
2019-10-02 17:49   
Branch CR30964 has been updated by mzernova.

SHA-1: 9d8ff4fa57c27a1791cf563194561f778bb502a6


Detailed log of new commits:

Author: mzernova
Date: Thu Sep 26 17:18:49 2019 +0300

    Standard_RedLineBuffer was used to load Stl files

(0087713)
git   
2019-10-02 17:55   
Branch CR30964_1 has been created by mzernova.

SHA-1: d1fbcd38aeb9728e0b87c04ea3a5d9b49eb5a4c5


Detailed log of new commits:

Author: mzernova
Date: Tue Sep 17 16:05:05 2019 +0300

    0030964: Data Exchange - use Standard_ReadLineBuffer within OBJ reader
    
    Standard_ReadLineBuffer now supports a processing of the special multi-line case with \ at the end of the line.
    
    Standard_RedLineBuffer was used to load Stl files
(0087714)
user897   
2019-10-02 17:57   
The patch CR30964 is ready to review
(0087715)
kgv   
2019-10-02 18:13   
Please put some comparison numbers for STL reader here.
(0087716)
kgv   
2019-10-02 18:15   
,
+                        bool theMultiline = false

I think it would be more convenient putting this as class field with getter/setter instead of method argument.
(0087717)
kgv   
2019-10-02 18:17   
+      if (Line != NULL)
+      {
+        delete [] Line;
+      }

This looks broken.
(0087816)
git   
2019-10-04 15:45   
Branch CR30964_1 has been updated by mzernova.

SHA-1: ac59d773bd7de802b8b3c095b4ab6f0eae903e9e


Detailed log of new commits:

Author: mzernova
Date: Thu Oct 3 16:40:20 2019 +0300

    remarks from kgv

(0087819)
git   
2019-10-04 15:55   
Branch CR30964_2 has been created by mzernova.

SHA-1: a51c3f01780df30d2792240e076b45908fdfb70f


Detailed log of new commits:

Author: mzernova
Date: Tue Sep 17 16:05:05 2019 +0300

    0030964: Data Exchange - use Standard_ReadLineBuffer within OBJ reader
    
    Standard_ReadLineBuffer now supports a processing of the special multi-line case with \ at the end of the line.
    
    Standard_RedLineBuffer was used to load Stl files
(0087822)
user897   
2019-10-04 16:14   
Time spent reading stl file:
    master: read in 211.481 s
    prototype: read in 89.087 s
(0087824)
user897   
2019-10-04 16:16   
Please review the branch CR30964_2
(0087883)
abv   
2019-10-06 09:29   
Kirill, please comment on why Standard_ReadLineBuffer is placed to package Standard instead of OSD or NCollection where we usually put this kind of tools?
(0087885)
kgv   
2019-10-06 09:42   
(edited on: 2019-10-06 09:44)
> Kirill, please comment on why Standard_ReadLineBuffer
> is placed to package Standard
> instead of OSD or NCollection where we usually put this kind of tools?
Because
- Standard_ArrayStreamBuffer and Standard_ReadBuffer of similar kind are already in Standard,
- they are not Operating-System-Dependent to be placed into OSD
- and they are not much templates to be placed into NCollection.
One can say there is also FSD (File-System-Dependent), but this package has been used for very strange classes so far.

(0088259)
git   
2019-10-17 15:29   
Branch CR30964_2 has been updated by mzernova.

SHA-1: d7ae4f6fed57aa859834073ad4067151f530105b


Detailed log of new commits:

Author: mzernova
Date: Thu Oct 17 15:19:19 2019 +0300

    remarks from osa

(0088260)
git   
2019-10-17 15:30   
Branch CR30964_3 has been created by mzernova.

SHA-1: 079d222802b6f1ddb0c98c928bbbb340647b334c


Detailed log of new commits:

Author: mzernova
Date: Tue Sep 17 16:05:05 2019 +0300

    0030964: Data Exchange - use Standard_ReadLineBuffer within OBJ reader
    
    Standard_ReadLineBuffer now supports a processing of the special multi-line case with \ at the end of the line.
    
    Standard_RedLineBuffer was used to load Stl files
(0088419)
osa   
2019-10-22 14:16   
>> if (isMultiline && myIsMultilineMode)
Check only isMultiline (it is true only when myIsMultilineMode is true)

>> if (myReadBuffer[myBufferPos] == '\n')
this condition should be as else if?

>> bool GetMultilineMode () const { return myIsMultilineMode; }
not protected

>> const Standard_Integer theMaxLineLen = 256
keep this input parameter and use it in Standard_ReadLineBuffer aBuffer (256); but not const value

>>Standard_Boolean isEmptyLine = false;
rename to isEOF

>>Standard_Real x, y, z;
use gp_XYZ aReadVertex
(0088630)
git   
2019-10-28 12:15   
Branch CR30964_3 has been updated by mzernova.

SHA-1: 41bf138b18e8e726d4561dba4154be985461fc15


Detailed log of new commits:

Author: mzernova
Date: Mon Oct 28 12:11:10 2019 +0300

    remarks from osa

(0088631)
user897   
2019-10-28 12:17   
Please review the branch CR30964_3
(0088641)
kgv   
2019-10-28 22:55   
Please don't forget switching bug state and assignee when asking for review.
(0088645)
user897   
2019-10-29 12:35   
The patch CR30964 is ready to review
(0088728)
git   
2019-11-05 14:43   
Branch CR30964_3 has been updated by mzernova.

SHA-1: d899fa273a9b0ec9fe3feda14f09eaf47e7e0a28


Detailed log of new commits:

Author: mzernova
Date: Fri Nov 1 12:03:14 2019 +0300

    now only sequences of the form \\n and \\r\n are deleted

(0088729)
user897   
2019-11-05 14:50   
Please review the branch CR30964_3
(0088730)
kgv   
2019-11-05 14:59   
  33     // allocate read buffer
  34     myReadBuffer.resize (theMaxBufferSizeBytes);
...
  84         // read new chunk from the stream
  85         if (!readStream (theStream, myReadBuffer.size(), myBytesLastRead))
...
 124           else if (myReadBuffer[myBufferPos + 1] == '\n')
 125           {
 126             myReadBuffer.erase (myReadBuffer.begin() + myBufferPos, myReadBuffer.begin() + myBufferPos 
+ 2);

As readStream() fills entire myReadBuffer (and there is no extra garanteed '\0' tail in myReadBuffer), I have some doubts that myReadBuffer[myBufferPos+1] and myReadBuffer[myBufferPos+2] are protected from out-of-range fetches.
Please check and handle correctly, if issue exists.
(0088769)
git   
2019-11-07 10:48   
Branch CR30964_4 has been created by osa.

SHA-1: ee8c0ff78b0658129b8e8d30ebe9f650e2aa52ad


Detailed log of new commits:

Author: mzernova
Date: Tue Sep 17 16:05:05 2019 +0300

    0030964: Data Exchange - use Standard_ReadLineBuffer within OBJ reader
    
    Standard_ReadLineBuffer now supports a processing of the special multi-line case with \ at the end of the line.
    
    Standard_RedLineBuffer was used to load Stl files
(0088786)
git   
2019-11-08 11:27   
Branch CR30964_3 has been updated forcibly by mzernova.

SHA-1: a62dd29246b17f2bdfc894f60d23d869c5297211
(0088787)
git   
2019-11-08 11:28   
Branch CR30964_4 has been updated forcibly by mzernova.

SHA-1: 68abbaa061bbfa1902112af58f2aa38dee0df8e0
(0088811)
mzernova   
2019-11-08 17:20   
Please review the branch CR30964_4