libslic3r: resolve user presets together

A user preset is read from its own file and flattened against the system preset
it inherits. It cannot inherit another file of the same pass, because the presets
being read only join the collection once they have all been read, so the files
are independent of each other.

load_presets splits the way the vendor cache load already does:
resolve_user_preset reads and flattens one file and touches nothing shared, and
commit_user_preset installs it, counts its errors, and does the file work a load
can trigger (removing an unreadable preset, writing back a filament preset that
named no compatible printer). Both callers now share resolve_then_commit, which
holds the two-phase shape, works through the items in batches so what is held at
once does not grow with how many there are, and gives each piece of a batch one
CNumericLocalesSetter rather than one per file. It throws rather than commit a
batch that a canceled task group of the caller left unresolved.

A setter nested in another on the same thread does nothing while the locale is
still "C", so one setter per piece of a batch covers every file in it, and each
batch runs isolated, so a thread waiting on it picks up none of the caller's
other work. A filament preset whose derived compatible printer cannot be written
back still loads, with the failure counted.

Loading 328 user presets goes from 105 to 34 ms on a 16-core desktop.

src/libslic3r/CMakeLists.txt

@@ -353,6 +353,7 @@ set(lisbslic3r_sources
     Optimize/Optimizer.hpp
     Orient.cpp
     Orient.hpp
+    ParallelResolve.hpp
     ParameterUtils.cpp
     ParameterUtils.hpp
     pchheader.cpp

src/libslic3r/LocalesUtils.cpp

@@ -13,8 +13,19 @@
 namespace Slic3r {
 
 
+// How many setters this thread holds, so the ones nested in another can skip
+// setlocale, which takes a lock the whole process shares on Windows.
+static thread_local int s_numeric_locale_depth = 0;
+
 CNumericLocalesSetter::CNumericLocalesSetter()
 {
+    // Nested in another setter on this thread, whose "C" the separator check
+    // confirms is still set.
+    if (s_numeric_locale_depth > 0 && is_decimal_separator_point()) {
+        m_nested = true;
+        ++ s_numeric_locale_depth;
+        return;
+    }
 #ifdef _WIN32
     _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
     m_orig_numeric_locale = std::setlocale(LC_NUMERIC, nullptr);
@@ -29,12 +40,17 @@ CNumericLocalesSetter::CNumericLocalesSetter()
     m_new_locale = newlocale(LC_NUMERIC_MASK, "C", m_new_locale);
     uselocale(m_new_locale);
 #endif
+    // Counted last, since the destructor does not run for a constructor that throws.
+    ++ s_numeric_locale_depth;
 }
 
 
 
 CNumericLocalesSetter::~CNumericLocalesSetter()
 {
+    -- s_numeric_locale_depth;
+    if (m_nested)
+        return;
 #ifdef _WIN32
     std::setlocale(LC_NUMERIC, m_orig_numeric_locale.data());
 #else

src/libslic3r/LocalesUtils.hpp

@@ -19,8 +19,13 @@ class CNumericLocalesSetter {
 public:
     CNumericLocalesSetter();
     ~CNumericLocalesSetter();
+    // A copy would restore the locale twice, and count down once more than up.
+    CNumericLocalesSetter(const CNumericLocalesSetter&) = delete;
+    CNumericLocalesSetter& operator=(const CNumericLocalesSetter&) = delete;
 
 private:
+    // Inside another setter on this thread, which does the setting and restoring.
+    bool m_nested { false };
 #ifdef _WIN32
     std::string m_orig_numeric_locale;
 #else

src/libslic3r/ParallelResolve.hpp

@@ -0,0 +1,65 @@
+#pragma once
+
+#include <algorithm>
+#include <cstddef>
+#include <type_traits>
+#include <utility>
+#include <variant>
+#include <vector>
+
+#include <tbb/blocked_range.h>
+#include <tbb/parallel_for.h>
+#include <tbb/task_arena.h>
+#include <tbb/task_group.h>
+
+#include "Exception.hpp"
+
+namespace Slic3r {
+
+// A batch of resolved presets, each a whole config, is what resolve_then_commit adds
+// to peak memory, so it stays far below a vendor's preset count and above any core count.
+inline constexpr size_t resolve_batch_size = 64;
+
+// Resolve `count` items that do not depend on each other and install them one at
+// a time.
+//
+// `resolve(i)` runs on any thread and must read only, since the items are
+// resolved side by side. `commit(i, resolved)` is called for every item in index
+// order, on the calling thread, and is where shared state is written.
+//
+// The items are worked through in batches, so what is resolved and held at once
+// does not grow with `count`. An exception from either callable propagates after
+// the batches before it have been committed. A cancellation of the caller's task
+// group, which stops a batch partway without an exception, throws RuntimeError
+// before that batch commits.
+//
+// `ChunkSetup`, when given, is constructed once for each piece of a batch TBB hands
+// out, for per-thread state a resolve would otherwise set up per item, such as the
+// C numeric locale, whose setting takes a lock the whole process shares.
+template<class ChunkSetup = std::monostate, class Resolve, class Commit>
+void resolve_then_commit(size_t count, Resolve resolve, Commit commit)
+{
+    using Resolved = std::invoke_result_t<Resolve&, size_t>;
+
+    std::vector<Resolved> resolved(std::min(count, resolve_batch_size));
+    for (size_t first = 0; first < count; first += resolve_batch_size) {
+        const size_t last = std::min(first + resolve_batch_size, count);
+        // Isolated, so a thread waiting on the batch runs none of the caller's other
+        // tasks before finishing it.
+        tbb::this_task_arena::isolate([&] {
+            tbb::parallel_for(tbb::blocked_range<size_t>(first, last),
+                [&](const tbb::blocked_range<size_t>& range) {
+                    ChunkSetup setup;
+                    (void) setup;
+                    for (size_t i = range.begin(); i < range.end(); ++ i)
+                        resolved[i - first] = resolve(i);
+                });
+        });
+        if (tbb::is_current_task_group_canceling())
+            throw RuntimeError("resolve_then_commit: canceled before the batch was resolved");
+        for (size_t i = first; i < last; ++ i)
+            commit(i, std::move(resolved[i - first]));
+    }
+}
+
+} // namespace Slic3r

src/libslic3r/Preset.cpp

@@ -5,6 +5,8 @@
 #include "Preset.hpp"
 #include "PresetBundle.hpp"
 #include "AppConfig.hpp"
+#include "LocalesUtils.hpp"
+#include "ParallelResolve.hpp"
 
 #ifdef _MSC_VER
     #define WIN32_LEAN_AND_MEAN
@@ -621,7 +623,6 @@ void Preset::load_info(const std::string& file)
             }
             else if (v.first.compare("base_id") == 0) {
                 this->base_id = v.second.get_value<std::string>();
-                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load info from: " << file << " and base_id: " << this->base_id;
                 if (this->base_id.compare("null") == 0)
                     this->base_id.clear();
             }
@@ -1669,6 +1670,170 @@ std::string PresetCollection::canonical_preset_name(const std::string &name, con
     return get_preset_canonical_name(parsed.bare, origin);
 }
 
+PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset(
+    const boost::filesystem::path &file, const std::string &canonical_name,
+    const PresetOrigin &load_origin, ForwardCompatibilitySubstitutionRule substitution_rule,
+    const std::string &extruder_id_name, const std::string &extruder_variant_name,
+    std::set<std::string> *key_set1, std::set<std::string> *key_set2) const
+{
+    UserPresetLoad out;
+    out.preset = Preset(m_type, canonical_name, false);
+    Preset &preset = out.preset;
+    preset.bundle_id = load_origin.bundle_id;
+    preset.file = file.string();
+    // Load the preset file, apply preset values on top of defaults.
+    try {
+        fs::path idx_path(preset.file);
+        idx_path.replace_extension(".info");
+        if (fs::exists(idx_path)) {
+            out.info_file = idx_path.string();
+            preset.load_info(out.info_file);
+        }
+        DynamicPrintConfig config;
+        //BBS: change to json format
+        //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
+        std::map<std::string, std::string> key_values;
+        std::string reason;
+        ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
+        if (! config_substitutions.empty())
+            out.substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) });
+        if (!reason.empty()) {
+            out.discard_file = true;
+            out.errors.push_back((boost::format("parse config %1% failed") % preset.file).str());
+            return out;
+        }
+
+        std::string version_str = key_values[BBL_JSON_KEY_VERSION];
+        boost::optional<Semver> version = Semver::parse(version_str);
+        if (!version) return out;
+        preset.version = *version;
+
+        if (key_values.find(BBL_JSON_KEY_FILAMENT_ID) != key_values.end())
+            preset.filament_id = key_values[BBL_JSON_KEY_FILAMENT_ID];
+        if (key_values.find(BBL_JSON_KEY_DESCRIPTION) != key_values.end())
+            preset.description = key_values[BBL_JSON_KEY_DESCRIPTION];
+        if (key_values.find(BBL_JSON_KEY_INSTANTIATION) != key_values.end())
+            preset.is_visible = key_values[BBL_JSON_KEY_INSTANTIATION] != "false";
+
+        //Orca: find and use the inherit config as the base
+        const Preset* inherit_preset = nullptr;
+        ConfigOption* inherits_config = config.option(BBL_JSON_KEY_INHERITS);
+
+        // check inherits_config
+        if (inherits_config) {
+            ConfigOptionString * option_str = dynamic_cast<ConfigOptionString *> (inherits_config);
+            std::string inherits_value = option_str->value;
+            // Orca: try to find if the parent preset has been renamed
+            inherit_preset = this->find_preset2(inherits_value);
+            Preset::normalize_inherits(config, inherit_preset);
+        }
+        const Preset& default_preset = this->default_preset_for(config);
+        if (inherit_preset) {
+            preset.config = inherit_preset->config;
+            preset.filament_id = inherit_preset->filament_id;
+            extend_default_config_length(config, false, {});
+            preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
+        }
+        else {
+            auto inherits_config2 = dynamic_cast<ConfigOptionString *>(inherits_config);
+            if ((inherits_config2 && !inherits_config2->value.empty())) {
+                out.errors.push_back((boost::format("can not find parent %1% for config %2%!") % inherits_config2->value % preset.file).str());
+                return out;
+            }
+            // We support custom root preset now
+            // Find a default preset for the config. The PrintPresetCollection provides different default preset based on the "printer_technology" field.
+            preset.config = default_preset.config;
+            preset.config.apply(std::move(config));
+            extend_default_config_length(preset.config, true, default_preset.config);
+        }
+
+        Preset::normalize(preset.config);
+        // Report configuration fields, which are misplaced into a wrong group.
+        std::string incorrect_keys = Preset::remove_invalid_keys(preset.config, default_preset.config);
+        if (!incorrect_keys.empty())
+            out.errors.push_back("Error in a preset file: The preset \"" + preset.file +
+                                 "\" contains the following incorrect keys: " + incorrect_keys + ", which were removed");
+
+        if (preset.type == Preset::TYPE_FILAMENT && preset.is_user() && preset.inherits().empty()) {
+            auto compatible_printers = dynamic_cast<ConfigOptionStrings *>(preset.config.option("compatible_printers", true));
+            if (compatible_printers && compatible_printers->values.empty()) {
+                size_t at_pos = canonical_name.find('@');
+                if (at_pos != std::string::npos && at_pos + 1 < canonical_name.length()) {
+                    compatible_printers->values.push_back(canonical_name.substr(at_pos + 1));
+                    out.save_compatible_printers = true;
+                }
+            }
+        }
+
+        preset.loaded = true;
+        out.complete = true;
+    } catch (const std::ifstream::failure &err) {
+        out.discard_file = true;
+        out.errors.push_back((boost::format("The user-config cannot be loaded: %1%. Reason: %2%") % preset.file % err.what()).str());
+        //throw Slic3r::RuntimeError(std::string("The selected preset cannot be loaded: ") + preset.file + "\n\tReason: " + err.what());
+    } catch (const std::runtime_error &err) {
+        out.discard_file = true;
+        out.errors.push_back((boost::format("Failed loading the user-config file: %1%. Reason: %2%") % preset.file % err.what()).str());
+        //throw Slic3r::RuntimeError(std::string("Failed loading the preset file: ") + preset.file + "\n\tReason: " + err.what());
+    }
+    out.install = true;
+    return out;
+}
+
+void PresetCollection::commit_user_preset(UserPresetLoad &&loaded, std::deque<Preset> &presets_loaded,
+                                          PresetsConfigSubstitutions &substitutions,
+                                          const std::function<void(Preset&)> &preset_loaded_fn,
+                                          bool read_only)
+{
+    Preset &preset = loaded.preset;
+    if (! loaded.info_file.empty())
+        BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load info from: " << loaded.info_file << " and base_id: " << preset.base_id;
+    append(substitutions, std::move(loaded.substitutions));
+    for (const std::string &error : loaded.errors) {
+        ++m_errors;
+        BOOST_LOG_TRIVIAL(error) << error;
+    }
+    if (loaded.discard_file && !read_only) {
+        fs::path file_path(loaded.preset.file);
+        if (fs::exists(file_path))
+            fs::remove(file_path);
+        file_path.replace_extension(".info");
+        if (fs::exists(file_path))
+            fs::remove(file_path);
+    }
+    if (!loaded.install)
+        return;
+
+    if (loaded.complete) {
+        if (loaded.save_compatible_printers) {
+            // The preset loads with the derived printer whether or not its file
+            // can be rewritten.
+            try {
+                if (!read_only)
+                    preset.save(nullptr);
+                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << preset.name;
+            } catch (const std::runtime_error &err) {
+                ++m_errors;
+                BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << " can not write compatible_printers back to " << preset.file << ": " << err.what();
+            }
+        }
+        //BBS: add some workaround for previous incorrect settings
+        if ((!preset.setting_id.empty())&&(preset.setting_id == preset.base_id))
+            preset.setting_id.clear();
+        BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load preset: " << preset.name << " and filament_id: " << preset.filament_id << " and base_id: " << preset.base_id;
+        //BBS: add config related logs
+        BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(", preset type %1%, name %2%, path %3%, is_system %4%, is_default %5% is_visible %6%")%Preset::get_type_string(m_type) %preset.name %preset.file %preset.is_system %preset.is_default %preset.is_visible;
+        // add alias for custom filament preset
+        set_custom_preset_alias(preset);
+    }
+
+    if (preset_loaded_fn != nullptr)
+        preset_loaded_fn(preset);
+
+    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << __LINE__ << " load config successful and preset name is:" << preset.name;
+    presets_loaded.emplace_back(std::move(preset));
+}
+
 // Load all presets found in dir_path.
 // Throws an exception on error.
 void PresetCollection::load_presets(
@@ -1706,6 +1871,8 @@ void PresetCollection::load_presets(
     std::set<std::string> *key_set1 = nullptr, *key_set2 = nullptr;
     Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2);
 
+    struct UserPresetFile { fs::path path; std::string canonical_name; };
+    std::vector<UserPresetFile> files;
     //BBS: change to json format
     for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
     {
@@ -1721,149 +1888,26 @@ void PresetCollection::load_presets(
                 BOOST_LOG_TRIVIAL(warning) << "Preset already present, not loading: " << canonical_name;
                 continue;
             }
+            files.push_back({ dir_entry.path(), std::move(canonical_name) });
+        }
+    }
+
+    resolve_then_commit<CNumericLocalesSetter>(files.size(),
+        [&](size_t i) {
+            return this->resolve_user_preset(files[i].path, files[i].canonical_name, resolved_origin, substitution_rule,
+                                             extruder_id_name, extruder_variant_name, key_set1, key_set2);
+        },
+        [&](size_t, UserPresetLoad &&loaded) {
+            // Committing can remove an unreadable preset's file, and a filesystem error
+            // there is reported without stopping the rest of the directory.
             try {
-                Preset preset(m_type, canonical_name, false);
-                preset.bundle_id = resolved_origin.bundle_id;
-                preset.file = dir_entry.path().string();
-                // Load the preset file, apply preset values on top of defaults.
-                try {
-                    fs::path idx_path(preset.file);
-                    idx_path.replace_extension(".info");
-                    if (fs::exists(idx_path)) {
-                        preset.load_info(idx_path.string());
-                    }
-                    DynamicPrintConfig config;
-                    //BBS: change to json format
-                    //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
-                    std::map<std::string, std::string> key_values;
-                    std::string reason;
-                    ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
-                    if (! config_substitutions.empty())
-                        substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) });
-                    if (!reason.empty()) {
-                        fs::path file_path(preset.file);
-                        if (!read_only && fs::exists(file_path))
-                            fs::remove(file_path);
-                        file_path.replace_extension(".info");
-                        if (!read_only && fs::exists(file_path))
-                            fs::remove(file_path);
-                        BOOST_LOG_TRIVIAL(error) << boost::format("parse config %1% failed")%preset.file;
-                        ++m_errors;
-                        continue;
-                    }
-
-                    std::string version_str = key_values[BBL_JSON_KEY_VERSION];
-                    boost::optional<Semver> version = Semver::parse(version_str);
-                    if (!version) continue;
-                    preset.version = *version;
-
-                    if (key_values.find(BBL_JSON_KEY_FILAMENT_ID) != key_values.end())
-                        preset.filament_id = key_values[BBL_JSON_KEY_FILAMENT_ID];
-                    if (key_values.find(BBL_JSON_KEY_DESCRIPTION) != key_values.end())
-                        preset.description = key_values[BBL_JSON_KEY_DESCRIPTION];
-                    if (key_values.find(BBL_JSON_KEY_INSTANTIATION) != key_values.end())
-                        preset.is_visible = key_values[BBL_JSON_KEY_INSTANTIATION] != "false";
-
-                    //Orca: find and use the inherit config as the base
-                    Preset* inherit_preset = nullptr;
-                    ConfigOption* inherits_config = config.option(BBL_JSON_KEY_INHERITS);
-
-                    // check inherits_config
-                    if (inherits_config) {
-                        ConfigOptionString * option_str = dynamic_cast<ConfigOptionString *> (inherits_config);
-                        std::string inherits_value = option_str->value;
-                        // Orca: try to find if the parent preset has been renamed
-                        inherit_preset = this->find_preset2(inherits_value);
-                        Preset::normalize_inherits(config, inherit_preset);
-                    } else {
-                        ;
-                    }
-                    const Preset& default_preset = this->default_preset_for(config);
-                    if (inherit_preset) {
-                        preset.config = inherit_preset->config;
-                        preset.filament_id = inherit_preset->filament_id;
-                        extend_default_config_length(config, false, {});
-                        preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
-                    }
-                    else {
-                        auto inherits_config2 = dynamic_cast<ConfigOptionString *>(inherits_config);
-                        if ((inherits_config2 && !inherits_config2->value.empty())) {
-                            BOOST_LOG_TRIVIAL(error) << boost::format("can not find parent %1% for config %2%!")%inherits_config2->value %preset.file;
-                            ++m_errors;
-                            continue;
-                        }
-                        // We support custom root preset now
-                        // Find a default preset for the config. The PrintPresetCollection provides different default preset based on the "printer_technology" field.
-                        preset.config = default_preset.config;
-                        preset.config.apply(std::move(config));
-                        extend_default_config_length(preset.config, true, default_preset.config);
-                    }
-                    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " load preset: " << name << " and filament_id: " << preset.filament_id << " and base_id: " << preset.base_id;
-
-                    Preset::normalize(preset.config);
-                    // Report configuration fields, which are misplaced into a wrong group.
-                    std::string incorrect_keys = Preset::remove_invalid_keys(preset.config, default_preset.config);
-                    if (!incorrect_keys.empty()) {
-                        ++m_errors;
-                        BOOST_LOG_TRIVIAL(error)
-                            << "Error in a preset file: The preset \"" << preset.file
-                            << "\" contains the following incorrect keys: " << incorrect_keys << ", which were removed";
-                    }
-
-                    if (preset.type == Preset::TYPE_FILAMENT && preset.is_user() && preset.inherits().empty()) {
-                        auto compatible_printers = dynamic_cast<ConfigOptionStrings *>(preset.config.option("compatible_printers", true));
-                        if (compatible_printers && compatible_printers->values.empty()) {
-                            size_t at_pos = name.find('@');
-                            if (at_pos != std::string::npos && at_pos + 1 < name.length()) {
-                                compatible_printers->values.push_back(name.substr(at_pos + 1));
-                                if (!read_only)
-                                    preset.save(nullptr);
-                                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << name;
-                            }
-                        }
-                    }
-
-                    preset.loaded = true;
-                    //BBS: add some workaround for previous incorrect settings
-                    if ((!preset.setting_id.empty())&&(preset.setting_id == preset.base_id))
-                        preset.setting_id.clear();
-                    //BBS: add config related logs
-                    BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(", preset type %1%, name %2%, path %3%, is_system %4%, is_default %5% is_visible %6%")%Preset::get_type_string(m_type) %preset.name %preset.file %preset.is_system %preset.is_default %preset.is_visible;
-                    // add alias for custom filament preset
-                    set_custom_preset_alias(preset);
-                } catch (const std::ifstream::failure &err) {
-                    ++m_errors;
-                    BOOST_LOG_TRIVIAL(error) << boost::format("The user-config cannot be loaded: %1%. Reason: %2%")%preset.file %err.what();
-                    fs::path file_path(preset.file);
-                    if (!read_only && fs::exists(file_path))
-                        fs::remove(file_path);
-                    file_path.replace_extension(".info");
-                    if (!read_only && fs::exists(file_path))
-                        fs::remove(file_path);
-                    //throw Slic3r::RuntimeError(std::string("The selected preset cannot be loaded: ") + preset.file + "\n\tReason: " + err.what());
-                } catch (const std::runtime_error &err) {
-                    ++m_errors;
-                    BOOST_LOG_TRIVIAL(error) << boost::format("Failed loading the user-config file: %1%. Reason: %2%")%preset.file %err.what();
-                    //throw Slic3r::RuntimeError(std::string("Failed loading the preset file: ") + preset.file + "\n\tReason: " + err.what());
-                    fs::path file_path(preset.file);
-                    if (!read_only && fs::exists(file_path))
-                        fs::remove(file_path);
-                    file_path.replace_extension(".info");
-                    if (!read_only && fs::exists(file_path))
-                        fs::remove(file_path);
-                }
-
-                if (preset_loaded_fn != nullptr)
-                    preset_loaded_fn(preset);
-
-                presets_loaded.emplace_back(preset);
-                BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << __LINE__ << " load config successful and preset name is:" << preset.name;
+                this->commit_user_preset(std::move(loaded), presets_loaded, substitutions, preset_loaded_fn, read_only);
             } catch (const std::runtime_error &err) {
                 errors_cummulative += err.what();
                 errors_cummulative += "\n";
             }
-        }
-    }
+        });
+
     if (presets_loaded.size() > 0)
         m_presets.insert(m_presets.end(), std::make_move_iterator(presets_loaded.begin()), std::make_move_iterator(presets_loaded.end()));
     sort_presets();

src/libslic3r/Preset.hpp

@@ -801,6 +801,8 @@ public:
     // Return number of presets including the "- default -" preset.
     size_t          size() const                { return m_presets.size(); }
     bool            has_defaults_only() const   { return m_presets.size() <= m_num_default_presets; }
+    // How many presets this collection refused or repaired while loading.
+    int             error_count() const         { return m_errors; }
 
     // For Print / Filament presets, disable those, which are not compatible with the printer.
     template<typename PreferedCondition>
@@ -890,6 +892,43 @@ protected:
     void            set_custom_preset_alias(Preset &preset);
 
 private:
+    // One preset file read and flattened against the presets already in this
+    // collection, before anything the collection shares has been touched.
+    struct UserPresetLoad
+    {
+        Preset      preset;
+        // Joins the collection. A file that threw partway still joins it, without
+        // the steps that did not run.
+        bool        install { false };
+        // The whole of the load ran, so the preset is ready to be aliased.
+        bool        complete { false };
+        // A filament preset that named no compatible printer and was given one from
+        // its name, which commit writes back to its file.
+        bool        save_compatible_printers { false };
+        // Unreadable, so commit removes it and its .info file.
+        bool        discard_file { false };
+        // The .info file read beside the preset, which commit logs.
+        std::string info_file;
+        // Counted and logged by commit, in the order the directory listed the files.
+        std::vector<std::string>   errors;
+        PresetsConfigSubstitutions substitutions;
+    };
+
+    // Read and flatten one preset file. It reads only, and resolves against the presets
+    // loaded before this pass, never another file of the same pass, so the files of a
+    // pass are independent of each other.
+    UserPresetLoad  resolve_user_preset(const boost::filesystem::path &file, const std::string &canonical_name,
+                                        const PresetOrigin &load_origin, ForwardCompatibilitySubstitutionRule substitution_rule,
+                                        const std::string &extruder_id_name, const std::string &extruder_variant_name,
+                                        std::set<std::string> *key_set1, std::set<std::string> *key_set2) const;
+
+    // Install one resolved preset. The collection, its alias maps, the error count
+    // and the preset files on disk are touched here and only here.
+    void            commit_user_preset(UserPresetLoad &&loaded, std::deque<Preset> &presets_loaded,
+                                       PresetsConfigSubstitutions &substitutions,
+                                       const std::function<void(Preset&)> &preset_loaded_fn,
+                                       bool read_only);
+
     std::string canonical_preset_name(const std::string &name, const PresetOrigin &load_origin = PresetOrigin()) const;
 
     // Comparator that sorts "Generic " prefixed presets before others, then alphabetically within each group.

src/libslic3r/PresetBundle.cpp

@@ -5,6 +5,7 @@
 
 #include "PresetBundle.hpp"
 
+#include "ParallelResolve.hpp"
 #include "PresetCacheFormat.hpp"
 #include "PrintConfig.hpp"
 #include "PublishSettings.hpp"
@@ -20,7 +21,6 @@
 #include <algorithm>
 #include <cstdlib>
 #include <numeric>
-#include <thread>
 #include <set>
 #include <fstream>
 #include <unordered_set>
@@ -8116,33 +8116,21 @@ bool PresetBundle::load_vendor_cache(const std::string& cache_path, const std::s
             // the order the vendor lists them, because what it registers in `configs`
             // or `include_maps` is what they resolve against. So is an entry that
             // includes one listed after it, which it must find missing. The rest
-            // depend on nothing else, so they resolve at once. Two cores or fewer
-            // stay one at a time.
-            const bool resolve_together = std::thread::hardware_concurrency() > 2;
+            // depend on nothing else.
             std::vector<size_t> leaves;
             leaves.reserve(entries.size());
             for (size_t i = 0; i < entries.size(); ++ i) {
                 const CachedPreset& entry = entries[i];
                 const bool includes_installed = std::all_of(entry.includes.begin(), entry.includes.end(),
                     [&](const std::string& name) { return include_maps.count(name) != 0; });
-                if (resolve_together && inherited.count(entry.name) == 0 && included.count(entry.name) == 0 && includes_installed)
+                if (inherited.count(entry.name) == 0 && included.count(entry.name) == 0 && includes_installed)
                     leaves.push_back(i);
                 else
                     commit(entry, resolve(entry));
             }
-            if (leaves.empty())
-                return;
-
-            std::vector<PresetInstall> resolved(leaves.size());
-            tbb::parallel_for(tbb::blocked_range<size_t>(0, leaves.size()),
-                [&](const tbb::blocked_range<size_t>& range) {
-                    for (size_t k = range.begin(); k < range.end(); ++ k)
-                        resolved[k] = resolve(entries[leaves[k]]);
-                });
-            // Installed in the order the vendor lists them, so a duplicate name
-            // or a rejected entry is reported as it would be one at a time.
-            for (size_t k = 0; k < leaves.size(); ++ k)
-                commit(entries[leaves[k]], std::move(resolved[k]));
+            resolve_then_commit(leaves.size(),
+                [&](size_t k) { return resolve(entries[leaves[k]]); },
+                [&](size_t k, PresetInstall&& resolved) { commit(entries[leaves[k]], std::move(resolved)); });
         };
         install_entries(data.process_entries, &this->prints, false);
         const bool is_orca_lib = vendor_name == ORCA_FILAMENT_LIBRARY;

tests/libslic3r/CMakeLists.txt

@@ -14,7 +14,9 @@ add_executable(${_TEST_NAME}_tests
     test_clipper_utils.cpp
     test_config.cpp
     test_config_variant_expansion.cpp
+    test_locales_utils.cpp
     test_toolordering_nozzle_group.cpp
+    test_parallel_resolve.cpp
     test_preset_bundle_loading.cpp
     test_preset_setting_id.cpp
     test_preset_diff.cpp

tests/libslic3r/test_locales_utils.cpp

@@ -0,0 +1,69 @@
+#include <catch2/catch_all.hpp>
+
+#include <clocale>
+
+#include "libslic3r/LocalesUtils.hpp"
+
+using namespace Slic3r;
+
+namespace {
+
+// Switches this thread's numeric locale to one whose decimal separator is a comma,
+// when the system has one installed.
+struct CommaNumericLocale
+{
+#ifdef _WIN32
+    bool apply()
+    {
+        for (const char* name : { "de-DE", "German_Germany.1252" })
+            if (std::setlocale(LC_NUMERIC, name) != nullptr)
+                return true;
+        return false;
+    }
+#else
+    locale_t locale { (locale_t) 0 };
+    bool apply()
+    {
+        for (const char* name : { "de_DE.UTF-8", "de_DE.utf8", "de_DE" })
+            if ((locale = newlocale(LC_NUMERIC_MASK, name, (locale_t) 0)) != (locale_t) 0) {
+                uselocale(locale);
+                return true;
+            }
+        return false;
+    }
+    // Freed once the setters around apply() have put the thread's own locale back.
+    ~CommaNumericLocale()
+    {
+        if (locale != (locale_t) 0)
+            freelocale(locale);
+    }
+#endif
+};
+
+} // namespace
+
+TEST_CASE("a setter nested in another leaves the C locale in place for the outer one", "[LocalesUtils]")
+{
+    CNumericLocalesSetter outer;
+    {
+        CNumericLocalesSetter inner;
+        CHECK(is_decimal_separator_point());
+    }
+    CHECK(is_decimal_separator_point());
+}
+
+TEST_CASE("a setter nested in another sets C again when the locale changed between them", "[LocalesUtils]")
+{
+    CommaNumericLocale comma;
+    {
+        CNumericLocalesSetter outer;
+        if (! comma.apply())
+            SKIP("no locale with a comma decimal separator is installed");
+        REQUIRE_FALSE(is_decimal_separator_point());
+        {
+            CNumericLocalesSetter inner;
+            CHECK(is_decimal_separator_point());
+        }
+        CHECK_FALSE(is_decimal_separator_point());
+    }
+}

tests/libslic3r/test_parallel_resolve.cpp

@@ -0,0 +1,122 @@
+#include <catch2/catch_all.hpp>
+
+#include <numeric>
+#include <stdexcept>
+#include <thread>
+#include <vector>
+
+#include <tbb/blocked_range.h>
+#include <tbb/parallel_for.h>
+#include <tbb/task_group.h>
+
+#include "libslic3r/ParallelResolve.hpp"
+
+using namespace Slic3r;
+
+namespace {
+
+thread_local int t_live_setups = 0;
+
+// Counts how many are alive on the constructing thread.
+struct CountingSetup
+{
+    CountingSetup() { ++ t_live_setups; }
+    ~CountingSetup() { -- t_live_setups; }
+};
+
+std::vector<size_t> first_indices(size_t count)
+{
+    std::vector<size_t> indices(count);
+    std::iota(indices.begin(), indices.end(), size_t(0));
+    return indices;
+}
+
+} // namespace
+
+TEST_CASE("every item resolves once and commits in index order on the calling thread", "[ParallelResolve]")
+{
+    const size_t        count = 200;
+    std::vector<int>    resolves(count, 0);
+    std::vector<size_t> committed, values;
+    std::vector<bool>   on_caller;
+    const std::thread::id caller = std::this_thread::get_id();
+    resolve_then_commit(count,
+        [&](size_t i) { ++ resolves[i]; return i * 3; },
+        [&](size_t i, size_t resolved) {
+            committed.push_back(i);
+            values.push_back(resolved);
+            on_caller.push_back(std::this_thread::get_id() == caller);
+        });
+
+    CHECK(committed == first_indices(count));
+    for (size_t i = 0; i < count; ++ i) {
+        CHECK(resolves[i] == 1);
+        CHECK(values[i] == i * 3);
+        CHECK(on_caller[i]);
+    }
+}
+
+TEST_CASE("every item resolves inside one chunk setup", "[ParallelResolve]")
+{
+    const size_t     count = 200;
+    std::vector<int> live(count, 0);
+    resolve_then_commit<CountingSetup>(count,
+        [&](size_t i) { live[i] = t_live_setups; return 0; },
+        [](size_t, int) {});
+
+    for (size_t i = 0; i < count; ++ i)
+        CHECK(live[i] == 1);
+}
+
+TEST_CASE("an exception from resolve leaves the batches before it committed", "[ParallelResolve]")
+{
+    const size_t        count = 200, fails_at = 150;
+    REQUIRE(fails_at >= resolve_batch_size);
+    std::vector<size_t> committed;
+    CHECK_THROWS_AS(resolve_then_commit(count,
+                        [&](size_t i) {
+                            if (i == fails_at)
+                                throw std::runtime_error("resolve failed");
+                            return i;
+                        },
+                        [&](size_t i, size_t) { committed.push_back(i); }),
+                    std::runtime_error);
+
+    CHECK(committed == first_indices(fails_at / resolve_batch_size * resolve_batch_size));
+}
+
+TEST_CASE("an exception from commit stops at the item that threw", "[ParallelResolve]")
+{
+    const size_t        count = 200, fails_at = 90;
+    std::vector<size_t> committed;
+    CHECK_THROWS_AS(resolve_then_commit(count,
+                        [](size_t i) { return i; },
+                        [&](size_t i, size_t) {
+                            if (i == fails_at)
+                                throw std::runtime_error("commit failed");
+                            committed.push_back(i);
+                        }),
+                    std::runtime_error);
+
+    CHECK(committed == first_indices(fails_at));
+}
+
+TEST_CASE("a canceled task group stops before committing an item it did not resolve", "[ParallelResolve]")
+{
+    std::vector<size_t>     committed;
+    bool                    threw = false;
+    tbb::task_group_context context;
+    tbb::parallel_for(tbb::blocked_range<size_t>(0, 1), [&](const tbb::blocked_range<size_t>&) {
+        context.cancel_group_execution();
+        try {
+            resolve_then_commit(200,
+                [](size_t i) { return i + 1; },
+                [&](size_t, size_t resolved) { committed.push_back(resolved); });
+        } catch (const std::runtime_error&) {
+            threw = true;
+        }
+    }, context);
+
+    CHECK(threw);
+    CHECK(committed.empty());
+}

tests/libslic3r/test_preset_bundle_loading.cpp

@@ -59,6 +59,16 @@ void write_preset_with_inherits(const DynamicPrintConfig &default_config, const
     config.save_to_json(file.string(), name, "User", "1.0.0");
 }
 
+// A user preset file stating nothing but the preset it inherits, so every value it
+// ends up with came from resolving that parent.
+void write_minimal_child(const fs::path &file, const std::string &name, const std::string &inherits)
+{
+    fs::create_directories(file.parent_path());
+    std::ofstream(file.string())
+        << R"({"type":"process","name":")" << name << R"(","from":"User","version":"1.0.0","inherits":")"
+        << inherits << R"("})";
+}
+
 // Add an in-memory preset (no file) with the given inherits value (empty => root preset).
 Preset &add_inmemory_preset(PresetCollection &coll, const std::string &name, const std::string &inherits = {})
 {
@@ -261,6 +271,162 @@ TEST_CASE("Selected printer uses its default or saved bed type", "[Preset][Bundl
     CHECK(app_config.get_printer_setting("Test Printer", "curr_bed_type") == std::to_string(static_cast<int>(expected_bed_type)));
 }
 
+TEST_CASE("A directory of user presets loads with each one resolved against its parent", "[Preset][Bundle]")
+{
+    ScopedTemporaryDir   temp_dir;
+    RenameTestCollection coll;
+
+    Preset &parent = add_inmemory_preset(coll, "Parent Process");
+    parent.config.option<ConfigOptionFloat>("layer_height", true)->value = 0.24;
+    parent.is_system = true;
+
+    constexpr int children = 400;
+    for (int i = 0; i < children; ++ i)
+        write_minimal_child(temp_dir.path() / PRESET_PRINT_NAME / ("Child " + std::to_string(i) + ".json"),
+                            "Child " + std::to_string(i), "Parent Process");
+
+    PresetsConfigSubstitutions substitutions;
+    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
+                      ForwardCompatibilitySubstitutionRule::Disable);
+
+    CHECK(coll.size() == size_t(children) + 2); // the children, the default preset and the parent
+    CHECK(coll.error_count() == 0);
+    for (int i = 0; i < children; ++ i) {
+        const Preset *child = coll.find_preset("Child " + std::to_string(i));
+        REQUIRE(child != nullptr);
+        CHECK(child->inherits() == "Parent Process");
+        CHECK(child->alias == "Child " + std::to_string(i));
+        CHECK(child->loaded);
+        REQUIRE(child->config.option<ConfigOptionFloat>("layer_height") != nullptr);
+        CHECK_THAT(child->config.opt_float("layer_height"), Catch::Matchers::WithinAbs(0.24, 1e-9));
+    }
+}
+
+TEST_CASE("Repeated loads of a user preset directory produce the same presets", "[Preset][Bundle]")
+{
+    ScopedTemporaryDir temp_dir;
+
+    auto seed_directory = [&]() {
+        for (int i = 0; i < 200; ++ i)
+            write_minimal_child(temp_dir.path() / PRESET_PRINT_NAME / ("Child " + std::to_string(i) + ".json"),
+                                "Child " + std::to_string(i), "Parent Process");
+    };
+
+    std::vector<std::vector<std::string>> names_per_run;
+    for (int run = 0; run < 3; ++ run) {
+        RenameTestCollection coll;
+        Preset &parent = add_inmemory_preset(coll, "Parent Process");
+        parent.is_system = true;
+        if (run == 0)
+            seed_directory();
+
+        PresetsConfigSubstitutions substitutions;
+        coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
+                          ForwardCompatibilitySubstitutionRule::Disable);
+
+        std::vector<std::string> names;
+        for (auto it = coll.begin(); it != coll.end(); ++ it)
+            names.push_back(it->name + "|" + it->alias + "|" + it->inherits());
+        names_per_run.push_back(std::move(names));
+    }
+
+    REQUIRE(names_per_run[0].size() > 200);
+    CHECK(names_per_run[1] == names_per_run[0]);
+    CHECK(names_per_run[2] == names_per_run[0]);
+}
+
+TEST_CASE("An unreadable user preset is counted and removed while the rest still load", "[Preset][Bundle]")
+{
+    ScopedTemporaryDir   temp_dir;
+    RenameTestCollection coll;
+    const fs::path       dir = temp_dir.path() / PRESET_PRINT_NAME;
+
+    for (int i = 0; i < 20; ++ i)
+        write_preset_with_inherits(coll.default_preset().config, dir / ("Good " + std::to_string(i) + ".json"),
+                                   "Good " + std::to_string(i), std::string());
+    fs::create_directories(dir);
+    std::ofstream((dir / "Broken.json").string()) << "{not-json";
+
+    PresetsConfigSubstitutions substitutions;
+    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
+                      ForwardCompatibilitySubstitutionRule::EnableSilent);
+
+    CHECK(coll.error_count() == 1);
+    CHECK(coll.find_preset("Broken") == nullptr);
+    CHECK_FALSE(fs::exists(dir / "Broken.json"));
+    for (int i = 0; i < 20; ++ i)
+        CHECK(coll.find_preset("Good " + std::to_string(i)) != nullptr);
+}
+
+TEST_CASE("A user filament naming no compatible printer gets the one after its @, in memory and on disk", "[Preset][Bundle]")
+{
+    ScopedTemporaryDir temp_dir;
+    PresetBundle       bundle;
+    const fs::path     file = temp_dir.path() / PRESET_FILAMENT_NAME / "My PLA @Test Printer.json";
+    REQUIRE(bundle.filaments.default_preset().config.option<ConfigOptionStrings>("compatible_printers")->values.empty());
+    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
+
+    PresetsConfigSubstitutions substitutions;
+    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
+                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
+
+    const std::vector<std::string> expected { "Test Printer" };
+    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
+    REQUIRE(preset != nullptr);
+    CHECK(preset->config.option<ConfigOptionStrings>("compatible_printers")->values == expected);
+
+    DynamicPrintConfig                 saved;
+    std::map<std::string, std::string> key_values;
+    std::string                        reason;
+    saved.load_from_json(file.string(), ForwardCompatibilitySubstitutionRule::EnableSilent, key_values, reason);
+    REQUIRE(reason.empty());
+    REQUIRE(saved.option<ConfigOptionStrings>("compatible_printers") != nullptr);
+    CHECK(saved.option<ConfigOptionStrings>("compatible_printers")->values == expected);
+}
+
+TEST_CASE("A user preset's setting id equal to its base id is dropped in memory, not in the .info written back", "[Preset][Bundle]")
+{
+    ScopedTemporaryDir temp_dir;
+    PresetBundle       bundle;
+    const fs::path     file = temp_dir.path() / PRESET_FILAMENT_NAME / "My PLA @Test Printer.json";
+    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
+    fs::path info = file;
+    info.replace_extension(".info");
+    std::ofstream(info.string()) << "sync_info = \nuser_id = \nsetting_id = PFUS1\nbase_id = PFUS1\nupdated_time = 0\n";
+
+    PresetsConfigSubstitutions substitutions;
+    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
+                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
+
+    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
+    REQUIRE(preset != nullptr);
+    CHECK(preset->setting_id.empty());
+    CHECK(preset->base_id == "PFUS1");
+    Preset reloaded(Preset::TYPE_FILAMENT, "My PLA @Test Printer");
+    reloaded.load_info(info.string());
+    CHECK(reloaded.setting_id == "PFUS1");
+}
+
+TEST_CASE("A user preset that is not loaded still reports its substituted values", "[Preset][Bundle]")
+{
+    ScopedTemporaryDir   temp_dir;
+    RenameTestCollection coll;
+    const fs::path       dir = temp_dir.path() / PRESET_PRINT_NAME;
+    fs::create_directories(dir);
+    std::ofstream((dir / "Orphan.json").string())
+        << R"({"type":"process","name":"Orphan","from":"User","version":"1.0.0","inherits":"No Such Parent",)"
+        << R"("wall_generator":"no_such_generator"})";
+
+    PresetsConfigSubstitutions substitutions;
+    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
+                      ForwardCompatibilitySubstitutionRule::Enable);
+
+    CHECK(coll.find_preset("Orphan") == nullptr);
+    CHECK(coll.error_count() == 1);
+    REQUIRE(substitutions.size() == 1);
+    CHECK(substitutions.front().preset_name == "Orphan");
+}
+
 TEST_CASE("find_preset resolves a system preset's renamed_from", "[Preset][Rename]")
 {
     RenameTestCollection coll;