From bf1099e360193e18c29e7b042dcc1480050a4e48 Mon Sep 17 00:00:00 2001
From: Noel Grandin <noel.grandin@collabora.co.uk>
Date: Tue, 27 Aug 2024 14:21:33 +0200
Subject: tdf#158556 reduce allocations in ZipFile

we can allocate these buffers one caller up in ReadCEN and then re-use
them

Change-Id: I07e98168ee2884286f4a1c281acd86e365416149
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172481
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
---
 package/inc/ZipFile.hxx           |  1 +
 package/source/zipapi/ZipFile.cxx | 29 +++++++++++++++++++++--------
 2 files changed, 22 insertions(+), 8 deletions(-)

(limited to 'package')

diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx
index 4704d0e09910..c52bfc7a0366 100644
--- a/package/inc/ZipFile.hxx
+++ b/package/inc/ZipFile.hxx
@@ -93,6 +93,7 @@ private:
     void getSizeAndCRC( sal_Int64 nOffset, sal_Int64 nCompressedSize, sal_Int64 *nSize, sal_Int32 *nCRC );
 
     sal_uInt64 readLOC(ZipEntry &rEntry);
+    sal_uInt64 readLOC_Impl(ZipEntry &rEntry, std::vector<sal_Int8>& rNameBuffer, std::vector<sal_Int8>& rExtraBuffer);
     sal_Int32 readCEN();
     std::tuple<sal_Int64, sal_Int64, sal_Int64> findCentralDirectory();
     void HandlePK34(std::span<const sal_Int8> data, sal_Int64 dataOffset, sal_Int64 totalSize);
diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx
index 706292148da3..7914c772dbb7 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -929,6 +929,16 @@ uno::Reference< XInputStream > ZipFile::getWrappedRawStream(
 }
 
 sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry)
+{
+    ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() );
+    std::vector<sal_Int8> aNameBuffer;
+    std::vector<sal_Int8> aExtraBuffer;
+    return readLOC_Impl(rEntry, aNameBuffer, aExtraBuffer);
+}
+
+// Pass in a shared name buffer to reduce the number of allocations
+// we do when reading the CEN.
+sal_uInt64 ZipFile::readLOC_Impl(ZipEntry &rEntry, std::vector<sal_Int8>& rNameBuffer, std::vector<sal_Int8>& rExtraBuffer)
 {
     ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() );
 
@@ -977,9 +987,9 @@ sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry)
     {
         // read always in UTF8, some tools seem not to set UTF8 bit
         // coverity[tainted_data] - we've checked negative lens, and up to max short is ok here
-        std::vector<sal_Int8> aNameBuffer(nPathLen);
-        sal_Int32 nRead = aGrabber.readBytes(aNameBuffer.data(), nPathLen);
-        std::string_view aNameView(reinterpret_cast<const char *>(aNameBuffer.data()), nRead);
+        rNameBuffer.reserve(nPathLen);
+        sal_Int32 nRead = aGrabber.readBytes(rNameBuffer.data(), nPathLen);
+        std::string_view aNameView(reinterpret_cast<const char *>(rNameBuffer.data()), nRead);
 
         OUString sLOCPath( aNameView.data(), aNameView.size(), RTL_TEXTENCODING_UTF8 );
 
@@ -999,9 +1009,9 @@ sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry)
         ::std::optional<sal_uInt64> oOffset64;
         if (nExtraLen != 0)
         {
-            std::vector<sal_Int8> aExtraBuffer(nExtraLen);
-            aGrabber.readBytes(aExtraBuffer.data(), nExtraLen);
-            MemoryByteGrabber extraMemGrabber(aExtraBuffer.data(), nExtraLen);
+            rExtraBuffer.reserve(nExtraLen);
+            aGrabber.readBytes(rExtraBuffer.data(), nExtraLen);
+            MemoryByteGrabber extraMemGrabber(rExtraBuffer.data(), nExtraLen);
 
             isZip64 = readExtraFields(extraMemGrabber, nExtraLen,
                     nLocSize, nLocCompressedSize, oOffset64, &aNameView);
@@ -1330,7 +1340,8 @@ sal_Int32 ZipFile::readCEN()
             throw ZipException(u"central directory too big"_ustr);
 
         aGrabber.seek(nCenPos);
-        std::vector<sal_Int8> aCENBuffer(nCenLen);
+        std::vector<sal_Int8> aCENBuffer;
+        aCENBuffer.reserve(nCenLen);
         sal_Int64 nRead = aGrabber.readBytes ( aCENBuffer.data(), nCenLen );
         if (nCenLen != nRead)
             throw ZipException (u"Error reading CEN into memory buffer!"_ustr );
@@ -1343,6 +1354,8 @@ sal_Int32 ZipFile::readCEN()
 
         aEntries.reserve(nTotal);
         sal_Int64 nCount;
+        std::vector<sal_Int8> aTempNameBuffer;
+        std::vector<sal_Int8> aTempExtraBuffer;
         for (nCount = 0 ; nCount < nTotal; nCount++)
         {
             sal_Int32 nTestSig = aMemGrabber.ReadInt32();
@@ -1420,7 +1433,7 @@ sal_Int32 ZipFile::readCEN()
             // unfortunately readLOC is required now to check the consistency
             assert(aEntry.nOffset <= 0);
             sal_uInt64 const nStart{ o3tl::make_unsigned(-aEntry.nOffset) };
-            sal_uInt64 const nEnd = readLOC(aEntry);
+            sal_uInt64 const nEnd = readLOC_Impl(aEntry, aTempNameBuffer, aTempExtraBuffer);
             assert(nStart < nEnd);
             for (auto it = unallocated.begin(); ; ++it)
             {
-- 
cgit