From 5b331b8b7af6736849ca47738d0d6e287bde7152 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 11:16:22 +0000 Subject: [PATCH 1/3] Initial plan From 2b371fe1d075bb4ca53ef464842a50107bb7e83a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 11:30:46 +0000 Subject: [PATCH 2/3] Implement binary store compression support in CStoreHelper Co-authored-by: jakesmith <902700+jakesmith@users.noreply.github.com> --- dali/base/dasds.cpp | 88 +++++++++++++++++++++++++++++++++++++-------- dali/base/dasds.hpp | 2 ++ 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/dali/base/dasds.cpp b/dali/base/dasds.cpp index e5baca8418f..9d2dfc3a11a 100644 --- a/dali/base/dasds.cpp +++ b/dali/base/dasds.cpp @@ -35,6 +35,8 @@ #include "dautils.hpp" #include "dadfs.hpp" #include "jmetrics.hpp" +#include "jlzw.hpp" +#include "jstream.hpp" #define DEBUG_DIR "debug" #define DEFAULT_KEEP_LASTN_STORES 10 // should match default in dali.xsd @@ -1816,11 +1818,14 @@ struct CStoreInfo { unsigned xmlCrc{0}; unsigned binaryCrc{0}; + bool binaryCompressed{false}; + // padding for struct alignment + char padding[3]{}; } crcInfo; StringAttr cache; - static void save(IFileIO *fileIO, unsigned *crcXml, unsigned *crcBinary) + static void save(IFileIO *fileIO, unsigned *crcXml, unsigned *crcBinary, bool binaryCompressed = false) { assertex(fileIO); @@ -1829,6 +1834,7 @@ struct CStoreInfo crcInfo.xmlCrc = *crcXml; if (crcBinary) crcInfo.binaryCrc = *crcBinary; + crcInfo.binaryCompressed = binaryCompressed; fileIO->write(0, sizeof(CrcInfo), &crcInfo); } @@ -1836,14 +1842,20 @@ struct CStoreInfo { assertex(fileIO); - // Only restore xmlCrc and binaryCrc + // Handle backward compatibility with old format size32_t sz = fileIO->read(0, sizeof(CrcInfo), &crcInfo); switch(sz) { - case sizeof(CrcInfo): + case sizeof(CrcInfo): // New format with compression flag break; - case sizeof(unsigned): + case sizeof(unsigned) * 2: // Old format with just CRCs + crcInfo.binaryCompressed = false; + memset(crcInfo.padding, 0, sizeof(crcInfo.padding)); + break; + case sizeof(unsigned): // Very old format with just XML CRC crcInfo.binaryCrc = 0; + crcInfo.binaryCompressed = false; + memset(crcInfo.padding, 0, sizeof(crcInfo.padding)); break; default: crcInfo = {}; @@ -5302,7 +5314,7 @@ class CStoreHelper : implements IStoreHelper, public CInterface } } - void writeStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo) + void writeStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo, bool binaryCompressed = false) { assertex(storeInfo); @@ -5314,17 +5326,17 @@ class CStoreHelper : implements IStoreHelper, public CInterface OwnedIFile iFile = createIFile(path.str()); OwnedIFileIO iFileIO = iFile->open(IFOcreate); - storeInfo->save(iFileIO, crcXml, crcBinary); + storeInfo->save(iFileIO, crcXml, crcBinary, binaryCompressed); storeInfo->cache.set(filename.str()); iFileIO->close(); } - void updateStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo) + void updateStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo, bool binaryCompressed = false) { assertex(storeInfo); clearStoreInfo(base, location, edition, storeInfo); - writeStoreInfo(base, location, edition, crcXml, crcBinary, storeInfo); + writeStoreInfo(base, location, edition, crcXml, crcBinary, storeInfo, binaryCompressed); } void refreshInfo(CStoreInfo &info, const char *base) @@ -5714,7 +5726,30 @@ class CStoreHelper : implements IStoreHelper, public CInterface Owned serialStream = createSerialOutputStream(iFileIOTmpStore); Owned crcSerialStream = createCrcOutputStream(serialStream); Owned bufOutStream = createBufferedOutputStream(crcSerialStream, bufferSize); - root->serializeToStream(*bufOutStream); + + // Check if binary compression is enabled + if (configFlags & SH_CompressBinary) + { + Owned compressor = getCompressor("LZ4"); + if (compressor) + { + LOG(MCdebugProgress, "Using LZ4 compression for binary store"); + Owned compressedStream = createCompressingOutputStream(bufOutStream, compressor); + root->serializeToStream(*compressedStream); + compressedStream->flush(); + compressedStream.clear(); + } + else + { + WARNLOG("Failed to create compressor, saving uncompressed"); + root->serializeToStream(*bufOutStream); + } + } + else + { + root->serializeToStream(*bufOutStream); + } + bufOutStream->flush(); bufOutStream.clear(); crcSerialStream->flush(); @@ -5882,7 +5917,8 @@ class CStoreHelper : implements IStoreHelper, public CInterface } } clearStoreInfo(storeFileName, location, 0, NULL); - writeStoreInfo(storeFileName, location, newEdition, &xmlCrc, binaryCrcPtr, &storeInfo); // binaryCrcPtr could be nullptr if the binary store save failed + bool binaryCompressed = (configFlags & SH_CompressBinary) && binaryCrcPtr; + writeStoreInfo(storeFileName, location, newEdition, &xmlCrc, binaryCrcPtr, &storeInfo, binaryCompressed); // binaryCrcPtr could be nullptr if the binary store save failed try { @@ -5900,7 +5936,8 @@ class CStoreHelper : implements IStoreHelper, public CInterface } clearStoreInfo(storeFileName, remoteBackupLocation, 0, NULL); - writeStoreInfo(storeFileName, remoteBackupLocation, newEdition, &xmlCrc, binaryCrcPtr, &storeInfo); // binaryCrcPtr could be nullptr if the binary store save failed + bool binaryCompressed = (configFlags & SH_CompressBinary) && binaryCrcPtr; + writeStoreInfo(storeFileName, remoteBackupLocation, newEdition, &xmlCrc, binaryCrcPtr, &storeInfo, binaryCompressed); // binaryCrcPtr could be nullptr if the binary store save failed PROGLOG("Copy store done"); } } @@ -6029,6 +6066,11 @@ class CStoreHelper : implements IStoreHelper, public CInterface backupLocation.append(remoteBackupLocation); return backupLocation; } + virtual bool isBinaryCompressed() override + { + refreshStoreInfo(); + return storeInfo.crcInfo.binaryCompressed; + } friend struct CheckDeltaBlock; }; @@ -6342,8 +6384,26 @@ CServerRemoteTree *CCovenSDSManager::loadStoreType(StoreFormat storeFormat, size Owned progressStream = createProgressStream(serialStream, 0, fSize, "Load progress", 60); Owned crcSerialStream = createCrcInputStream(progressStream); Owned bufInStream = createBufferedInputStream(crcSerialStream, bufferSize); + + // Check if binary is compressed and apply decompression if needed + Owned finalInputStream = bufInStream; + if (isBinary && iStoreHelper->isBinaryCompressed()) + { + LOG(MCdebugProgress, "Decompressing binary store using LZ4"); + Owned expander = getExpander("LZ4"); + if (expander) + { + Owned decompressedStream = createDecompressingInputStream(bufInStream, expander); + finalInputStream.setown(createBufferedInputStream(decompressedStream, bufferSize)); + } + else + { + WARNLOG("Failed to create expander for compressed binary store"); + } + } + if (isBinary) - result.setown(createPTreeFromBinary(*bufInStream, nodeCreator)); + result.setown(createPTreeFromBinary(*finalInputStream, nodeCreator)); else { Owned treeMaker = createPTreeMaker(ipt_none, nullptr, nodeCreator); @@ -6360,11 +6420,11 @@ CServerRemoteTree *CCovenSDSManager::loadStoreType(StoreFormat storeFormat, size } }; - Owned wrapper = new CSerialInputStreamWrapper(bufInStream); + Owned wrapper = new CSerialInputStreamWrapper(finalInputStream); result.setown(createPTree(*wrapper, ipt_none, ptr_ignoreWhiteSpace, treeMaker)); } - bufInStream.clear(); + finalInputStream.clear(); unsigned crc = crcSerialStream->queryCrc(); if (storedCrc && storedCrc != crc) diff --git a/dali/base/dasds.hpp b/dali/base/dasds.hpp index 19891c60a1a..af28017fb47 100644 --- a/dali/base/dasds.hpp +++ b/dali/base/dasds.hpp @@ -267,6 +267,7 @@ interface IStoreHelper : extends IInterface virtual void backup(const char *filename) = 0; virtual StringBuffer &getPrimaryLocation(StringBuffer &location) = 0; virtual StringBuffer &getBackupLocation(StringBuffer &backupLocation) = 0; + virtual bool isBinaryCompressed() = 0; }; enum @@ -275,6 +276,7 @@ enum SH_RecoverFromIncErrors = 0x0002, SH_BackupErrorFiles = 0x0004, SH_CheckNewDelta = 0x0008, + SH_CompressBinary = 0x0010, }; extern da_decl IStoreHelper *createStoreHelper(const char *storeName, const char *location, const char *remoteBackupLocation, unsigned configFlags, unsigned keepStores=0, unsigned delay=5000, const bool *abort=nullptr, bool saveBinary=false); extern da_decl bool applyXmlDeltas(IPropertyTree &root, IIOStream &stream, bool stopOnError=false); From 5abace85435c14116cb0de22e27308bc435fe6fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 14:10:25 +0000 Subject: [PATCH 3/3] Address review feedback: use unsigned flags, remove default params, fix compression code structure Co-authored-by: jakesmith <902700+jakesmith@users.noreply.github.com> --- dali/base/dasds.cpp | 55 ++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/dali/base/dasds.cpp b/dali/base/dasds.cpp index 9d2dfc3a11a..2dca252598c 100644 --- a/dali/base/dasds.cpp +++ b/dali/base/dasds.cpp @@ -1818,14 +1818,16 @@ struct CStoreInfo { unsigned xmlCrc{0}; unsigned binaryCrc{0}; - bool binaryCompressed{false}; - // padding for struct alignment - char padding[3]{}; + unsigned flags{0}; // bit flags for future expandability } crcInfo; StringAttr cache; + enum : unsigned + { + FLAG_BINARY_COMPRESSED = 0x0001 + }; - static void save(IFileIO *fileIO, unsigned *crcXml, unsigned *crcBinary, bool binaryCompressed = false) + static void save(IFileIO *fileIO, unsigned *crcXml, unsigned *crcBinary, bool binaryCompressed) { assertex(fileIO); @@ -1834,7 +1836,8 @@ struct CStoreInfo crcInfo.xmlCrc = *crcXml; if (crcBinary) crcInfo.binaryCrc = *crcBinary; - crcInfo.binaryCompressed = binaryCompressed; + if (binaryCompressed) + crcInfo.flags |= FLAG_BINARY_COMPRESSED; fileIO->write(0, sizeof(CrcInfo), &crcInfo); } @@ -1846,16 +1849,14 @@ struct CStoreInfo size32_t sz = fileIO->read(0, sizeof(CrcInfo), &crcInfo); switch(sz) { - case sizeof(CrcInfo): // New format with compression flag + case sizeof(CrcInfo): // Current format with flags break; case sizeof(unsigned) * 2: // Old format with just CRCs - crcInfo.binaryCompressed = false; - memset(crcInfo.padding, 0, sizeof(crcInfo.padding)); + crcInfo.flags = 0; break; case sizeof(unsigned): // Very old format with just XML CRC crcInfo.binaryCrc = 0; - crcInfo.binaryCompressed = false; - memset(crcInfo.padding, 0, sizeof(crcInfo.padding)); + crcInfo.flags = 0; break; default: crcInfo = {}; @@ -5314,7 +5315,7 @@ class CStoreHelper : implements IStoreHelper, public CInterface } } - void writeStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo, bool binaryCompressed = false) + void writeStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo, bool binaryCompressed) { assertex(storeInfo); @@ -5332,7 +5333,7 @@ class CStoreHelper : implements IStoreHelper, public CInterface iFileIO->close(); } - void updateStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo, bool binaryCompressed = false) + void updateStoreInfo(const char *base, const char *location, unsigned edition, unsigned *crcXml, unsigned *crcBinary, CStoreInfo *storeInfo, bool binaryCompressed) { assertex(storeInfo); clearStoreInfo(base, location, edition, storeInfo); @@ -5408,7 +5409,7 @@ class CStoreHelper : implements IStoreHelper, public CInterface wcard.append(base).append(".*"); Owned dIter = createDirectoryIterator(location, wcard.str()); if (!dIter->first()) - updateStoreInfo(base, location, 0, nullptr, nullptr, &info); + updateStoreInfo(base, location, 0, nullptr, nullptr, &info, false); else if (dIter->next()) throw MakeStringException(0, "Multiple store.X files - only one corresponding to latest dalisds.xml should exist"); } @@ -5725,7 +5726,6 @@ class CStoreHelper : implements IStoreHelper, public CInterface { Owned serialStream = createSerialOutputStream(iFileIOTmpStore); Owned crcSerialStream = createCrcOutputStream(serialStream); - Owned bufOutStream = createBufferedOutputStream(crcSerialStream, bufferSize); // Check if binary compression is enabled if (configFlags & SH_CompressBinary) @@ -5734,24 +5734,34 @@ class CStoreHelper : implements IStoreHelper, public CInterface if (compressor) { LOG(MCdebugProgress, "Using LZ4 compression for binary store"); - Owned compressedStream = createCompressingOutputStream(bufOutStream, compressor); - root->serializeToStream(*compressedStream); - compressedStream->flush(); - compressedStream.clear(); + Owned stream = createBufferedOutputStream(crcSerialStream, bufferSize); + Owned compressed = createCompressingOutputStream(stream, compressor); + Owned bufOutStream = createBufferedOutputStream(compressed, bufferSize, false); + root->serializeToStream(*bufOutStream); + bufOutStream->flush(); + bufOutStream.clear(); + compressed->flush(); + compressed.clear(); + stream->flush(); + stream.clear(); } else { WARNLOG("Failed to create compressor, saving uncompressed"); + Owned bufOutStream = createBufferedOutputStream(crcSerialStream, bufferSize); root->serializeToStream(*bufOutStream); + bufOutStream->flush(); + bufOutStream.clear(); } } else { + Owned bufOutStream = createBufferedOutputStream(crcSerialStream, bufferSize); root->serializeToStream(*bufOutStream); + bufOutStream->flush(); + bufOutStream.clear(); } - bufOutStream->flush(); - bufOutStream.clear(); crcSerialStream->flush(); crc = crcSerialStream->queryCrc(); crcSerialStream.clear(); @@ -5936,7 +5946,6 @@ class CStoreHelper : implements IStoreHelper, public CInterface } clearStoreInfo(storeFileName, remoteBackupLocation, 0, NULL); - bool binaryCompressed = (configFlags & SH_CompressBinary) && binaryCrcPtr; writeStoreInfo(storeFileName, remoteBackupLocation, newEdition, &xmlCrc, binaryCrcPtr, &storeInfo, binaryCompressed); // binaryCrcPtr could be nullptr if the binary store save failed PROGLOG("Copy store done"); } @@ -6069,7 +6078,7 @@ class CStoreHelper : implements IStoreHelper, public CInterface virtual bool isBinaryCompressed() override { refreshStoreInfo(); - return storeInfo.crcInfo.binaryCompressed; + return (storeInfo.crcInfo.flags & CStoreInfo::FLAG_BINARY_COMPRESSED) != 0; } friend struct CheckDeltaBlock; }; @@ -6398,7 +6407,7 @@ CServerRemoteTree *CCovenSDSManager::loadStoreType(StoreFormat storeFormat, size } else { - WARNLOG("Failed to create expander for compressed binary store"); + throw MakeStringException(0, "Failed to create expander for compressed binary store"); } }