Iridas .cube and other text format parsing optimizations

Primarily driven by a wish to increase performance of parsing .cube
files. But the functions that are added or changed are used across
parsing of all/most of text based formats.

With these changes, parsing "Khronos PBR Neutral" .cube file (5.4MB)
on Ryzen 5950X / VS2022 Release build: 167ms -> 123ms.

- Add locale independent IsSpace(char). Somewhat similar to 3aab90d,
  whitespace trimming perhaps should not be locale dependent.
- Add IsEmptyOrWhiteSpace() and use that inside ParseUtils::nextline,
  instead of doing Trim(line).empty().
- Add StringUtils::StartsWith(char) and use that in various parsers
  that were constructing whole std::string object just to check for a
  single character.
- When building for C++17 or later, NumberUtils can use standard
  <charconv> from_chars functions (except on Apple platforms, where
  those are not implemented for floating point types as of Xcode 15).
  This has advantage of not having to deal with errno or locales. Saves
  some thread local storage accesses and function calls (e.g. on Windows
  errno is actually a function call).
- There's a CMake setup change that adds /Zc:__cplusplus flag for MSVC;
  for backwards compat reasons it does not report proper C++ version
  detection defines otherwise.

Signed-off-by: Aras Pranckevicius <[email protected]>

share/cmake/utils/CompilerFlags.cmake

@@ -62,6 +62,9 @@ if(USE_MSVC)
         )
     endif()
 
+    # Make MSVC compiler report correct __cplusplus version (otherwise reports 199711L)
+    set(PLATFORM_COMPILE_OPTIONS "${PLATFORM_COMPILE_OPTIONS};/Zc:__cplusplus")
+
     # Explicitely specify the default warning level i.e. /W3.
     # Note: Do not use /Wall (i.e. /W4) which adds 'informational' warnings.
     set(PLATFORM_COMPILE_OPTIONS "${PLATFORM_COMPILE_OPTIONS};/W3")

src/OpenColorIO/LookParse.cpp

@@ -17,13 +17,13 @@ void LookParseResult::Token::parse(const std::string & str)
 {
     // Assert no commas, colons, or | in str.
 
-    if(StringUtils::StartsWith(str, "+"))
+    if(StringUtils::StartsWith(str, '+'))
     {
         name = StringUtils::LeftTrim(str, '+');
         dir = TRANSFORM_DIR_FORWARD;
     }
     // TODO: Handle --
-    else if(StringUtils::StartsWith(str, "-"))
+    else if(StringUtils::StartsWith(str, '-'))
     {
         name = StringUtils::LeftTrim(str, '-');
         dir = TRANSFORM_DIR_INVERSE;

src/OpenColorIO/ParseUtils.cpp

@@ -672,13 +672,13 @@ bool nextline(std::istream &istream, std::string &line)
         {
             line.resize(line.size() - 1);
         }
-        if(!StringUtils::Trim(line).empty())
+        if(!StringUtils::IsEmptyOrWhiteSpace(line))
         {
             return true;
         }
     }
 
-    line = "";
+    line.clear();
     return false;
 }
 

src/OpenColorIO/fileformats/FileFormat3DL.cpp

@@ -263,11 +263,11 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             if(lineParts.empty()) continue;
             if (lineParts.size() > 0)
             {
-                if (StringUtils::StartsWith(lineParts[0], "#"))
+                if (StringUtils::StartsWith(lineParts[0], '#'))
                 {
                     continue;
                 }
-                if (StringUtils::StartsWith(lineParts[0], "<"))
+                if (StringUtils::StartsWith(lineParts[0], '<'))
                 {
                     // Format error: reject files that could be
                     // formatted as xml.

src/OpenColorIO/fileformats/FileFormatIridasCube.cpp

@@ -180,7 +180,7 @@ LocalFileFormat::read(std::istream & istream,
         {
             ++lineNumber;
             // All lines starting with '#' are comments
-            if (StringUtils::StartsWith(line,"#")) continue;
+            if (StringUtils::StartsWith(line,'#')) continue;
 
             line = StringUtils::Lower(StringUtils::Trim(line));
 
@@ -310,10 +310,10 @@ LocalFileFormat::read(std::istream & istream,
 
         do
         {
-            line = StringUtils::Trim(line);
+            line = StringUtils::LeftTrim(line);
 
             // All lines starting with '#' are comments
-            if (StringUtils::StartsWith(line,"#")) continue;
+            if (StringUtils::StartsWith(line,'#')) continue;
 
             if (line.empty()) continue;
 

src/OpenColorIO/fileformats/FileFormatIridasItx.cpp

@@ -146,7 +146,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
         {
             ++lineNumber;
             // All lines starting with '#' are comments
-            if(StringUtils::StartsWith(line,"#")) continue;
+            if(StringUtils::StartsWith(line,'#')) continue;
 
             // Strip, lowercase, and split the line
             parts = StringUtils::SplitByWhiteSpaces(StringUtils::Lower(StringUtils::Trim(line)));

src/OpenColorIO/fileformats/FileFormatPandora.cpp

@@ -129,7 +129,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             if(parts.empty()) continue;
 
             // Skip all lines starting with '#'
-            if(StringUtils::StartsWith(parts[0],"#")) continue;
+            if(StringUtils::StartsWith(parts[0],'#')) continue;
 
             if(parts[0] == "channel")
             {

src/OpenColorIO/fileformats/FileFormatResolveCube.cpp

@@ -296,7 +296,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             ++lineNumber;
 
             // All lines starting with '#' are comments
-            if(StringUtils::StartsWith(line,"#"))
+            if(StringUtils::StartsWith(line,'#'))
             {
                 if(headerComplete)
                 {

src/OpenColorIO/fileformats/FileFormatSpi1D.cpp

@@ -168,7 +168,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
                 }
             }
         }
-        while (istream.good() && !StringUtils::StartsWith(headerLine,"{"));
+        while (istream.good() && !StringUtils::StartsWith(headerLine,'{'));
     }
 
     if (version == -1)

src/OpenColorIO/fileformats/FileFormatTruelight.cpp

@@ -124,7 +124,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             if(parts.empty()) continue;
 
             // Parse header metadata (which starts with #)
-            if(StringUtils::StartsWith(parts[0],"#"))
+            if(StringUtils::StartsWith(parts[0],'#'))
             {
                 if(parts.size() < 2) continue;
 

src/OpenColorIO/fileformats/FileFormatVF.cpp

@@ -129,7 +129,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
 
             if(parts.empty()) continue;
 
-            if(StringUtils::StartsWith(parts[0],"#")) continue;
+            if(StringUtils::StartsWith(parts[0],'#')) continue;
 
             if(!in3d)
             {

src/utils/NumberUtils.h

@@ -4,6 +4,18 @@
 #ifndef INCLUDED_NUMBERUTILS_H
 #define INCLUDED_NUMBERUTILS_H
 
+// With C++17, we can use <charconv> from_chars.
+//
+// That has advantage of not dealing with locale / errno,
+// which might involve locks or thread local storage accesses.
+//
+// Except on Apple platforms, where (as of Xcode 15 / Apple Clang 15)
+// these are not implemented for float/double types.
+#if __cpp_lib_to_chars >= 201611L && !defined(__APPLE__)
+#define USE_CHARCONV_FROM_CHARS
+#include <charconv>
+#endif
+
 #if defined(_MSC_VER)
 #define really_inline __forceinline
 #else
@@ -56,12 +68,21 @@ static const Locale loc;
 
 really_inline from_chars_result from_chars(const char *first, const char *last, double &value) noexcept
 {
-    errno = 0;
     if (!first || !last || first == last)
     {
         return {first, std::errc::invalid_argument};
     }
 
+#ifdef USE_CHARCONV_FROM_CHARS
+    if (first < last && first[0] == '+')
+    {
+        ++first;
+    }
+    std::from_chars_result res = std::from_chars(first, last, value);
+    return from_chars_result{ res.ptr, res.ec };
+#else
+
+    errno = 0;
     char * endptr = nullptr;
 
     double
@@ -88,16 +109,26 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     {
         return {first, std::errc::argument_out_of_domain};
     }
+#endif
 }
 
 really_inline from_chars_result from_chars(const char *first, const char *last, float &value) noexcept
 {
-    errno = 0;
     if (!first || !last || first == last)
     {
         return {first, std::errc::invalid_argument};
     }
 
+#ifdef USE_CHARCONV_FROM_CHARS
+    if (first < last && first[0] == '+')
+    {
+        ++first;
+    }
+    std::from_chars_result res = std::from_chars(first, last, value);
+    return from_chars_result{ res.ptr, res.ec };
+#else
+
+    errno = 0;
     char *endptr = nullptr;
 
     float
@@ -132,16 +163,26 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     {
         return {first, std::errc::argument_out_of_domain};
     }
+#endif
 }
 
 really_inline from_chars_result from_chars(const char *first, const char *last, long int &value) noexcept
 {
-    errno = 0;
     if (!first || !last || first == last)
     {
         return {first, std::errc::invalid_argument};
     }
 
+#ifdef USE_CHARCONV_FROM_CHARS
+    if (first < last && first[0] == '+')
+    {
+        ++first;
+    }
+    std::from_chars_result res = std::from_chars(first, last, value);
+    return from_chars_result{ res.ptr, res.ec };
+#else
+
+    errno = 0;
     char *endptr = nullptr;
 
     long int
@@ -170,6 +211,7 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     {
         return {first, std::errc::argument_out_of_domain};
     }
+#endif
 }
 } // namespace NumberUtils
 } // namespace OCIO_NAMESPACE

src/utils/StringUtils.h

@@ -44,6 +44,13 @@ inline unsigned char Upper(unsigned char c)
     }
 }
 
+// Checks if character is whitespace (space, tab, newline or other
+// non-printable char). Does not take locale into account.
+inline bool IsSpace(char c)
+{
+    return c <= ' ';
+}
+
 // Return the lower case string.
 inline std::string Lower(std::string str)
 {
@@ -95,6 +102,13 @@ inline bool StartsWith(const std::string & str, const std::string & prefix)
     return str.size() >= prefix.size() && 0 == str.compare(0, prefix.size(), prefix);
 }
 
+// Return true if the string starts with the prefix character.
+// Note: The comparison is case sensitive.
+inline bool StartsWith(const std::string& str, char prefix)
+{
+    return !str.empty() && str.front() == prefix;
+}
+
 // Starting from the left, trim the character.
 inline std::string LeftTrim(std::string str, char c)
 {
@@ -106,7 +120,7 @@ inline std::string LeftTrim(std::string str, char c)
 // Starting from the left, trim all the space characters i.e. space, tabulation, etc.
 inline std::string LeftTrim(std::string str)
 {
-    const auto it = std::find_if(str.begin(), str.end(), [](char ch) { return !std::isspace(static_cast<unsigned char>(ch)); });
+    const auto it = std::find_if(str.begin(), str.end(), [](char ch) { return !IsSpace(ch); });
     str.erase(str.begin(), it);
     return str;
 }
@@ -123,7 +137,7 @@ inline std::string RightTrim(std::string str, char c)
 inline std::string RightTrim(std::string str)
 {
     const auto it =
-        std::find_if(str.rbegin(), str.rend(), [](char ch) { return !std::isspace(static_cast<unsigned char>(ch)); });
+        std::find_if(str.rbegin(), str.rend(), [](char ch) { return !IsSpace(ch); });
     str.erase(it.base(), str.end());
     return str;
 }
@@ -148,6 +162,12 @@ inline void Trim(StringVec & list)
     }
 }
 
+// Checks if string is empty or white-space only.
+inline bool IsEmptyOrWhiteSpace(const std::string& str)
+{
+    return std::find_if(str.begin(), str.end(), [](char ch) { return !IsSpace(ch); }) == str.end();
+}
+
 // Split a string content using an arbitrary separator.
 inline StringVec Split(const std::string & str, char separator)
 {

tests/cpu/fileformats/FileFormatIridasCube_tests.cpp

@@ -161,6 +161,43 @@ OCIO_ADD_TEST(FileFormatIridasCube, read_failure)
     }
 }
 
+OCIO_ADD_TEST(FileFormatIridasCube, whitespace_handling)
+{
+    const std::string SAMPLE =
+        "# comment\n"
+        "# comment with trailing space  \n"
+        "# next up various forms of empty lines\n"
+        "\n"
+        "   \n"
+        "   \t  \n"
+        "# whitespace before keywords or after data should be supported\n"
+        "  LUT_3D_SIZE \t 2  \t\n"
+        "\t \tDOMAIN_MIN    0.25    0.5    0.75\n"
+        "\n"
+        "DOMAIN_MAX\t1.5\t2.5\t3.5\n"
+
+        "0.0 0.0 0.0\n"
+        "# comments in between data should be ignored\n"
+        "   1.0    0.0 \t 0.0\n"
+        "0.0 1.0 0.0    \n"
+        "     1.0 1.0 0.0\n"
+        "    \n"
+        "0.0 0.0 1.0\n"
+        "1.0 0.0 1.0\n"
+        "0.0 1.0 1.0\n"
+        "1.0 1.0 1.0\n"
+        "   \n"
+        "\n";
+
+    OCIO::LocalCachedFileRcPtr file;
+    OCIO_CHECK_NO_THROW(file = ReadIridasCube(SAMPLE));
+    OCIO_CHECK_EQUAL(file->domain_min[0], 0.25f);
+    OCIO_CHECK_EQUAL(file->domain_min[2], 0.75f);
+    OCIO_CHECK_EQUAL(file->domain_max[0], 1.5f);
+    OCIO_CHECK_EQUAL(file->domain_max[2], 3.5f);
+    OCIO_CHECK_EQUAL(file->lut3D->getArray().getLength(), 2);
+}
+
 OCIO_ADD_TEST(FileFormatIridasCube, no_shaper)
 {
     // check baker output