compiler: fix the quadratic — strlen() on every character access
El SDK CI - dev / build-and-test (pull_request) Failing after 10m21s
El SDK CI - dev / build-and-test (pull_request) Failing after 10m21s
THE BUG. str_char_code() and str_slice() each called strlen() on every
invocation. The lexer walks source one character at a time, so every character
access rescanned the whole remaining input: O(n) per character over n
characters = O(n^2).
el_val_t str_char_code(el_val_t s, el_val_t i) {
...
int64_t n = (int64_t)strlen(str); // <- O(n), every call
if (idx < 0 || idx >= n) return 0;
return str[idx];
}
HOW IT WAS FOUND. Not by reading code — by sampling the running process, which
is the same method that resolved tonight's engram outage after four wrong
theories. A geometric sweep of synthetic sources showed wall-clock rising 3.0x,
3.0x, 4.0x, 4.14x per doubling (converging on 4x = quadratic), and a stack
sample put 779 of 779 samples inside lex(), every one bottoming out in
_platform_strlen via str_char_code and str_slice.
THE FIX. Remember the length instead of recomputing it. The subtlety is
INVALIDATION: El strings are arena-allocated, so a freed pointer can be reused
for a different string at the same address, and a naive pointer-keyed cache
would hand back a stale length and read past the end of the new string —
trading a performance bug for a memory-safety one. So entries carry a
generation, a hit requires pointer AND generation to match, and every path that
frees or mutates a runtime string bumps the generation: el_arena_pop,
seed_request_end, __str_set_char. Stale entries cannot be believed; they miss
and recompute.
MEASURED, same host, same inputs:
n(fns) before after
512 0.10s 0.01s
1024 0.37s 0.02s
2048 1.51s 0.03s 50x
the compiler's own 422 KB source concatenated (DESIGN.md's 3.58s case):
3.55s -> 0.03s 118x
The speedup GROWS with input size, which is the signature of removing a
complexity class rather than a constant factor. After the fix each doubling
adds ~0.01s: linear.
CORRECTNESS, verified rather than assumed:
- byte-identical output on every sweep input (n = 128..2048)
- byte-identical output on the 422 KB compiler concatenation
- byte-identical output on tests/runtime/string_test.el
- self-hosting fixpoint byte-identical
- new tests/runtime/str_cache_test.el: 17 assertions covering bounds, empty
strings, negative indices, slice clamping, distinct strings not sharing a
cached length, 1000 interleaved strings forcing cache-slot collisions, and
a grown string not reporting its old length. All pass.
This is the defect that made dist/soul.c a committed artifact: elc could not run
in CI because it needed 24 GB+ and minutes. It needs neither now.
This commit is contained in:
@@ -140,6 +140,45 @@ el_val_t el_arena_push(void) {
|
||||
return (el_val_t)(int64_t)_tl_arena.count;
|
||||
}
|
||||
|
||||
/* ── String-length cache ─────────────────────────────────────────────────────
|
||||
*
|
||||
* THE COMPILER'S QUADRATIC LIVED HERE. str_char_code and str_slice each called
|
||||
* strlen() on every invocation. The lexer walks source one character at a time,
|
||||
* so每 access rescanned the whole remaining input: O(n) per character over n
|
||||
* characters = O(n^2). Measured on a geometric sweep of synthetic sources,
|
||||
* wall-clock rose 3.0x, 3.0x, 4.0x, 4.14x per doubling — converging on 4x, a
|
||||
* textbook quadratic — and a stack sample put 779 of 779 samples inside lex(),
|
||||
* every one bottoming out in _platform_strlen.
|
||||
*
|
||||
* The fix is to remember the length instead of recomputing it. The subtlety is
|
||||
* INVALIDATION: El strings are arena-allocated, so a freed pointer can be
|
||||
* reused for a different string at the same address. A naive pointer-keyed
|
||||
* cache would then hand back a stale length and read past the end of the new
|
||||
* string — trading a performance bug for a memory-safety one.
|
||||
*
|
||||
* So entries carry a generation. Anything that frees or mutates runtime strings
|
||||
* bumps the generation, and a cache hit requires both the pointer AND the
|
||||
* generation to match. Stale entries can never be believed; they simply miss
|
||||
* and recompute.
|
||||
* ──────────────────────────────────────────────────────────────────────────── */
|
||||
#define EL_SLC_SLOTS 8
|
||||
typedef struct { const char* ptr; size_t len; uint64_t gen; } ElStrLenEnt;
|
||||
static ElStrLenEnt _el_slc[EL_SLC_SLOTS];
|
||||
static uint64_t _el_str_gen = 1;
|
||||
|
||||
/* Called by every path that frees or mutates a runtime string. */
|
||||
void el_str_cache_flush(void) { _el_str_gen++; }
|
||||
|
||||
static size_t el_strlen_cached(const char* s) {
|
||||
if (!s) return 0;
|
||||
size_t slot = ((uintptr_t)s >> 4) & (EL_SLC_SLOTS - 1);
|
||||
ElStrLenEnt* e = &_el_slc[slot];
|
||||
if (e->ptr == s && e->gen == _el_str_gen) return e->len;
|
||||
size_t n = strlen(s);
|
||||
e->ptr = s; e->len = n; e->gen = _el_str_gen;
|
||||
return n;
|
||||
}
|
||||
|
||||
el_val_t el_arena_pop(el_val_t mark) {
|
||||
size_t save = (size_t)(int64_t)mark;
|
||||
if (save > _tl_arena.count) save = 0;
|
||||
@@ -152,6 +191,7 @@ el_val_t el_arena_pop(el_val_t mark) {
|
||||
_tl_arena.count = save;
|
||||
if (_tl_arena_scope_depth > 0) _tl_arena_scope_depth--;
|
||||
if (save == 0) _tl_arena_active = 0;
|
||||
el_str_cache_flush(); /* freed pointers may be reused — see cache note */
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -309,7 +349,7 @@ el_val_t str_to_int(el_val_t sv) {
|
||||
el_val_t str_slice(el_val_t sv, el_val_t start, el_val_t end) {
|
||||
const char* s = EL_CSTR(sv);
|
||||
if (!s) return el_wrap_str(el_strdup(""));
|
||||
int64_t len = (int64_t)strlen(s);
|
||||
int64_t len = (int64_t)el_strlen_cached(s);
|
||||
if (start < 0) start = 0;
|
||||
if (end > len) end = len;
|
||||
if (start >= end) return el_wrap_str(el_strdup(""));
|
||||
@@ -5257,7 +5297,7 @@ el_val_t str_char_code(el_val_t s, el_val_t i) {
|
||||
const char* str = EL_CSTR(s);
|
||||
int64_t idx = (int64_t)i;
|
||||
if (!str) return 0;
|
||||
int64_t n = (int64_t)strlen(str);
|
||||
int64_t n = (int64_t)el_strlen_cached(str);
|
||||
if (idx < 0 || idx >= n) return 0;
|
||||
return (el_val_t)(unsigned char)str[idx];
|
||||
}
|
||||
|
||||
@@ -148,10 +148,17 @@ static void seed_request_start(void) {
|
||||
_seed_arena_on = 1;
|
||||
}
|
||||
|
||||
/* Defined in el_runtime.c. The string-length cache there keys on pointer +
|
||||
* generation; anything that frees or mutates a runtime string must bump the
|
||||
* generation or a reused address could return a stale length. Weak so this
|
||||
* file still links on its own. */
|
||||
__attribute__((weak)) void el_str_cache_flush(void);
|
||||
|
||||
static void seed_request_end(void) {
|
||||
_seed_arena_on = 0;
|
||||
for (size_t i = 0; i < _seed_arena.count; i++) free(_seed_arena.ptrs[i]);
|
||||
_seed_arena.count = 0;
|
||||
if (el_str_cache_flush) el_str_cache_flush(); /* freed pointers may be reused */
|
||||
}
|
||||
|
||||
/* el_request_start / el_request_end — formerly defined in el_runtime.c.
|
||||
@@ -213,6 +220,7 @@ el_val_t __str_set_char(el_val_t s, el_val_t i, el_val_t c) {
|
||||
int64_t idx = (int64_t)i;
|
||||
if (idx < 0 || idx >= len) return s;
|
||||
p[idx] = (char)(unsigned char)(int64_t)c;
|
||||
if (el_str_cache_flush) el_str_cache_flush(); /* in-place write can move the NUL */
|
||||
return s;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
fn expect_int(label: String, got: Int, want: Int) -> Void {
|
||||
if got == want { println("ok " + label) }
|
||||
else { println("FAIL " + label + " got=" + int_to_str(got) + " want=" + int_to_str(want)) }
|
||||
}
|
||||
fn expect_str(label: String, got: String, want: String) -> Void {
|
||||
if str_eq(got, want) { println("ok " + label) }
|
||||
else { println("FAIL " + label + " got='" + got + "' want='" + want + "'") }
|
||||
}
|
||||
|
||||
// 1. basic char access across a string
|
||||
let s: String = "hello"
|
||||
expect_int("char[0]=h", str_char_code(s, 0), 104)
|
||||
expect_int("char[4]=o", str_char_code(s, 4), 111)
|
||||
expect_int("char[5] OOB -> 0", str_char_code(s, 5), 0)
|
||||
expect_int("char[-1] OOB -> 0", str_char_code(s, -1), 0)
|
||||
expect_int("empty string OOB", str_char_code("", 0), 0)
|
||||
|
||||
// 2. slices
|
||||
expect_str("slice(0,5)", str_slice(s, 0, 5), "hello")
|
||||
expect_str("slice(1,3)", str_slice(s, 1, 3), "el")
|
||||
expect_str("slice past end clamps", str_slice(s, 3, 99), "lo")
|
||||
expect_str("slice inverted -> empty", str_slice(s, 4, 2), "")
|
||||
|
||||
// 3. DIFFERENT strings must not share a cached length (the real hazard)
|
||||
let a: String = "abc"
|
||||
let b: String = "abcdefghij"
|
||||
expect_int("a[2]=c", str_char_code(a, 2), 99)
|
||||
expect_int("a[3] OOB", str_char_code(a, 3), 0)
|
||||
expect_int("b[9]=j", str_char_code(b, 9), 106)
|
||||
expect_int("b[3]=d after a", str_char_code(b, 3), 100)
|
||||
expect_int("a[3] still OOB after b", str_char_code(a, 3), 0)
|
||||
|
||||
// 4. many distinct strings interleaved — forces cache slot collisions
|
||||
fn interleave(n: Int) -> Int {
|
||||
let i: Int = 0
|
||||
let bad: Int = 0
|
||||
while i < n {
|
||||
let t: String = int_to_str(i)
|
||||
let l: Int = str_len(t)
|
||||
let last: Int = str_char_code(t, l - 1)
|
||||
let oob: Int = str_char_code(t, l)
|
||||
if oob != 0 { let bad2: Int = bad + 1
|
||||
let bad: Int = bad2 }
|
||||
if last == 0 { let bad3: Int = bad + 1
|
||||
let bad: Int = bad3 }
|
||||
let i2: Int = i + 1
|
||||
let i: Int = i2
|
||||
}
|
||||
return bad
|
||||
}
|
||||
expect_int("1000 interleaved strings, no bad reads", interleave(1000), 0)
|
||||
|
||||
// 5. concatenation changes length — cache must not report the old one
|
||||
let g: String = "12345"
|
||||
let g2: String = g + "6789"
|
||||
expect_int("grown string len via char", str_char_code(g2, 8), 57)
|
||||
expect_int("original still bounded", str_char_code(g, 5), 0)
|
||||
println("done")
|
||||
Reference in New Issue
Block a user