gbranden pushed a commit to branch master
in repository groff.
commit 1c14dd46f9c53082fb48b77f97acb99e2946f912
Author: G. Branden Robinson <[email protected]>
AuthorDate: Sat Jun 20 02:47:46 2026 -0500
[libgroff]: Size `string` buffers better.
* src/libs/libgroff/string.cpp: Document invariant.
(salloc, sfree_alloc): Always allocate room for a null terminator
since the groff `string` class's `contents()` member function returns
a C string. That interface does not combine well with a class design
that uses neither memset(3) nor ISO C++03 value initialization is used
to fastidiously clean the allocated buffers (which, as of recently, we
do). This way we wear suspenders _and_ a belt. As a side effect, we
more often {re}allocate in chunks of 64 bytes a time, reducing the
total number of allocations and improving build time by about 5%.
Performance analysis (`make check`)
===================================
Before
------
real 2m53.625s
user 2m46.855s
sys 0m18.341s
After
-----
real 2m44.926s
user 2m36.947s
sys 0m19.019s
The slight increase in system time is likely because this revision more
aggressively allocates 64-byte chunks of backing storage for the
buffers, and, as noted, fastidiously cleans them. This means we're
faulting in "zero pages" rather than reusing "dirty pages" already
managed by the language runtime's memory allocator.
---
ChangeLog | 13 +++++++++++++
src/libs/libgroff/string.cpp | 19 +++++++++----------
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 17591d1aa..ce76b4d9b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2026-06-20 G. Branden Robinson <[email protected]>
+
+ * src/libs/libgroff/string.cpp: Document invariant.
+ (salloc, sfree_alloc): Always allocate room for a null
+ terminator since the groff `string` class's `contents()` member
+ function returns a C string. That interface does not combine
+ well with a class design that uses neither memset(3) nor ISO
+ C++03 value initialization to fastidiously clean the allocated
+ buffers (which, as of recently, we do). This way we wear
+ suspenders _and_ a belt. As a side effect, we more often
+ {re}allocate in chunks of 64 bytes a time, reducing the total
+ number of allocations and improving build time by about 5%.
+
2026-06-20 G. Branden Robinson <[email protected]>
* tmac/tmac.am: Add "tmac/devtag.tmac" to `TMAC_PACKAGE_MS`
diff --git a/src/libs/libgroff/string.cpp b/src/libs/libgroff/string.cpp
index e1d756afd..dddcd6644 100644
--- a/src/libs/libgroff/string.cpp
+++ b/src/libs/libgroff/string.cpp
@@ -44,6 +44,9 @@ along with this program. If not, see
<http://www.gnu.org/licenses/>. */
// TODO 1: Replace all this memory management stuff with vector<char>.
// TODO 2: Replace this entire class. See Savannah #67735.
+// Invariant: a groff string must always be null-terminated, because its
+// `contents()` member function returns a `const char *`.
+
// An initial buffer size of 64 appears to balance build time against
// minimization of reallocations.
static const size_t initial_string_buffer_size = 64;
@@ -51,13 +54,9 @@ static const size_t initial_string_buffer_size = 64;
static char *salloc(size_t len, size_t *sizep)
{
char *p = 0 /* nullptr */;
- size_t amount = len;
- if (0 == amount)
- amount = initial_string_buffer_size;
- // XXX: Add 1 as a hack to work around a problem created in
- // src/devices/html/post-html.cpp:page::add_and_encode_char() when it
- // is omitted. If/when we resolve that, we can assign simply `len`.
- amount += 1;
+ size_t amount = initial_string_buffer_size;
+ if (len >= amount)
+ amount = len + 1 /* '\0' */;
try {
p = new char[*sizep = amount];
}
@@ -93,9 +92,9 @@ static char *sfree_alloc(char *ptr, size_t oldsz, size_t len,
}
delete[] ptr;
char *p = 0 /* nullptr */;
- size_t amount = len;
- if (0 == amount)
- amount = initial_string_buffer_size;
+ size_t amount = initial_string_buffer_size;
+ if (len >= amount)
+ amount = len + 1 /* '\0' */;
try {
p = new char[*sizep = amount];
}
_______________________________________________
groff-commit mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/groff-commit
