From 17af2d406c22f5313e0fd268e2bd8613e175b682 Mon Sep 17 00:00:00 2001 From: Tachikoma Date: Mon, 3 Aug 2026 21:44:16 -0400 Subject: [PATCH] Cut akbasic_Runtime's static footprint from 10.75 MiB to 2.40 MiB Nothing in this interpreter mallocs; every pool is a fixed array sized by an AKBASIC_MAX_* constant, so sizeof(akbasic_Runtime) is a compile-time number and most of it was headroom nobody was using. Measured concurrent-use high-water marks off examples/breakout and examples/megademo -- the two most demanding programs this interpreter runs -- against each pool's ceiling: AKBASIC_MAX_ENVIRONMENTS 32 -> 12 (measured peak concurrency: 6-7) AKBASIC_MAX_FUNCTIONS 64 -> 8 (measured: 0, neither program uses DEF FN) AKBASIC_MAX_ARRAY_VALUES 4096 -> 2048 (measured peak: 1618 slots) AKBASIC_MAX_SOURCE_LINES 9999 -> 2048 (measured: ~1270-1496 non-blank lines) AKBASIC_SYMTAB_MAX_SLOTS 256 -> 172 (no caller ever requests more than 128) AKBASIC_SYMTAB_MAX_KEY 64 -> 24 (longest identifier measured: 11 chars) AKBASIC_MAX_LINE_LENGTH 256 -> 80 (Commodore BASIC's own line limit) AKBASIC_MAX_VARIABLES (128) is untouched on purpose: breakout alone reaches 121 of 128 concurrent named variables, so it has the least slack of any pool measured and is not a shrink candidate. akbasic_Variable.name shrinks from AKBASIC_MAX_STRING_LENGTH (256) to AKBASIC_SYMTAB_MAX_KEY: every variable name is registered with akbasic_symtab_set() right after this field is populated (akbasic_environment_create(), src/environment.c), and that call already refuses anything AKBASIC_SYMTAB_MAX_KEY characters or longer. The wider field was headroom nothing could ever put a byte into. Two defects surfaced while testing the line-length drop against the golden corpus, both fixed here because the 80-byte ceiling makes them routine rather than theoretical: - sourcepath (runtime.h) was borrowing AKBASIC_MAX_LINE_LENGTH by accident. It holds a directory, not a line of BASIC, and this checkout's own test paths are 81+ characters deep -- every golden test failed to load until this split into its own AKBASIC_MAX_SOURCE_PATH_LENGTH, backed by PATH_MAX the way libakerror already sizes its own path buffers. - src/sink_stdio.c's stdio_readline() called aksl_fgets() but never checked its own documented contract: a full buffer with no trailing newline means the line was longer than the buffer, and the unread remainder is still in the stream. Unchecked, the next readline() picks that remainder up as its own statement -- a real line silently becomes two wrong ones instead of a clean AKBASIC_ERR_BOUNDS refusal. At 256 bytes this was theoretical; at 80 it is not, so it now refuses loudly. tests/value_pool.c's test_pool_is_untouched_by_scopes() was pinned to the old 4x1024=4096 pool math (four max-size arrays proving nothing leaked); rewritten to 2x1024=2048 for the same proof against the new AKBASIC_MAX_ARRAY_VALUES. Known consequence, tracked in andrew/akbasic#32 rather than worked around here: two files in the protected tests/reference/ corpus (language/functions/mod.bas, language/flowcontrol/nestedforloopwaitingfor command.bas) have 82-character lines and cannot be shortened -- MAINTENANCE.md and CMakeLists.txt:585 are explicit that tests/reference/ is never edited to suit this interpreter. Twelve tests/language/ cases and one docs/18 line are in the same position but are this project's own content. Shipping 80 anyway, with the fallout tracked rather than hidden, was an explicit call on this PR rather than something decided here. Verified: cmake --build build-akgl && ctest --test-dir build-akgl, 97/112 (15 known failures, all AKBASIC_MAX_LINE_LENGTH-related, filed as #32). Co-Authored-By: Claude Sonnet 5 Co-Authored-By: Andrew Kesterson --- include/akbasic/runtime.h | 8 ++++++-- include/akbasic/symtab.h | 12 ++++++++++-- include/akbasic/types.h | 37 +++++++++++++++++++++++++++++++------ include/akbasic/variable.h | 12 +++++++++++- src/runtime.c | 2 +- src/sink_stdio.c | 13 +++++++++++++ tests/value_pool.c | 10 ++++------ 7 files changed, 76 insertions(+), 18 deletions(-) diff --git a/include/akbasic/runtime.h b/include/akbasic/runtime.h index f828d69..8de41fa 100644 --- a/include/akbasic/runtime.h +++ b/include/akbasic/runtime.h @@ -211,8 +211,12 @@ typedef struct akbasic_Runtime * not exist relative to the working directory, which is what makes a `.bas` * beside its art work from anywhere. Group F's disk verbs will want the * same, which is why it is on the runtime rather than in the sprite state. + * + * Sized to AKBASIC_MAX_SOURCE_PATH_LENGTH, not AKBASIC_MAX_LINE_LENGTH: a + * directory is a filesystem path, not a line of BASIC, and the two do not + * belong to the same budget. */ - char sourcepath[AKBASIC_MAX_LINE_LENGTH]; + char sourcepath[AKBASIC_MAX_SOURCE_PATH_LENGTH]; /* * The armed interrupts, and the environment the one currently running was @@ -365,7 +369,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_set_ui(akbasic_Runtime *obj, * @param path Path to the program file, or NULL for none. * @return `NULL` on success, otherwise an error context owned by the caller. * @throws AKERR_NULLPOINTER When `obj` is NULL. - * @throws AKBASIC_ERR_BOUNDS When the path is longer than AKBASIC_MAX_LINE_LENGTH. + * @throws AKBASIC_ERR_BOUNDS When the path is longer than AKBASIC_MAX_SOURCE_PATH_LENGTH. */ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_set_source_path(akbasic_Runtime *obj, const char *path); diff --git a/include/akbasic/symtab.h b/include/akbasic/symtab.h index c636cf3..63af692 100644 --- a/include/akbasic/symtab.h +++ b/include/akbasic/symtab.h @@ -23,9 +23,17 @@ * serves variables, functions and labels. Capacity is a member; the table is * kept below a 75% load factor by construction because `capacity` counts slots * and the caller's logical maximum is smaller. + * + * 172 rather than the old 256: no caller passes akbasic_symtab_init() anything + * larger than AKBASIC_MAX_VARIABLES (128), and 172 keeps that under the 75% + * load factor the comment above promises (128 / 0.75 = 170.7). + * + * AKBASIC_SYMTAB_MAX_KEY 24 rather than 64: the longest identifier across + * examples/breakout and examples/megademo -- variables, DIMmed arrays, labels + * and DEF FN names alike -- is 11 characters (TITLESCREEN, CLEARPOWERS). */ -#define AKBASIC_SYMTAB_MAX_SLOTS 256 -#define AKBASIC_SYMTAB_MAX_KEY 64 +#define AKBASIC_SYMTAB_MAX_SLOTS 172 +#define AKBASIC_SYMTAB_MAX_KEY 24 typedef struct { diff --git a/include/akbasic/types.h b/include/akbasic/types.h index 97bfad2..49484fb 100644 --- a/include/akbasic/types.h +++ b/include/akbasic/types.h @@ -6,6 +6,7 @@ #ifndef _AKBASIC_TYPES_H_ #define _AKBASIC_TYPES_H_ +#include #include #include @@ -27,15 +28,39 @@ #define AKBASIC_MAX_VALUES 64 #define AKBASIC_MAX_VARIABLES 128 -/* Whole-runtime pools */ -#define AKBASIC_MAX_SOURCE_LINES 9999 -#define AKBASIC_MAX_LINE_LENGTH 256 +/* + * Whole-runtime pools. + * + * AKBASIC_MAX_SOURCE_LINES, AKBASIC_MAX_LINE_LENGTH, AKBASIC_MAX_ARRAY_VALUES, + * AKBASIC_MAX_ENVIRONMENTS and AKBASIC_MAX_FUNCTIONS were cut from their + * original values against measurements taken off examples/breakout and + * examples/megademo, the two most demanding programs this interpreter runs -- + * see the memory-footprint discussion this commit's PR body links. Each is + * sized at roughly 1.5-2x the peak the reference corpus actually reaches, not + * at the peak itself. + * + * AKBASIC_MAX_LINE_LENGTH in particular follows Commodore BASIC's own 80-column + * line limit rather than a measurement, which is why sink_stdio.c now refuses a + * line that fills the buffer with no terminator instead of silently truncating + * it: at 256 bytes that failure mode was theoretical, and at 80 it is not. + * + * AKBASIC_MAX_SOURCE_PATH_LENGTH is its own constant rather than a reuse of + * AKBASIC_MAX_LINE_LENGTH, which is where it used to come from. `sourcepath` + * (runtime.h) holds a directory, not a line of BASIC, and the two ideas do not + * scale together: shrinking the line limit to 80 broke every golden test in + * this checkout, because this repository's own working directory is deeper + * than that. PATH_MAX is the actual bound a filesystem path is subject to, so + * it is the one this borrows. + */ +#define AKBASIC_MAX_SOURCE_LINES 2048 +#define AKBASIC_MAX_LINE_LENGTH 80 /* Commodore BASIC's own line limit */ +#define AKBASIC_MAX_SOURCE_PATH_LENGTH PATH_MAX #define AKBASIC_MAX_ARRAY_DEPTH 64 /* dimensions per array */ #define AKBASIC_MAX_ARRAY_ELEMENTS 1024 /* elements in one array */ -#define AKBASIC_MAX_ARRAY_VALUES 4096 /* array elements across all variables */ +#define AKBASIC_MAX_ARRAY_VALUES 2048 /* array elements across all variables */ #define AKBASIC_MAX_STRING_LENGTH 256 /* see TODO.md 1.2 */ -#define AKBASIC_MAX_ENVIRONMENTS 32 /* new: Go allocated these unbounded */ -#define AKBASIC_MAX_FUNCTIONS 64 /* new: Go used an unbounded map */ +#define AKBASIC_MAX_ENVIRONMENTS 12 /* new: Go allocated these unbounded */ +#define AKBASIC_MAX_FUNCTIONS 8 /* new: Go used an unbounded map */ #define AKBASIC_MAX_LABELS 64 /* new: Go used an unbounded map */ /* * Leaves a DO/LOOP condition may use. Its own small pool rather than a second diff --git a/include/akbasic/variable.h b/include/akbasic/variable.h index 1216585..b4929a8 100644 --- a/include/akbasic/variable.h +++ b/include/akbasic/variable.h @@ -13,12 +13,22 @@ #include +#include #include #include typedef struct { - char name[AKBASIC_MAX_STRING_LENGTH]; + /* + * Sized to AKBASIC_SYMTAB_MAX_KEY, not AKBASIC_MAX_STRING_LENGTH: this name + * only ever gets here by surviving akbasic_symtab_set() first + * (akbasic_environment_create() calls it right after this field is + * populated), and that call refuses anything AKBASIC_SYMTAB_MAX_KEY + * characters or longer with AKBASIC_ERR_BOUNDS. A variable whose name did + * not fit could never exist, so the wider buffer was 232 bytes of headroom + * nothing could ever put a byte into. + */ + char name[AKBASIC_SYMTAB_MAX_KEY]; akbasic_Type valuetype; akbasic_Value *values; /** The pool, or `inlinevalue` for a scalar */ int valuecount; diff --git a/src/runtime.c b/src/runtime.c index 69983ae..0b61e0f 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -301,7 +301,7 @@ akerr_ErrorContext *akbasic_runtime_set_source_path(akbasic_Runtime *obj, const } FAIL_ZERO_RETURN(errctx, (length < sizeof(obj->sourcepath)), AKBASIC_ERR_BOUNDS, "Program path of %zu characters exceeds the %d character limit", - length, AKBASIC_MAX_LINE_LENGTH - 1); + length, AKBASIC_MAX_SOURCE_PATH_LENGTH - 1); PASS(errctx, aksl_memcpy(obj->sourcepath, path, length)); obj->sourcepath[length] = '\0'; SUCCEED_RETURN(errctx); diff --git a/src/sink_stdio.c b/src/sink_stdio.c index 1e50bf3..7c1ab34 100644 --- a/src/sink_stdio.c +++ b/src/sink_stdio.c @@ -74,6 +74,19 @@ static akerr_ErrorContext *stdio_readline(akbasic_TextSink *self, char *dest, si if ( *eof ) { SUCCEED_RETURN(errctx); } + /* + * aksl_fgets(3)'s own contract: a full buffer with no trailing newline is + * how a caller spots a line longer than the buffer, because the rest of it + * is still sitting unread in the stream. Refusing here is what makes that + * true -- without it, the unread remainder is picked up by the *next* + * readline() as if it were its own statement, which does not fail, it just + * runs the wrong program. AKBASIC_MAX_LINE_LENGTH is small enough now that + * this is not a hypothetical: examples/breakout's own longest line used to + * clear the old 256-byte ceiling by more than half. + */ + FAIL_NONZERO_RETURN(errctx, (used == len - 1 && dest[used - 1] != '\n' && dest[used - 1] != '\r'), + AKBASIC_ERR_BOUNDS, + "Source line exceeds the %zu character limit", len - 1); /* * Strip the line terminator. The scanner treats \r and \n as end-of-line * anyway, but leaving them on would make a stored source line differ from diff --git a/tests/value_pool.c b/tests/value_pool.c index 51d7ed7..aec9ff6 100644 --- a/tests/value_pool.c +++ b/tests/value_pool.c @@ -97,9 +97,9 @@ static void test_scoped_loop_counter_costs_nothing(void) /** * @brief The pool is genuinely untouched, not merely large enough. * - * Two hundred scope entries and then the pool's *entire* width -- four arrays, + * Two hundred scope entries and then the pool's *entire* width -- two arrays, * because AKBASIC_MAX_ARRAY_ELEMENTS caps any one of them at 1024 while the pool - * holds 4096. One leaked slot and the fourth `DIM` has nowhere to go, which + * holds 2048. One leaked slot and the second `DIM` has nowhere to go, which * makes this the sharpest of the three and by far the cheapest: the two above * prove a program finishes, this proves nothing at all was spent. The long ones * stay because they are the shape the defect was found in. @@ -111,10 +111,8 @@ static void test_pool_is_untouched_by_scopes(void) "30 NEXT T#\n" "40 DIM A#(1024)\n" "50 DIM B#(1024)\n" - "60 DIM C#(1024)\n" - "70 DIM D#(1024)\n" - "80 D#(1023) = 7\n" - "90 PRINT D#(1023)\n" + "70 B#(1023) = 7\n" + "90 PRINT B#(1023)\n" "100 END\n" "110 LABEL SUBA\n" "120 LOC# = 1\n"