[Libreoffice-commits] core.git: sal/osl

[Noel Grandin (via logerrit)]

 sal/osl/unx/file.cxx |    8 +++++---
 sal/osl/w32/file.cxx |    9 ++++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

New commits:
commit 0d04315c17a6df9f971237d45d9e5e8af765dd17
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Mon Jun 3 11:27:17 2019 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Mon Jun 3 15:23:16 2019 +0200

    tdf#67538 XTypeDetection::queryTypeByDescriptor poor performance, part4
    
    WPXSvInputStreamImpl was hammering on getFilePos pretty hard, and
    getFilePos uses a mutex, which is slow when it is called from every
    single read. So switch to using std::atomic to access position.
    
    This is specifically fixing the performance of queryTypeByDescriptor
    when called from a basic macro on a local test file.
    
    This takes my test macro from 8s to 4s.
    
    Change-Id: Iab707a374359e2ee0e92425b2d9a903d67cb53d4
    Reviewed-on: https://gerrit.libreoffice.org/73377
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/sal/osl/unx/file.cxx b/sal/osl/unx/file.cxx
index 1f04802aa9e6..0ee0b8887dd4 100644
--- a/sal/osl/unx/file.cxx
+++ b/sal/osl/unx/file.cxx
@@ -36,6 +36,7 @@
 #include "unixerrnostring.hxx"
 
 #include <algorithm>
+#include <atomic>
 #include <cassert>
 #include <limits>
 
@@ -84,7 +85,8 @@ struct FileHandle_Impl
 
     sal_uInt64   m_size;    /*< file size */
     off_t        m_offset;  /*< physical offset from begin of file */
-    off_t        m_fileptr; /*< logical offset from begin of file */
+    // m_fileptr is hit hard in some situations, where the overhead of a mutex starts to show up, so use an atomic
+    std::atomic<off_t> m_fileptr; /*< logical offset from begin of file */
 
     off_t        m_bufptr;  /*< buffer offset from begin of file */
     size_t       m_buflen;  /*< buffer filled [0, m_bufsiz - 1] */
@@ -224,7 +226,7 @@ size_t FileHandle_Impl::getpagesize()
 
 sal_uInt64 FileHandle_Impl::getPos() const
 {
-    return sal::static_int_cast< sal_uInt64 >(m_fileptr);
+    return sal::static_int_cast< sal_uInt64 >(m_fileptr.load());
 }
 
 void FileHandle_Impl::setPos(sal_uInt64 uPos)
@@ -1421,7 +1423,7 @@ oslFileError SAL_CALL osl_getFilePos(oslFileHandle Handle, sal_uInt64* pPos)
     if ((!pImpl) || ((pImpl->m_kind == FileHandle_Impl::KIND_FD) && (pImpl->m_fd == -1)) || (!pPos))
         return osl_File_E_INVAL;
 
-    FileHandle_Impl::Guard lock(&(pImpl->m_mutex));
+    // no need to lock because pos is atomic
     *pPos = pImpl->getPos();
 
     return osl_File_E_None;
diff --git a/sal/osl/w32/file.cxx b/sal/osl/w32/file.cxx
index a3600844fd29..7fda6107adeb 100644
--- a/sal/osl/w32/file.cxx
+++ b/sal/osl/w32/file.cxx
@@ -29,6 +29,7 @@
 #include "file_url.hxx"
 #include "file_error.hxx"
 
+#include <atomic>
 #include <cassert>
 #include <algorithm>
 #include <limits>
@@ -60,7 +61,8 @@ struct FileHandle_Impl
 
     sal_uInt64    m_size;    /*< file size */
     LONGLONG      m_offset;  /*< physical offset from begin of file */
-    LONGLONG      m_filepos; /*< logical offset from begin of file */
+    // m_filepos is hit hard in some situations, where the overhead of a mutex starts to show up, so use an atomic
+    std::atomic<LONGLONG> m_filepos; /*< logical offset from begin of file */
 
     LONGLONG      m_bufptr;  /*< buffer offset from begin of file */
     SIZE_T        m_buflen;  /*< buffer filled [0, m_bufsiz - 1] */
@@ -185,7 +187,7 @@ SIZE_T FileHandle_Impl::getpagesize()
 
 sal_uInt64 FileHandle_Impl::getPos() const
 {
-    return sal::static_int_cast< sal_uInt64 >(m_filepos);
+    return sal::static_int_cast< sal_uInt64 >(m_filepos.load());
 }
 
 oslFileError FileHandle_Impl::setPos(sal_uInt64 uPos)
@@ -946,8 +948,9 @@ oslFileError SAL_CALL osl_getFilePos(oslFileHandle Handle, sal_uInt64 *pPos)
     if ((!pImpl) || !IsValidHandle(pImpl->m_hFile) || (!pPos))
         return osl_File_E_INVAL;
 
-    FileHandle_Impl::Guard lock(&(pImpl->m_mutex));
+    // no need to lock because pos is atomic
     *pPos = pImpl->getPos();
+
     return osl_File_E_None;
 }