View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0032979Open CASCADEOCCT:Data Exchangepublic2022-05-17 10:492023-03-19 20:13
Reporterkgv Assigned Tosmoskvin 
PrioritynormalSeverityfeature 
Status closedResolutionfixed 
Target Version7.7.0Fixed in Version7.7.0 
Summary0032979: Data Exchange, RWGltf_CafWriter - support multi-threaded Draco compression
DescriptionDraco compression might be time-consuming on large models. Multi-threaded encoding might be helpful.
TagsNo tags attached.
Test case numberde_mesh/gltf_write/bull_parallel

Attached Files

  • performance.JPG (85,219 bytes)
  • performance_new.JPG (85,301 bytes)

Activities

msv

2022-05-18 17:20

developer   ~0108490

It might be useful to add the boolean option MultiThread to DracoParameters, and involve multi-threading only when it is on.

git

2022-07-26 14:08

administrator   ~0109999

Branch CR32979 has been created by ichesnokova.

SHA-1: fb2c70cc7076988c1ca16e4486742dffc9df8c5b


Detailed log of new commits:

Author: ichesnok
Date: Tue Jul 26 14:06:58 2022 +0300

    0032979: Data Exchange, RWGltf_CafWriter - support multi-threaded Draco compression
    
    'MultiThread' field was added to structure RWGltf_DracoParameters for using multithreading.
    Class CafWriter_DracoEncodingFunctor was added for multithreaded compression.

git

2022-07-27 09:49

administrator   ~0110013

Branch CR32979 has been updated forcibly by ichesnokova.

SHA-1: f89bdae5d445bbe9d3dc36ec0df08164472f6489

git

2022-07-28 08:53

administrator   ~0110028

Branch CR32979 has been updated forcibly by ichesnokova.

SHA-1: 927e4f5632b464d86801ba9623d2505a93b672ca

git

2022-07-28 14:34

administrator   ~0110055

Branch CR32979 has been updated forcibly by ichesnokova.

SHA-1: a00c3ac237e97f3d79b8dcdd87ca75fa619c282d

ichesnokova

2022-07-29 16:34

developer   ~0110075

Branch CR32979 is ready for review.
Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/ALL/

kgv

2022-07-29 16:53

developer   ~0110078

@ichesnokova,
It could be useful adding at least one test case for verifying multi-threaded mode.

Attaching performance comparison results would be also interesting.

-    aDracoEncoder.SetSpeedOptions (myDracoParameters.CompressionLevel, myDracoParameters.CompressionLevel);
+    aDracoEncoder.SetSpeedOptions(myDracoParameters.CompressionLevel, myDracoParameters.CompressionLevel);
...
-        Message::SendFail (TCollection_AsciiString("File '") + myBinFileNameFull + "' cannot be written");
+        Message::SendFail(TCollection_AsciiString("File '") + myBinFileNameFull + "' cannot be written");

Please revert unrelated cosmetic changes and re-visit OCCT Coding Style suggestions (space is expected before opening bracket).

+  bool MultiThread;         //!< flag to use multithreading (FALSE by default)

Consider using consistent naming - see similar property in RWGltf_CafReader:
> Standard_Boolean myToParallel; //!< flag to use multithreading; FALSE by default
> .
> //! Return TRUE if multithreaded optimizations are allowed; FALSE by default.
> bool ToParallel() const { return myToParallel; }

Not sure if it is for good or bad putting this flag into Draco-specific options instead of a `RWGltf_CafWriter`, @msv please share your suggestion.

 
+class RWGltf_CafWriter::CafWriter_DracoEncodingFunctor
+{

Cosmetic description would be useful.

+    draco::Mesh aMesh;
+    if (aCurrentMesh->NodesVec.empty())
+    {
+      return;
+    }

It looks strange to declare an unused variable `aMesh` before termination condition.

+#include <OSD_ThreadPool.hxx>
 #include <OSD_Timer.hxx>

Alphabetical order is preferred ('i' precedes 'h').

msv

2022-07-29 17:15

developer   ~0110082

>Alphabetical order is preferred ('i' precedes 'h').
You are mistaking. 'h' precedes 'i'.

kgv

2022-07-29 17:21

developer   ~0110083

@msv, my bad.

msv

2022-08-01 11:53

developer   ~0110094

src/RWGltf/RWGltf_CafWriter.hxx
There is no necessity to put the class CafWriter_DracoEncodingFunctor into RWGltf_CafWriter. Please revert this change.

src/RWGltf/RWGltf_CafWriter.cxx
174: Name the class DracoEncodingFunctor and put it into unnamed namespace.
179: This parameter should be const.
212: You are using default constructor, then copy constructor. Simplify as:
    std::shared_ptr<draco::EncoderBuffer> anEncoderBuffer = std::make_shared<draco::EncoderBuffer>();

221: concurrent call of the method Next() will lead to data races. You must create a new progress range for each mesh item beforehand. Put mesh and progress range in a structure.
609: aPSentryBin is initialized only for 4 progress units, but now a new progress is added.
869: For this simple case it is better to use OSD_Parallel::For, like this:
    OSD_Parallel::For (0, int(aMeshes.size()), aFunctor, !myDracoParameters.MultiThread);

git

2022-08-02 10:44

administrator   ~0110099

Branch CR32979_1 has been created by ichesnokova.

SHA-1: acf484e0082e69758aa259fe83b84da752fb398c


Detailed log of new commits:

Author: ichesnok
Date: Tue Jul 26 14:06:58 2022 +0300

    0032979: Data Exchange, RWGltf_CafWriter - support multi-threaded Draco compression
    
    'MultiThread' field was added to structure RWGltf_DracoParameters for using multithreading.
    Class CafWriter_DracoEncodingFunctor was added for multithreaded compression.

ichesnokova

2022-08-02 14:07

developer   ~0110105

Branch CR32979_1 is ready for review.
Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/COMPARE/
performance.JPG (85,219 bytes)

kgv

2022-08-02 14:17

developer   ~0110106

@ichesnokova,

you may put preformatted ASCII table directly into comment using HTML tag 'pre'.

msv

2022-08-02 18:38

developer   ~0110116

Please organize progressing like in the class BRepExtrema_DistShapeShape.cxx

git

2022-08-03 10:01

administrator   ~0110119

Branch CR32979_1 has been updated forcibly by ichesnokova.

SHA-1: dd9a6d2d3b2f7bef05a4c0e9bc9062f8cd95248f

ichesnokova

2022-08-03 13:40

developer   ~0110121

Branch CR32979_1 is ready for review.
Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/COMPARE/
performance_new.JPG (85,301 bytes)

msv

2022-08-03 14:02

developer   ~0110122

This is not needed any more:
#include <OSD_ThreadPool.hxx>

kgv

2022-08-03 14:11

developer   ~0110123 

+  Standard_Boolean                              myToParallel;

New class field is left uninitialized

kgv

2022-08-03 14:15

developer   ~0110124 

+  Message_ProgressScope aScope(theProgress, "Write binary data", 2);

2 should be conditional based on draco on/off.

+    if (!aStatus.ok())
+    {
+      Message::SendFail (TCollection_AsciiString("Error: mesh cannot be encoded in draco buffer."));
+      return;

Failure should be propagated to the main interface.

git

2022-08-03 22:37

administrator   ~0110128

Branch CR32979_2 has been created by ichesnokova.

SHA-1: 21d070b115c03e98bc1b474211aaadfe60241b69


Detailed log of new commits:

Author: ichesnok
Date: Tue Jul 26 14:06:58 2022 +0300

    0032979: Data Exchange, RWGltf_CafWriter - support multi-threaded Draco compression
    
    'MultiThread' field was added to structure RWGltf_DracoParameters for using multithreading.
    Class CafWriter_DracoEncodingFunctor was added for multithreaded compression.

git

2022-08-04 08:13

administrator   ~0110129

Branch CR32979_2 has been updated forcibly by ichesnokova.

SHA-1: 403277678876c6d09ecff17ff5d2a8091e303577

ichesnokova

2022-08-04 17:54

developer   ~0110137

Branch CR32979_2 is ready for review.
Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/ALL/

kgv

2022-08-05 09:50

developer   ~0110138

Last edited: 2022-08-05 09:51

 
+  : myProgress(theProgress, "Convert meshes to Draco buffer", Max(1, int(theMeshes.size()))),

Title looks unreasonably long to me. I would suggest "Draco compression" instead.

+      aBinFile->write ((*anEncoderBuffers.at(aBuffInd).get()).data(), std::streamsize((*anEncoderBuffers.at(aBuffInd).get()).size()));

const draco::EncoderBuffer& anEncBuff = *anEncoderBuffers.at(aBuffInd);
`aBinFile->write (anEncBuff.data(), std::streamsize(anEncBuff.size()));`

git

2022-08-05 10:54

administrator   ~0110139

Branch CR32979_2 has been updated forcibly by ichesnokova.

SHA-1: ab9b10435378b00e10bbd8356fd29f302a190444

ichesnokova

2022-08-05 16:40

developer   ~0110140

Branch CR32979_2 is ready for review.
Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/ALL/

msv

2022-08-05 21:15

developer   ~0110141

For integration:
occt - CR32979_2
products - none

smoskvin

2022-08-12 10:39

administrator   ~0110207

Compilation problem with the USE_DRACO=OFF parameter

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:180:25: error: use of undeclared identifier 'draco'
                        draco::Encoder& theDracoEncoder,
                        ^

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:182:53: error: use of undeclared identifier 'draco'
                        std::vector<std::shared_ptr<draco::EncoderBuffer>>& theEncoderBuffers)
                                                    ^

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:231:37: error: expected a type
  Message_ProgressScope myProgress;
                                    ^

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:231:37: error: expected ')'
/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:179:24: note: to match this '('
  DracoEncodingFunctor (const Message_ProgressRange& theProgress,
                       ^

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:232:3: error: use of undeclared identifier 'draco'
  draco::Encoder* myDracoEncoder;
  ^

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:235:31: error: use of undeclared identifier 'draco'
  std::vector<std::shared_ptr<draco::EncoderBuffer>>* myEncoderBuffers;
                              ^

/dn61/builds/CR0-WEEK-32_CR0-WEEK-32/OCCT_SRC/src/RWGltf/RWGltf_CafWriter.cxx:235:79: error: expected a type
  std::vector<std::shared_ptr<draco::EncoderBuffer>>* myEncoderBuffers;
                                                                              ^

git

2022-08-12 11:06

administrator   ~0110208

Branch CR32979_2 has been updated forcibly by ichesnokova.

SHA-1: b396f5dfdb501901e497a85e722fbafd97216116

smoskvin

2022-08-14 12:27

administrator   ~0110222

Combination -
OCCT branch : IR-2022-08-12
master SHA - fd5c113a0367cc5e0b086544f2e900265545aa72
e0ceb716c70188b98130b1550914140d0502a6f9
Products branch : IR-2022-08-12 SHA - changes and them, and you can discard any commits you make in this
45cefb797e0ba22a343c7826a817db6b0c5c6332
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: 18719.310000000743 / 18726.600000000624 [-0.04%]
Products
Total CPU difference: 11976.710000000106 / 11968.290000000105 [+0.07%]
Windows-64-VC14:
OCCT
Total CPU difference: 20763.5 / 20741.5 [+0.11%]
Products
Total CPU difference: 13535.640625 / 13528.765625 [+0.05%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2022-08-14 13:54

administrator   ~0110226

Branch CR32979 has been deleted by mnt.

SHA-1: a00c3ac237e97f3d79b8dcdd87ca75fa619c282d

git

2022-08-14 13:54

administrator   ~0110227

Branch CR32979_1 has been deleted by mnt.

SHA-1: dd9a6d2d3b2f7bef05a4c0e9bc9062f8cd95248f

git

2022-08-14 13:54

administrator   ~0110228

Branch CR32979_2 has been deleted by mnt.

SHA-1: b396f5dfdb501901e497a85e722fbafd97216116

Related Changesets

occt: master f74f684b

2022-07-26 14:06:58

ichesnok


Committer: smoskvin Details Diff		 0032979: Data Exchange, RWGltf_CafWriter - support multi-threaded Draco compression

'MultiThread' field was added to structure RWGltf_DracoParameters for using multithreading.
Class CafWriter_DracoEncodingFunctor was added for multithreaded compression.		 Affected Issues
0032979
mod - src/RWGltf/RWGltf_CafWriter.cxx Diff File
mod - src/RWGltf/RWGltf_CafWriter.hxx Diff File
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML.cxx Diff File
add - tests/de_mesh/gltf_write/bull_parallel Diff File

Issue History

Date Modified Username Field Change
2022-05-17 10:49 kgv New Issue
2022-05-17 10:49 kgv Assigned To => ichesnokova
2022-05-18 17:20 msv Note Added: 0108490
2022-05-18 17:21 msv Status new => assigned
2022-07-26 14:08 git Note Added: 0109999
2022-07-27 09:49 git Note Added: 0110013
2022-07-28 08:53 git Note Added: 0110028
2022-07-28 14:34 git Note Added: 0110055
2022-07-29 16:34 ichesnokova Note Added: 0110075
2022-07-29 16:34 ichesnokova Assigned To ichesnokova => msv
2022-07-29 16:34 ichesnokova Status assigned => resolved
2022-07-29 16:53 kgv Note Added: 0110078
2022-07-29 17:15 msv Note Added: 0110082
2022-07-29 17:21 kgv Note Added: 0110083
2022-08-01 11:53 msv Note Added: 0110094
2022-08-01 11:53 msv Assigned To msv => ichesnokova
2022-08-01 11:53 msv Status resolved => assigned
2022-08-02 10:44 git Note Added: 0110099
2022-08-02 14:07 ichesnokova Note Added: 0110105
2022-08-02 14:07 ichesnokova File Added: performance.JPG
2022-08-02 14:08 ichesnokova Assigned To ichesnokova => msv
2022-08-02 14:08 ichesnokova Status assigned => resolved
2022-08-02 14:17 kgv Note Added: 0110106
2022-08-02 18:38 msv Note Added: 0110116
2022-08-02 18:38 msv Assigned To msv => ichesnokova
2022-08-02 18:38 msv Status resolved => assigned
2022-08-03 10:01 git Note Added: 0110119
2022-08-03 13:40 ichesnokova Note Added: 0110121
2022-08-03 13:40 ichesnokova File Added: performance_new.JPG
2022-08-03 13:40 ichesnokova Assigned To ichesnokova => msv
2022-08-03 13:40 ichesnokova Status assigned => resolved
2022-08-03 14:02 msv Note Added: 0110122
2022-08-03 14:02 msv Assigned To msv => ichesnokova
2022-08-03 14:02 msv Status resolved => assigned
2022-08-03 14:11 kgv Note Added: 0110123
2022-08-03 14:15 kgv Note Added: 0110124
2022-08-03 22:37 git Note Added: 0110128
2022-08-04 08:13 git Note Added: 0110129
2022-08-04 17:54 ichesnokova Note Added: 0110137
2022-08-04 17:54 ichesnokova Assigned To ichesnokova => msv
2022-08-04 17:54 ichesnokova Status assigned => resolved
2022-08-05 09:50 kgv Note Added: 0110138
2022-08-05 09:51 kgv Note Edited: 0110138
2022-08-05 10:05 msv Assigned To msv => ichesnokova
2022-08-05 10:05 msv Status resolved => assigned
2022-08-05 10:54 git Note Added: 0110139
2022-08-05 16:40 ichesnokova Note Added: 0110140
2022-08-05 16:40 ichesnokova Assigned To ichesnokova => msv
2022-08-05 16:40 ichesnokova Status assigned => resolved
2022-08-05 21:15 msv Assigned To msv => bugmaster
2022-08-05 21:15 msv Status resolved => reviewed
2022-08-05 21:15 msv Note Added: 0110141
2022-08-08 08:48 ichesnokova Time allocated 8.8.2022: 166 h. => set
2022-08-12 10:29 smoskvin Assigned To bugmaster => ichesnokova
2022-08-12 10:29 smoskvin Status reviewed => assigned
2022-08-12 10:39 smoskvin Note Added: 0110207
2022-08-12 11:06 git Note Added: 0110208
2022-08-12 17:42 ichesnokova Assigned To ichesnokova => bugmaster
2022-08-12 17:42 ichesnokova Status assigned => resolved
2022-08-12 17:59 kgv Status resolved => reviewed
2022-08-14 12:27 smoskvin Status reviewed => tested
2022-08-14 12:27 smoskvin Note Added: 0110222
2022-08-14 12:28 smoskvin Test case number => de_mesh/gltf_write/bull_parallel
2022-08-14 12:58 smoskvin Changeset attached => occt master f74f684b
2022-08-14 12:58 smoskvin Assigned To bugmaster => smoskvin
2022-08-14 12:58 smoskvin Status tested => verified
2022-08-14 12:58 smoskvin Resolution open => fixed
2022-08-14 13:54 git Note Added: 0110226
2022-08-14 13:54 git Note Added: 0110227
2022-08-14 13:54 git Note Added: 0110228
2023-03-19 20:13 vglukhik Status verified => closed
2023-03-19 20:13 vglukhik Fixed in Version => 7.7.0