Skip to content

Commit 96c8260

Browse files
committed
FIX: debounce app_config->save() in wxEVT_IDLE handler (F-015)
The idle handler fired app_config->save() on every dirty idle event. On a busy loop this produces dozens of synchronous disk writes per second. Add a 5-second rate limit via debounce_elapsed() (Debounce.hpp, pure C++, no wx dependency). OnExit() flushes any remaining dirty state so no settings are lost on clean shutdown. Extract debounce_elapsed() to src/libslic3r/Debounce.hpp and add tests/libslic3r/test_debounce.cpp with 5 Catch2 scenarios covering first-call, suppression, expiry, zero-interval, and no-mutation-on-suppress. All tests passed (7 assertions in 5 test cases). Ref: #10289
1 parent 4bf3e98 commit 96c8260

4 files changed

Lines changed: 136 additions & 2 deletions

File tree

src/libslic3r/Debounce.hpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
///|/ Copyright (c) Prusa Research 2023 Vojtěch Bubník @bubnikv
2+
///|/
3+
///|/ PrusaSlicer is released under the terms of the AGPLv3 or higher
4+
///|/
5+
#ifndef slic3r_Debounce_hpp_
6+
#define slic3r_Debounce_hpp_
7+
8+
#include <chrono>
9+
10+
namespace Slic3r {
11+
12+
// Rate-limit a recurring action.
13+
//
14+
// Returns true and updates `last_tp` when at least `interval` has elapsed
15+
// since the last accepted call. On the very first call (last_tp ==
16+
// time_point{}) the action is always accepted.
17+
//
18+
// Usage:
19+
// static auto s_last = std::chrono::steady_clock::time_point{};
20+
// if (debounce_elapsed(s_last, std::chrono::seconds(5)))
21+
// do_expensive_thing();
22+
//
23+
// The function is intentionally free of wx / GUI dependencies so it can be
24+
// exercised by the plain libslic3r unit-test suite.
25+
inline bool debounce_elapsed(
26+
std::chrono::steady_clock::time_point &last_tp,
27+
std::chrono::steady_clock::duration interval)
28+
{
29+
const auto now = std::chrono::steady_clock::now();
30+
if (now - last_tp >= interval) {
31+
last_tp = now;
32+
return true;
33+
}
34+
return false;
35+
}
36+
37+
} // namespace Slic3r
38+
39+
#endif // slic3r_Debounce_hpp_

src/slic3r/GUI/GUI_App.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <iterator>
2222
#include <exception>
2323
#include <cstdlib>
24+
#include <chrono>
2425
#include <regex>
2526
#include <thread>
2627
#include <string_view>
@@ -55,6 +56,7 @@
5556
#include <wx/glcanvas.h>
5657

5758
#include "libslic3r/Utils.hpp"
59+
#include "libslic3r/Debounce.hpp"
5860
#include "libslic3r/Model.hpp"
5961
#include "libslic3r/I18N.hpp"
6062
#include "libslic3r/LogSink.hpp"
@@ -2774,6 +2776,10 @@ int GUI_App::OnExit()
27742776
m_agent = nullptr;
27752777
}
27762778

2779+
// Flush any config changes that were deferred by the idle-handler debounce.
2780+
if (app_config && app_config->dirty())
2781+
app_config->save();
2782+
27772783
return wxApp::OnExit();
27782784
}
27792785

@@ -3314,8 +3320,13 @@ bool GUI_App::on_init_inner()
33143320
if (! plater_)
33153321
return;
33163322

3317-
if (app_config->dirty())
3318-
app_config->save();
3323+
if (app_config->dirty()) {
3324+
// Debounce: avoid synchronous disk writes on every idle event.
3325+
// Save at most once per 5 seconds; OnExit() flushes any remaining dirty state.
3326+
static auto s_last_config_save = std::chrono::steady_clock::time_point{};
3327+
if (Slic3r::debounce_elapsed(s_last_config_save, std::chrono::seconds(5)))
3328+
app_config->save();
3329+
}
33193330

33203331
// BBS
33213332
//this->obj_manipul()->update_if_dirty();

tests/libslic3r/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ add_executable(${_TEST_NAME}_tests
2222
test_png_io.cpp
2323
test_timeutils.cpp
2424
test_indexed_triangle_set.cpp
25+
test_debounce.cpp
2526
../libnest2d/printer_parts.cpp
2627
)
2728

tests/libslic3r/test_debounce.cpp

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#include <catch2/catch.hpp>
2+
3+
#include "libslic3r/Debounce.hpp"
4+
5+
#include <thread>
6+
7+
using namespace Slic3r;
8+
using namespace std::chrono;
9+
10+
// ---------------------------------------------------------------------------
11+
// debounce_elapsed() — unit tests
12+
// ---------------------------------------------------------------------------
13+
14+
SCENARIO("debounce_elapsed: first call always fires", "[Debounce]") {
15+
GIVEN("A default-initialised time_point (epoch)") {
16+
steady_clock::time_point last{};
17+
WHEN("debounce_elapsed is called with a 5-second interval") {
18+
const bool fired = debounce_elapsed(last, seconds(5));
19+
THEN("It returns true") {
20+
REQUIRE(fired);
21+
}
22+
THEN("last_tp is updated to a non-epoch value") {
23+
REQUIRE(last != steady_clock::time_point{});
24+
}
25+
}
26+
}
27+
}
28+
29+
SCENARIO("debounce_elapsed: second immediate call is suppressed", "[Debounce]") {
30+
GIVEN("A time_point primed by a first accepted call") {
31+
steady_clock::time_point last{};
32+
debounce_elapsed(last, seconds(5)); // prime
33+
WHEN("debounce_elapsed is called again immediately") {
34+
const bool fired = debounce_elapsed(last, seconds(5));
35+
THEN("It returns false") {
36+
REQUIRE_FALSE(fired);
37+
}
38+
}
39+
}
40+
}
41+
42+
SCENARIO("debounce_elapsed: call fires again after interval expires", "[Debounce]") {
43+
GIVEN("A time_point set to 10 seconds in the past") {
44+
// Simulate 'last' being 10 seconds old without sleeping.
45+
steady_clock::time_point last = steady_clock::now() - seconds(10);
46+
WHEN("debounce_elapsed is called with a 5-second interval") {
47+
const bool fired = debounce_elapsed(last, seconds(5));
48+
THEN("It returns true") {
49+
REQUIRE(fired);
50+
}
51+
THEN("last_tp is updated") {
52+
REQUIRE(last >= steady_clock::now() - milliseconds(100));
53+
}
54+
}
55+
}
56+
}
57+
58+
SCENARIO("debounce_elapsed: zero interval fires every time", "[Debounce]") {
59+
GIVEN("A time_point primed by a first call") {
60+
steady_clock::time_point last{};
61+
debounce_elapsed(last, seconds(0));
62+
WHEN("debounce_elapsed is called immediately again with zero interval") {
63+
const bool fired = debounce_elapsed(last, seconds(0));
64+
THEN("It returns true") {
65+
REQUIRE(fired);
66+
}
67+
}
68+
}
69+
}
70+
71+
SCENARIO("debounce_elapsed: last_tp is not modified on suppressed calls", "[Debounce]") {
72+
GIVEN("A time_point primed by a first accepted call") {
73+
steady_clock::time_point last{};
74+
debounce_elapsed(last, seconds(5));
75+
const auto snapshot = last;
76+
WHEN("A suppressed call is made") {
77+
debounce_elapsed(last, seconds(5));
78+
THEN("last_tp is unchanged") {
79+
REQUIRE(last == snapshot);
80+
}
81+
}
82+
}
83+
}

0 commit comments

Comments
 (0)