From 8e960abea52cfaba69587f60ea15be53897b0530 Mon Sep 17 00:00:00 2001 From: jingkaimori Date: Fri, 12 Jul 2024 23:30:26 +0800 Subject: [PATCH 1/3] Revert "[8_12] Revert: improve strategy of expanding string for performance" This reverts commit d4cfb60e573747b54e22c8434e2b955e50b2f963. --- Kernel/Types/string.cpp | 56 +++++++++++++++++---- Kernel/Types/string.hpp | 24 ++++++++- bench/Kernel/Types/string_bench.cpp | 2 +- tests/Kernel/Types/string_test.cpp | 75 +++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 12 deletions(-) diff --git a/Kernel/Types/string.cpp b/Kernel/Types/string.cpp index ab5ba550..897b6ef0 100644 --- a/Kernel/Types/string.cpp +++ b/Kernel/Types/string.cpp @@ -32,7 +32,7 @@ round_length (int n) { } string_rep::string_rep (int n2) - : n (n2), + : n (n2), a_N (round_length (n)), a ((n == 0) ? ((char*) NULL) : tm_new_array (round_length (n))) {} void @@ -42,17 +42,53 @@ string_rep::resize (int m) { if (mm != nn) { if (mm != 0) { if (nn != 0) { - a= tm_resize_array (mm, a); + a_N= mm; + a = tm_resize_array (mm, a); } else { - a= tm_new_array (mm); + a_N= mm; + a = tm_new_array (mm); } } - else if (nn != 0) tm_delete_array (a); + else if (nn != 0) { + tm_delete_array (a); + a_N= 0; + a = NULL; + }; } n= m; } +int +string_rep::expand_or_shrink_by (int delta) { + int old_n= n; + n+= delta; + reserve (n); + return old_n; +} + +void +string_rep::reserve (int new_n) { + int old_size= a_N; + if (new_n != 0) { + if (old_size == 0) { + a_N= round_length (new_n); + a = tm_new_array (a_N); + } + else if (old_size < new_n) { + a_N= round_length (new_n); + a = tm_resize_array (a_N, a); + } + } + else { + if (old_size != 0) { + tm_delete_array (a); + a = NULL; + a_N= 0; + }; + } +} + string::string (char c) { rep = tm_new (1); rep->a[0]= c; @@ -138,17 +174,17 @@ copy (string s) { string& operator<< (string& a, char x) { - a->resize (N (a) + 1); - a[N (a) - 1]= x; + int old_a_N= a->expand_or_shrink_by (1); + a[old_a_N] = x; return a; } string& operator<< (string& a, string b) { - int i, k1= N (a), k2= N (b); - a->resize (k1 + k2); - for (i= 0; i < k2; i++) - a[i + k1]= b[i]; + int b_N = N (b); + int old_a_N= a->expand_or_shrink_by (b_N); + for (int i= 0; i < b_N; i++) + a[i + old_a_N]= b[i]; return a; } diff --git a/Kernel/Types/string.hpp b/Kernel/Types/string.hpp index 7274d17f..eb16a639 100644 --- a/Kernel/Types/string.hpp +++ b/Kernel/Types/string.hpp @@ -23,16 +23,38 @@ using lolly::data::string_view; class string; class string_rep : concrete_struct { int n; + int a_N; char* a; public: - inline string_rep () : n (0), a (NULL) {} + inline string_rep () : n (0), a_N (0), a (NULL) {} string_rep (int n); inline ~string_rep () { if (n != 0) tm_delete_array (a); } + /** + * @brief expand (or shrink) string by delta, but do not release memory when + * string is shrinked. + * + * @return string length before expansion + */ + int expand_or_shrink_by (int delta); + + /** + * @brief expand (or shrink) string to given length n, and try to release + * memory when string is shrinked. + * + * @note expand_or_shrink_by may be faster if memory space is reserved + */ void resize (int n); + /** + * @brief reserve memory space to contain at least n word in the whole string. + * Do not affect length of string, and do not release memory when n is smaller + * than current space. + */ + void reserve (int n); + friend class string; friend inline int N (string a); }; diff --git a/bench/Kernel/Types/string_bench.cpp b/bench/Kernel/Types/string_bench.cpp index a0608172..4e52f323 100644 --- a/bench/Kernel/Types/string_bench.cpp +++ b/bench/Kernel/Types/string_bench.cpp @@ -14,6 +14,7 @@ static ankerl::nanobench::Bench bench; int main () { lolly::init_tbox (); + bench.minEpochTime (std::chrono::milliseconds (20)); bench.run ("construct string", [&] { string ("abc"); string (); @@ -43,7 +44,6 @@ main () { static string a ("abcdefgh"); a (1, 6); }); - bench.minEpochIterations (40000); bench.run ("concat string", [&] { static string a ("abc"), b ("de"); a* b; diff --git a/tests/Kernel/Types/string_test.cpp b/tests/Kernel/Types/string_test.cpp index 85675b38..946e5665 100644 --- a/tests/Kernel/Types/string_test.cpp +++ b/tests/Kernel/Types/string_test.cpp @@ -64,6 +64,81 @@ TEST_CASE ("test append") { CHECK_EQ (str == string ("xyz"), true); } +TEST_CASE ("test append") { + auto str= string (); + str << 'x'; + CHECK (str == "x"); + str << string ("yz"); + CHECK (str == "xyz"); +} + +TEST_CASE ("test reserve along with append") { + + SUBCASE ("reserved more space") { + auto str= string (); + str->reserve (6); + str << 'x'; + CHECK_EQ (str == "x", true); + str << string ("yz"); + CHECK_EQ (str == "xyz", true); + str << string (": larger than reserved space"); + CHECK_EQ (str == "xyz: larger than reserved space", true); + } + SUBCASE ("reserved the same space") { + auto str= string ("abc"); + str->reserve (3); + CHECK_EQ (str == "abc", true); + } + SUBCASE ("reserved less space should take no effect") { + auto str= string ("abc"); + str->reserve (2); + CHECK_EQ (str == "abc", true); + } +} + +TEST_CASE ("test expand_or_shrink_by along with sub index") { + + SUBCASE ("expand") { + auto str = string ("abc"); + int previous_n= str->expand_or_shrink_by (1); + CHECK_EQ (previous_n, 3); + str[3]= 'd'; + CHECK_EQ (str == "abcd", true); + } + SUBCASE ("shrink") { + auto str = string ("abc"); + int previous_n= str->expand_or_shrink_by (-1); + CHECK_EQ (previous_n, 3); + CHECK_EQ (str == "ab", true); + } + SUBCASE ("delta 0 takes no effect") { + auto str = string ("abc"); + int previous_n= str->expand_or_shrink_by (0); + CHECK_EQ (previous_n, 3); + CHECK_EQ (str == "abc", true); + } +} + +TEST_CASE ("test resize") { + + SUBCASE ("expand") { + auto str= string ("abc"); + str->resize (4); + str[3]= 'd'; + CHECK_EQ (str == "abcd", true); + } + SUBCASE ("shrink") { + auto str= string ("abc"); + str->resize (2); + CHECK_EQ (str == "ab", true); + } + SUBCASE ("delta 0 takes no effect") { + auto str= string ("abc"); + str->resize (3); + CHECK_EQ (str == "abc", true); + } +} + /****************************************************************************** * Conversions ******************************************************************************/ From 287960252f2a36843af3fe8bfed8940f5060c459 Mon Sep 17 00:00:00 2001 From: jingkaimori Date: Sat, 13 Jul 2024 00:04:48 +0800 Subject: [PATCH 2/3] adjust string layout --- Kernel/Types/string.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Kernel/Types/string.hpp b/Kernel/Types/string.hpp index eb16a639..1099c262 100644 --- a/Kernel/Types/string.hpp +++ b/Kernel/Types/string.hpp @@ -23,8 +23,8 @@ using lolly::data::string_view; class string; class string_rep : concrete_struct { int n; - int a_N; char* a; + int a_N; public: inline string_rep () : n (0), a_N (0), a (NULL) {} From 0a9770684e9c96110b99831aa2fba1f46e4696b8 Mon Sep 17 00:00:00 2001 From: jingkaimori Date: Sat, 13 Jul 2024 21:41:18 +0800 Subject: [PATCH 3/3] improve performance of string initialization --- Kernel/Types/string.cpp | 8 +++++--- Kernel/Types/string.hpp | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Kernel/Types/string.cpp b/Kernel/Types/string.cpp index 897b6ef0..c5da6504 100644 --- a/Kernel/Types/string.cpp +++ b/Kernel/Types/string.cpp @@ -31,9 +31,11 @@ round_length (int n) { return i; } -string_rep::string_rep (int n2) - : n (n2), a_N (round_length (n)), - a ((n == 0) ? ((char*) NULL) : tm_new_array (round_length (n))) {} +string_rep::string_rep (int n2) : n (n2), a_N (round_length (n2)), a (nullptr) { + if (a_N != 0) { + a= tm_new_array (a_N); + } +} void string_rep::resize (int m) { diff --git a/Kernel/Types/string.hpp b/Kernel/Types/string.hpp index 1099c262..40b10b0d 100644 --- a/Kernel/Types/string.hpp +++ b/Kernel/Types/string.hpp @@ -30,7 +30,7 @@ class string_rep : concrete_struct { inline string_rep () : n (0), a_N (0), a (NULL) {} string_rep (int n); inline ~string_rep () { - if (n != 0) tm_delete_array (a); + if (a_N != 0) tm_delete_array (a); } /** * @brief expand (or shrink) string by delta, but do not release memory when