From e2e4c1daaa47df7892ca2dee4d317164726928f9 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Sun, 27 Mar 2022 14:25:48 +0300 Subject: OBJ: use fmt library instead of sprintf for faster formatting On Windows/MSVC this gives a minor (~20%) speedup presumably due to a faster float/int formatter. On macOS (Xcode13), this gives a massive speedup, since snprintf that is in system libraries ends up spending almost all the time inside some locale-related mutex lock. The actual exporter code becomes quite a bit smaller too, since it does not have to do any juggling to support std::string arguments, and the buffer handling code is smaller as well. Windows (VS2022 release build, Ryzen 5950X 32 threads) timings: - Blender 3.0 splash scene (2.4GB obj): 4.57s -> 3.86s - Monkey subdivided level 6 (330MB obj): 1.10s -> 0.99s macOS (Xcode 13 release build, Apple M1Max) timings: - Blender 3.0 splash scene (2.4GB obj): 21.03s -> 5.52s - Monkey subdivided level 6 (330MB obj): 3.28s -> 1.20s Linux (ThreadRipper 3960X 48 threads) timings: - Blender 3.0 splash scene (2.4GB obj): 10.10s -> 4.40s - Monkey subdivided level 6 (330MB obj): 2.16s -> 1.37s The produced obj/mtl files are identical to before. Reviewed By: Howard Trickey, Dalai Felinto Differential Revision: https://developer.blender.org/D13998 --- source/blender/io/wavefront_obj/CMakeLists.txt | 1 + .../io/wavefront_obj/exporter/obj_export_io.hh | 144 +++++++-------------- .../io/wavefront_obj/tests/obj_exporter_tests.cc | 2 +- 3 files changed, 50 insertions(+), 97 deletions(-) (limited to 'source') diff --git a/source/blender/io/wavefront_obj/CMakeLists.txt b/source/blender/io/wavefront_obj/CMakeLists.txt index 8045d51e0da..cc375577b52 100644 --- a/source/blender/io/wavefront_obj/CMakeLists.txt +++ b/source/blender/io/wavefront_obj/CMakeLists.txt @@ -13,6 +13,7 @@ set(INC ../../makesrna ../../nodes ../../windowmanager + ../../../../extern/fmtlib/include ../../../../intern/guardedalloc ) diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_io.hh b/source/blender/io/wavefront_obj/exporter/obj_export_io.hh index 3bee93b36a5..0095384609a 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_io.hh +++ b/source/blender/io/wavefront_obj/exporter/obj_export_io.hh @@ -8,7 +8,6 @@ #include #include -#include #include #include @@ -17,6 +16,11 @@ #include "BLI_string_ref.hh" #include "BLI_utility_mixins.hh" +/* SEP macro from BLI path utils clashes with SEP symbol in fmt headers. */ +#undef SEP +#define FMT_HEADER_ONLY +#include + namespace blender::io::obj { enum class eFileType { @@ -124,40 +128,40 @@ constexpr FormattingSyntax syntax_elem_to_formatting(const eOBJSyntaxElement key { switch (key) { case eOBJSyntaxElement::vertex_coords: { - return {"v %f %f %f\n", 3, is_type_float}; + return {"v {:.6f} {:.6f} {:.6f}\n", 3, is_type_float}; } case eOBJSyntaxElement::uv_vertex_coords: { - return {"vt %f %f\n", 2, is_type_float}; + return {"vt {:.6f} {:.6f}\n", 2, is_type_float}; } case eOBJSyntaxElement::normal: { - return {"vn %.4f %.4f %.4f\n", 3, is_type_float}; + return {"vn {:.4f} {:.4f} {:.4f}\n", 3, is_type_float}; } case eOBJSyntaxElement::poly_element_begin: { return {"f", 0, is_type_string_related}; } case eOBJSyntaxElement::vertex_uv_normal_indices: { - return {" %d/%d/%d", 3, is_type_integral}; + return {" {}/{}/{}", 3, is_type_integral}; } case eOBJSyntaxElement::vertex_normal_indices: { - return {" %d//%d", 2, is_type_integral}; + return {" {}//{}", 2, is_type_integral}; } case eOBJSyntaxElement::vertex_uv_indices: { - return {" %d/%d", 2, is_type_integral}; + return {" {}/{}", 2, is_type_integral}; } case eOBJSyntaxElement::vertex_indices: { - return {" %d", 1, is_type_integral}; + return {" {}", 1, is_type_integral}; } case eOBJSyntaxElement::poly_usemtl: { - return {"usemtl %s\n", 1, is_type_string_related}; + return {"usemtl {}\n", 1, is_type_string_related}; } case eOBJSyntaxElement::edge: { - return {"l %d %d\n", 2, is_type_integral}; + return {"l {} {}\n", 2, is_type_integral}; } case eOBJSyntaxElement::cstype: { return {"cstype bspline\n", 0, is_type_string_related}; } case eOBJSyntaxElement::nurbs_degree: { - return {"deg %d\n", 1, is_type_integral}; + return {"deg {}\n", 1, is_type_integral}; } case eOBJSyntaxElement::curve_element_begin: { return {"curv 0.0 1.0", 0, is_type_string_related}; @@ -166,7 +170,7 @@ constexpr FormattingSyntax syntax_elem_to_formatting(const eOBJSyntaxElement key return {"parm u 0.0", 0, is_type_string_related}; } case eOBJSyntaxElement::nurbs_parameters: { - return {" %f", 1, is_type_float}; + return {" {:.6f}", 1, is_type_float}; } case eOBJSyntaxElement::nurbs_parameter_end: { return {" 1.0\n", 0, is_type_string_related}; @@ -184,19 +188,19 @@ constexpr FormattingSyntax syntax_elem_to_formatting(const eOBJSyntaxElement key return {"\n", 0, is_type_string_related}; } case eOBJSyntaxElement::mtllib: { - return {"mtllib %s\n", 1, is_type_string_related}; + return {"mtllib {}\n", 1, is_type_string_related}; } case eOBJSyntaxElement::smooth_group: { - return {"s %d\n", 1, is_type_integral}; + return {"s {}\n", 1, is_type_integral}; } case eOBJSyntaxElement::object_group: { - return {"g %s\n", 1, is_type_string_related}; + return {"g {}\n", 1, is_type_string_related}; } case eOBJSyntaxElement::object_name: { - return {"o %s\n", 1, is_type_string_related}; + return {"o {}\n", 1, is_type_string_related}; } case eOBJSyntaxElement::string: { - return {"%s", 1, is_type_string_related}; + return {"{}", 1, is_type_string_related}; } } } @@ -206,56 +210,56 @@ constexpr FormattingSyntax syntax_elem_to_formatting(const eMTLSyntaxElement key { switch (key) { case eMTLSyntaxElement::newmtl: { - return {"newmtl %s\n", 1, is_type_string_related}; + return {"newmtl {}\n", 1, is_type_string_related}; } case eMTLSyntaxElement::Ni: { - return {"Ni %.6f\n", 1, is_type_float}; + return {"Ni {:.6f}\n", 1, is_type_float}; } case eMTLSyntaxElement::d: { - return {"d %.6f\n", 1, is_type_float}; + return {"d {:.6f}\n", 1, is_type_float}; } case eMTLSyntaxElement::Ns: { - return {"Ns %.6f\n", 1, is_type_float}; + return {"Ns {:.6f}\n", 1, is_type_float}; } case eMTLSyntaxElement::illum: { - return {"illum %d\n", 1, is_type_integral}; + return {"illum {}\n", 1, is_type_integral}; } case eMTLSyntaxElement::Ka: { - return {"Ka %.6f %.6f %.6f\n", 3, is_type_float}; + return {"Ka {:.6f} {:.6f} {:.6f}\n", 3, is_type_float}; } case eMTLSyntaxElement::Kd: { - return {"Kd %.6f %.6f %.6f\n", 3, is_type_float}; + return {"Kd {:.6f} {:.6f} {:.6f}\n", 3, is_type_float}; } case eMTLSyntaxElement::Ks: { - return {"Ks %.6f %.6f %.6f\n", 3, is_type_float}; + return {"Ks {:.6f} {:.6f} {:.6f}\n", 3, is_type_float}; } case eMTLSyntaxElement::Ke: { - return {"Ke %.6f %.6f %.6f\n", 3, is_type_float}; + return {"Ke {:.6f} {:.6f} {:.6f}\n", 3, is_type_float}; } /* Keep only one space between options since filepaths may have leading spaces too. */ case eMTLSyntaxElement::map_Kd: { - return {"map_Kd %s %s\n", 2, is_type_string_related}; + return {"map_Kd {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Ks: { - return {"map_Ks %s %s\n", 2, is_type_string_related}; + return {"map_Ks {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Ns: { - return {"map_Ns %s %s\n", 2, is_type_string_related}; + return {"map_Ns {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_d: { - return {"map_d %s %s\n", 2, is_type_string_related}; + return {"map_d {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_refl: { - return {"map_refl %s %s\n", 2, is_type_string_related}; + return {"map_refl {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Ke: { - return {"map_Ke %s %s\n", 2, is_type_string_related}; + return {"map_Ke {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Bump: { - return {"map_Bump %s %s\n", 2, is_type_string_related}; + return {"map_Bump {} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::string: { - return {"%s", 1, is_type_string_related}; + return {"{}", 1, is_type_string_related}; } } } @@ -270,9 +274,7 @@ constexpr FormattingSyntax syntax_elem_to_formatting(const eMTLSyntaxElement key * Call write_fo_file once in a while to write the memory buffer(s) * into the given file. */ -template +template class FormatHandler : NonCopyable, NonMovable { private: typedef std::vector VectorChar; @@ -299,7 +301,7 @@ class FormatHandler : NonCopyable, NonMovable { return blocks_.size(); } - void append_from(FormatHandler &v) + void append_from(FormatHandler &v) { blocks_.insert(blocks_.end(), std::make_move_iterator(v.blocks_.begin()), @@ -328,33 +330,6 @@ class FormatHandler : NonCopyable, NonMovable { } private: - /* Remove this after upgrading to C++20. */ - template using remove_cvref_t = std::remove_cv_t>; - - /** - * Make #std::string etc., usable for `fprintf` family. int float etc. are not affected. - * \return: `const char *` or the original argument if the argument is - * not related to #std::string. - */ - template constexpr auto convert_to_primitive(T &&arg) const - { - if constexpr (std::is_same_v, std::string> || - std::is_same_v, blender::StringRefNull>) { - return arg.c_str(); - } - else if constexpr (std::is_same_v, blender::StringRef>) { - BLI_STATIC_ASSERT( - (always_false::value), - "Null-terminated string not present. Please use blender::StringRefNull instead."); - /* Another trick to cause a compile-time error: returning nothing to #std::printf. */ - return; - } - else { - /* For int, float etc. */ - return std::forward(arg); - } - } - /* Ensure the last block contains at least this amount of free space. * If not, add a new block with max of block size & the amount of space needed. */ void ensure_space(size_t at_least) @@ -365,38 +340,15 @@ class FormatHandler : NonCopyable, NonMovable { } } - template constexpr void write_impl(const char *fmt, T &&...args) + template void write_impl(const char *fmt, T &&...args) { - if constexpr (sizeof...(T) == 0) { - /* No arguments: just emit the format string. */ - size_t len = strlen(fmt); - ensure_space(len); - VectorChar &bb = blocks_.back(); - bb.insert(bb.end(), fmt, fmt + len); - } - else { - /* Format into a local buffer. */ - char buf[write_local_buffer_size]; - int needed = std::snprintf( - buf, write_local_buffer_size, fmt, convert_to_primitive(std::forward(args))...); - if (needed < 0) - throw std::system_error( - errno, std::system_category(), "Failed to format obj export string into a buffer"); - ensure_space(needed + 1); /* Ensure space for zero terminator. */ - VectorChar &bb = blocks_.back(); - if (needed < write_local_buffer_size) { - /* String formatted successfully into the local buffer, copy it. */ - bb.insert(bb.end(), buf, buf + needed); - } - else { - /* Would need more space than the local buffer: insert said space and format again into - * that. */ - size_t bbEnd = bb.size(); - bb.insert(bb.end(), needed, ' '); - std::snprintf( - bb.data() + bbEnd, needed + 1, fmt, convert_to_primitive(std::forward(args))...); - } - } + /* Format into a local buffer. */ + fmt::memory_buffer buf; + fmt::format_to(fmt::appender(buf), fmt, std::forward(args)...); + size_t len = buf.size(); + ensure_space(len); + VectorChar &bb = blocks_.back(); + bb.insert(bb.end(), buf.begin(), buf.end()); } }; diff --git a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc index b8fecfb25f3..7e3a9228c3b 100644 --- a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc +++ b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc @@ -241,7 +241,7 @@ TEST(obj_exporter_writer, mtllib) TEST(obj_exporter_writer, format_handler_buffer_chunking) { /* Use a tiny buffer chunk size, so that the test below ends up creating several blocks. */ - FormatHandler h; + FormatHandler h; h.write("abc"); h.write("abcd"); h.write("abcde"); -- cgit v1.2.3