commit 49a17e9ba500451929a2c4cb63c30e50e989886c
author Noel Grandin <noel.grandin@collabora.co.uk> Mon Jun 03 11:27:17 2019 +0200
committer Noel Grandin <noel.grandin@collabora.co.uk> Tue Jun 04 09:35:23 2019 +0200
tree d13702c2b680a547fa6d3b06c72222b92353eaca
parent 6e4513e2b4e32926354d745ecf08fdf440392a75

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@collabora.co.uk>
(cherry picked from commit 0d04315c17a6df9f971237d45d9e5e8af765dd17)
Reviewed-on: https://gerrit.libreoffice.org/73401

sal/osl/unx/file.cxx

diff --git a/sal/osl/unx/file.cxx b/sal/osl/unx/file.cxx
index 1f04802..0ee0b88 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 @@

    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 @@

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 @@
    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;

sal/osl/w32/file.cxx

diff --git a/sal/osl/w32/file.cxx b/sal/osl/w32/file.cxx
index a360084..7fda610 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 @@

    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 @@

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 @@
    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;
}