From ea815f121c0e60d0ef1e9b4e42a1036b1fb2ad78 Mon Sep 17 00:00:00 2001 From: Martin Weismann Date: Tue, 17 Apr 2018 14:34:07 +0200 Subject: [PATCH] NC-7659: Use libzip callback interface Instead of copiiing the full input stream into memory. This decreases the memory consumption of lib3MF on import by the size of the compressed zip-file. The speed of the import is higher for files with large attachments (e.g. large textures, binary blobs) and not slower for "regular" XML content of 3MFs. --- CMakeLists.txt | 2 +- Include/Common/NMR_ErrorConst.h | 6 ++ Source/Common/NMR_Exception.cpp | 2 + Source/Common/OPC/NMR_OpcPackageReader.cpp | 69 +++++++++++++++++-- .../Platform/NMR_ImportStream_Memory.cpp | 12 +++- .../Reader/NMR_ModelReader_3MF_Native.cpp | 20 ++++-- UnitTests/Source/UnitTest_AllTests.cpp | 2 +- UnitTests/Source/UnitTest_Attachments.cpp | 5 +- 8 files changed, 100 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c323c0e2..a90fbc356 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,7 +9,7 @@ project (lib3MF) # Define Version set(LIB3MF_VERSION_MAJOR 1) # increase on every backward-compatibility breaking change of the API set(LIB3MF_VERSION_MINOR 1) # increase on every backward compatible change of the API -set(LIB3MF_VERSION_MICRO 4) # increase on on every change that does not alter the API +set(LIB3MF_VERSION_MICRO 5) # increase on on every change that does not alter the API set(CMAKE_INSTALL_BINDIR bin CACHE PATH "directory for installing binary files") set(CMAKE_INSTALL_LIBDIR lib CACHE PATH "directory for installing library files") diff --git a/Include/Common/NMR_ErrorConst.h b/Include/Common/NMR_ErrorConst.h index 90b2c3597..6dd7aa987 100644 --- a/Include/Common/NMR_ErrorConst.h +++ b/Include/Common/NMR_ErrorConst.h @@ -275,6 +275,12 @@ NMR_ErrorConst.h defines all error code constants. // ZIP Entry too large for non zip64 zip-file #define NMR_ERROR_ZIPENTRYNON64_TOOLARGE 0x104A +// An individual custom attachment is too large +#define NMR_ERROR_ATTACHMENTTOOLARGE 0x104B + +// Error in zip-callback +#define NMR_ERROR_ZIPCALLBACK 0x104C + /*------------------------------------------------------------------- Core framework error codes (0x2XXX) -------------------------------------------------------------------*/ diff --git a/Source/Common/NMR_Exception.cpp b/Source/Common/NMR_Exception.cpp index 132359315..4c1206ffd 100644 --- a/Source/Common/NMR_Exception.cpp +++ b/Source/Common/NMR_Exception.cpp @@ -124,6 +124,8 @@ namespace NMR { case NMR_ERROR_IMPORTSTREAMISEMPTY: return "An attachment to be read does coes not have any content."; case NMR_ERROR_UUIDGENERATIONFAILED: return "Generation of a UUID failed."; case NMR_ERROR_ZIPENTRYNON64_TOOLARGE: return "A ZIP Entry is too large for non zip64 zip-file"; + case NMR_ERROR_ATTACHMENTTOOLARGE: return "An individual custom attachment is too large."; + case NMR_ERROR_ZIPCALLBACK: return "Error in libzip callback."; // Unhandled exception diff --git a/Source/Common/OPC/NMR_OpcPackageReader.cpp b/Source/Common/OPC/NMR_OpcPackageReader.cpp index f9f33b3aa..0f0da3fa5 100644 --- a/Source/Common/OPC/NMR_OpcPackageReader.cpp +++ b/Source/Common/OPC/NMR_OpcPackageReader.cpp @@ -42,7 +42,60 @@ NMR_OpcPackageReader.cpp defines an OPC Package reader in a portable way. #include namespace NMR { + + // custom callbck function for reading from a CImportStream on the fly + zip_int64_t custom_zip_source_callback(void *userData, void *data, zip_uint64_t len, zip_source_cmd_t cmd) { + if (userData == nullptr) + throw CNMRException(NMR_ERROR_INVALIDPARAM); + + CImportStream* pImportStream = (CImportStream*)(userData); + + switch (cmd) { + case ZIP_SOURCE_SUPPORTS: + zip_int64_t bitmap; + bitmap = zip_source_make_command_bitmap(ZIP_SOURCE_OPEN, ZIP_SOURCE_READ, ZIP_SOURCE_CLOSE, + ZIP_SOURCE_STAT, ZIP_SOURCE_ERROR, ZIP_SOURCE_SEEK, ZIP_SOURCE_TELL, ZIP_SOURCE_SUPPORTS, -1); + return bitmap; + + case ZIP_SOURCE_SEEK: + zip_source_args_seek argsSeek; + argsSeek = * ((zip_source_args_seek *)data); + if (argsSeek.whence == SEEK_SET) + pImportStream->seekPosition(argsSeek.offset, true); + else if (argsSeek.whence == SEEK_CUR) + pImportStream->seekForward(argsSeek.offset, true); + else if (argsSeek.whence == SEEK_END) { + pImportStream->seekFromEnd(argsSeek.offset, true); + } + else + throw CNMRException(0); + return 0; + + case ZIP_SOURCE_OPEN: + return 0; + + case ZIP_SOURCE_READ: + return pImportStream->readBuffer((nfByte*)data, len, true); + + case ZIP_SOURCE_CLOSE: + return 0; + + case ZIP_SOURCE_TELL: + return pImportStream->getPosition(); + case ZIP_SOURCE_STAT: + zip_stat_t* zipStat; + zipStat = (zip_stat_t*)data; + zip_stat_init(zipStat); + zipStat->size = pImportStream->retrieveSize(); + zipStat->valid |= ZIP_STAT_SIZE; + return sizeof(zip_stat_t); + + default: + throw CNMRException(NMR_ERROR_ZIPCALLBACK); + } + return -1; + } COpcPackageReader::COpcPackageReader(_In_ PImportStream pImportStream, _In_ PModelReaderWarnings pWarnings) { @@ -65,13 +118,19 @@ namespace NMR { if (nStreamSize == 0) throw CNMRException(NMR_ERROR_COULDNOTGETSTREAMPOSITION); - // read ZIP into memory - m_Buffer.resize ((size_t) nStreamSize); - pImportStream->readBuffer(&m_Buffer[0], nStreamSize, true); - // create ZIP objects zip_error_init(&m_ZIPError); - m_ZIPsource = zip_source_buffer_create(&m_Buffer[0], (size_t) nStreamSize, 0, &m_ZIPError); + + if (true) { + // read ZIP from callback: faster and requires less memory + m_ZIPsource = zip_source_function_create(custom_zip_source_callback, pImportStream.get(), &m_ZIPError); + } + else { + // read ZIP into memory + m_Buffer.resize((size_t)nStreamSize); + pImportStream->readBuffer(&m_Buffer[0], nStreamSize, true); + m_ZIPsource = zip_source_buffer_create(&m_Buffer[0], (size_t)nStreamSize, 0, &m_ZIPError); + } if (m_ZIPsource == nullptr) throw CNMRException(NMR_ERROR_COULDNOTREADZIPFILE); diff --git a/Source/Common/Platform/NMR_ImportStream_Memory.cpp b/Source/Common/Platform/NMR_ImportStream_Memory.cpp index f56165d97..1cfeb389a 100644 --- a/Source/Common/Platform/NMR_ImportStream_Memory.cpp +++ b/Source/Common/Platform/NMR_ImportStream_Memory.cpp @@ -59,7 +59,12 @@ namespace NMR { // Retrieve Capacity and allocate buffer. nfUint64 cbCapacity = cbBytesToCopy; - m_Buffer.resize((size_t) cbCapacity); + try { + m_Buffer.resize((size_t)cbCapacity); + } + catch (std::bad_alloc) { + throw CNMRException(NMR_ERROR_INVALIDBUFFERSIZE); + } m_cbSize = 0; m_nPosition = 0; @@ -151,6 +156,11 @@ namespace NMR { nfBool CImportStream_Memory::seekFromEnd(_In_ nfUint64 cbBytes, _In_ nfBool bHasToSucceed) { + // all seekFromEnd functions follow the calling conventions of fseek. + // fseek expects a negative number to seek from the end (SEEK_END). + // The program logic in this function requires a positive offset from the end. + cbBytes = 1+~cbBytes; + if (cbBytes > m_cbSize) { if (bHasToSucceed) throw CNMRException(NMR_ERROR_COULDNOTSEEKSTREAM); diff --git a/Source/Model/Reader/NMR_ModelReader_3MF_Native.cpp b/Source/Model/Reader/NMR_ModelReader_3MF_Native.cpp index 9c3c8a4ed..bccc86451 100644 --- a/Source/Model/Reader/NMR_ModelReader_3MF_Native.cpp +++ b/Source/Model/Reader/NMR_ModelReader_3MF_Native.cpp @@ -151,13 +151,21 @@ namespace NMR { sURI = sTargetPartURIDir + sURI; POpcPackagePart pPart = m_pPackageReader->createPart(sURI); PImportStream pAttachmentStream = pPart->getImportStream(); - PImportStream pMemoryStream = pAttachmentStream->copyToMemory(); - - if (pMemoryStream->retrieveSize() == 0) - m_pWarnings->addException(CNMRException(NMR_ERROR_IMPORTSTREAMISEMPTY), mrwMissingMandatoryValue); + try { + PImportStream pMemoryStream = pAttachmentStream->copyToMemory(); - // Add Attachment Stream to Model - m_pModel->addAttachment(sURI, sRelationShipType, pMemoryStream); + if (pMemoryStream->retrieveSize() == 0) + m_pWarnings->addException(CNMRException(NMR_ERROR_IMPORTSTREAMISEMPTY), mrwMissingMandatoryValue); + + // Add Attachment Stream to Model + m_pModel->addAttachment(sURI, sRelationShipType, pMemoryStream); + } + catch (CNMRException &e) { + if (e.getErrorCode() == NMR_ERROR_INVALIDBUFFERSIZE) + m_pWarnings->addException(CNMRException(NMR_ERROR_ATTACHMENTTOOLARGE), mrwMissingMandatoryValue); + else + throw; + } } } } diff --git a/UnitTests/Source/UnitTest_AllTests.cpp b/UnitTests/Source/UnitTest_AllTests.cpp index f269f319c..a338e714d 100644 --- a/UnitTests/Source/UnitTest_AllTests.cpp +++ b/UnitTests/Source/UnitTest_AllTests.cpp @@ -38,7 +38,7 @@ int main(int argc, char **argv) // testing::GTEST_FLAG(filter) = "*LoadFromMemoryBuffer*"; // testing::GTEST_FLAG(filter) = "*Production_LoadFileTest*ReadFiles*"; // testing::GTEST_FLAG(filter) = "*ProgressCallback*"; - // testing::GTEST_FLAG(filter) = "*WriteBeamLattice_Negative*"; + // testing::GTEST_FLAG(filter) = "*Attachments*ReadAttachment*"; testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/UnitTests/Source/UnitTest_Attachments.cpp b/UnitTests/Source/UnitTest_Attachments.cpp index 832ee3070..6c809b912 100644 --- a/UnitTests/Source/UnitTest_Attachments.cpp +++ b/UnitTests/Source/UnitTest_Attachments.cpp @@ -87,7 +87,6 @@ namespace NMR { << L"Could not read attachment from file"; } - ASSERT_EQ(NMR::lib3mf_model_addcustomcontenttypeutf8(pModel.get(), "xml", "application/xml"), S_OK) << L"Could not add custom contenttype attachment from buffer"; @@ -151,14 +150,12 @@ namespace NMR { if (hResult != S_OK) { DWORD errorCode; LPCSTR errString; - hResult = lib3mf_getlasterror(p3MFReader.get(), &errorCode, &errString); + lib3mf_getlasterror(p3MFReader.get(), &errorCode, &errString); std::string errorMessage = std::string(errString); } ASSERT_EQ(hResult, S_OK) << L"Could not read 3MF file."; } - Read3MFAttachments(pModel); - } };