View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0032979 | Open CASCADE | OCCT:Data Exchange | public | 2022-05-17 10:49 | 2023-03-19 20:13 |
Reporter | kgv | Assigned To | |||
Priority | normal | Severity | feature | ||
Status | closed | Resolution | fixed | ||
Target Version | 7.7.0 | Fixed in Version | 7.7.0 | ||
Summary | 0032979: Data Exchange, RWGltf_CafWriter - support multi-threaded Draco compression | ||||
Description | Draco compression might be time-consuming on large models. Multi-threaded encoding might be helpful. | ||||
Tags | No tags attached. | ||||
Test case number | de_mesh/gltf_write/bull_parallel | ||||
|
It might be useful to add the boolean option MultiThread to DracoParameters, and involve multi-threading only when it is on. |
|
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. |
|
Branch CR32979 has been updated forcibly by ichesnokova. SHA-1: f89bdae5d445bbe9d3dc36ec0df08164472f6489 |
|
Branch CR32979 has been updated forcibly by ichesnokova. SHA-1: 927e4f5632b464d86801ba9623d2505a93b672ca |
|
Branch CR32979 has been updated forcibly by ichesnokova. SHA-1: a00c3ac237e97f3d79b8dcdd87ca75fa619c282d |
|
Branch CR32979 is ready for review. Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/ALL/ |
|
@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`, +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'). |
|
>Alphabetical order is preferred ('i' precedes 'h'). You are mistaking. 'h' precedes 'i'. |
|
|
|
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); |
|
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. |
|
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) |
|
@ichesnokova, you may put preformatted ASCII table directly into comment using HTML tag 'pre'. |
|
Please organize progressing like in the class BRepExtrema_DistShapeShape.cxx |
|
Branch CR32979_1 has been updated forcibly by ichesnokova. SHA-1: dd9a6d2d3b2f7bef05a4c0e9bc9062f8cd95248f |
|
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) |
|
This is not needed any more:#include <OSD_ThreadPool.hxx> |
|
+ Standard_Boolean myToParallel; New class field is left uninitialized |
|
+ 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. |
|
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. |
|
Branch CR32979_2 has been updated forcibly by ichesnokova. SHA-1: 403277678876c6d09ecff17ff5d2a8091e303577 |
|
Branch CR32979_2 is ready for review. Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/ALL/ |
|
+ : 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()));` |
|
Branch CR32979_2 has been updated forcibly by ichesnokova. SHA-1: ab9b10435378b00e10bbd8356fd29f302a190444 |
|
Branch CR32979_2 is ready for review. Tests: http://jenkins-test-occt.nnov.opencascade.com/view/CR32979-master-ichesnok/view/ALL/ |
|
For integration: occt - CR32979_2 products - none |
|
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; ^ |
|
Branch CR32979_2 has been updated forcibly by ichesnokova. SHA-1: b396f5dfdb501901e497a85e722fbafd97216116 |
|
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 |
|
Branch CR32979 has been deleted by mnt. SHA-1: a00c3ac237e97f3d79b8dcdd87ca75fa619c282d |
|
Branch CR32979_1 has been deleted by mnt. SHA-1: dd9a6d2d3b2f7bef05a4c0e9bc9062f8cd95248f |
|
Branch CR32979_2 has been deleted by mnt. SHA-1: b396f5dfdb501901e497a85e722fbafd97216116 |
occt: master f74f684b 2022-07-26 14:06:58 ichesnok Committer: |
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 |
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 |
|
Note Added: 0108490 | |
2022-05-18 17:21 |
|
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 |
|
Note Added: 0110082 | |
2022-07-29 17:21 | kgv | Note Added: 0110083 | |
2022-08-01 11:53 |
|
Note Added: 0110094 | |
2022-08-01 11:53 |
|
Assigned To | msv => ichesnokova |
2022-08-01 11:53 |
|
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 |
|
Note Added: 0110116 | |
2022-08-02 18:38 |
|
Assigned To | msv => ichesnokova |
2022-08-02 18:38 |
|
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 |
|
Note Added: 0110122 | |
2022-08-03 14:02 |
|
Assigned To | msv => ichesnokova |
2022-08-03 14:02 |
|
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 |
|
Assigned To | msv => ichesnokova |
2022-08-05 10:05 |
|
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 |
|
Assigned To | msv => bugmaster |
2022-08-05 21:15 |
|
Status | resolved => reviewed |
2022-08-05 21:15 |
|
Note Added: 0110141 | |
2022-08-08 08:48 | ichesnokova | Time allocated | 8.8.2022: 166 h. => set |
2022-08-12 10:29 |
|
Assigned To | bugmaster => ichesnokova |
2022-08-12 10:29 |
|
Status | reviewed => assigned |
2022-08-12 10:39 |
|
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 |
|
Status | reviewed => tested |
2022-08-14 12:27 |
|
Note Added: 0110222 | |
2022-08-14 12:28 |
|
Test case number | => de_mesh/gltf_write/bull_parallel |
2022-08-14 12:58 |
|
Changeset attached | => occt master f74f684b |
2022-08-14 12:58 |
|
Assigned To | bugmaster => smoskvin |
2022-08-14 12:58 |
|
Status | tested => verified |
2022-08-14 12:58 |
|
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 |