[poppler] [PATCH v2 5/5] SplashXPathScanner: Optionally use small_vector from boost

[Adam Reichold]

Hello again,

I do like this one. If the maintainers consider an optional build time
dependency too complicated, one could also consider vendoring the
necessary headers. (But that might get complicated as well due to header
interdependencies.)

For the record, I would also add that Boost would indeed be a fine
addition in the tool box to write code for a larger project like
Poppler. Personally, I would hence even argue for a required build time
dependency since Boost's data structures alone would be tremendously
helpful, e.g. Dict really wants to be boost::container::flat_map.

Regards,
Adam

Am 26.05.2018 um 19:51 schrieb Stefan Brüns:
> Currently, each row in the intersections vector is allocated separately,
> when the first intersection is added.
> 
> To avoid these allocations for common simple polygons,
> boost::container::small_vector<4, T> is used, which stores up to
> 4 intersections inline. small_vector is a header-only class.
> 
> For the documents from fdo#96728 and fdo#78728, the runtime/memory is
> significantly reduced (according to /usr/bin/time -v):
> (1) $> pdftoppm -r 18 -aa no runsforever-poppler.pdf
> (2) $> pdftoppm surf-types.pdf
> 
> Before/After
>                                   runsforever-poppler |   surf-types
> User time (seconds):                2348.08 / 1773.53 |   7.76 /  5.02
> Maximum resident set size (kbytes):   46288 /   45896 |  14076 / 13748
> ---
>  CMakeLists.txt               | 10 ++++++++++
>  splash/SplashXPathScanner.cc |  2 ++
>  splash/SplashXPathScanner.h  |  8 ++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index b5a86a5f..463d1e39 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -403,6 +403,11 @@ set(poppler_SRCS
>  )
>  set(poppler_LIBS ${FREETYPE_LIBRARIES})
>  if(ENABLE_SPLASH)
> +  find_package(Boost 1.58.0)
> +  if(Boost_FOUND)
> +    include_directories(${Boost_INCLUDE_DIRS})
> +    add_definitions(-DUSE_BOOST_HEADERS)
> +  endif()
>    set(poppler_SRCS ${poppler_SRCS}
>      poppler/SplashOutputDev.cc
>      splash/Splash.cc
> @@ -726,6 +731,7 @@ show_end_message_yesno("use nss3" ENABLE_NSS3)
>  show_end_message_yesno("use curl" ENABLE_LIBCURL)
>  show_end_message_yesno("use libopenjpeg2" WITH_OPENJPEG)
>  show_end_message_yesno("use lcms2" USE_CMS)
> +show_end_message_yesno("use boost" Boost_FOUND)
>  show_end_message_yesno("command line utils" ENABLE_UTILS)
>  show_end_message("test data dir" ${TESTDATADIR})
>  
> @@ -757,6 +763,10 @@ if(NOT HAVE_JPX_DECODER)
>    message("Warning: You're not compiling any JPX decoder. Some files will fail to display properly.")
>  endif()
>  
> +if(ENABLE_SPLASH AND NOT Boost_FOUND)
> +  message("Warning: Use of boost is recommended for better performance.")
> +endif()
> +
>  set(ARCHIVE_NAME ${CMAKE_PROJECT_NAME}-${POPPLER_VERSION})
>  add_custom_target(dist
>      COMMAND
> diff --git a/splash/SplashXPathScanner.cc b/splash/SplashXPathScanner.cc
> index 444b8f96..7107586d 100644
> --- a/splash/SplashXPathScanner.cc
> +++ b/splash/SplashXPathScanner.cc
> @@ -337,9 +337,11 @@ GBool SplashXPathScanner::addIntersection(double segYMin, double segYMax,
>    }
>  
>    auto& line = allIntersections[y - yMin];
> +#ifndef USE_BOOST_HEADERS
>    if (line.empty()) {
>        line.reserve(4);
>    }
> +#endif
>    line.push_back(intersect);
>  
>    return gTrue;
> diff --git a/splash/SplashXPathScanner.h b/splash/SplashXPathScanner.h
> index 37b85f4b..7405dbd8 100644
> --- a/splash/SplashXPathScanner.h
> +++ b/splash/SplashXPathScanner.h
> @@ -28,6 +28,10 @@
>  
>  #include "SplashTypes.h"
>  
> +#ifdef USE_BOOST_HEADERS
> +#include <boost/container/small_vector.hpp>
> +#endif
> +
>  #include <vector>
>  
>  class SplashXPath;
> @@ -106,7 +110,11 @@ private:
>    int xMin, yMin, xMax, yMax;
>    GBool partialClip;
>  
> +#ifdef USE_BOOST_HEADERS
> +  typedef boost::container::small_vector<SplashIntersect, 4> IntersectionLine;
> +#else
>    typedef std::vector<SplashIntersect> IntersectionLine;
> +#endif
>    std::vector<IntersectionLine> allIntersections;
>  
>    unsigned int interIdx;	// current index into <inter> - used by
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20180526/5bab1562/attachment-0001.sig>