Bytecode cache: inline 1–3 character strings, alias long strings from the payload; follow-ups from #497 review
Inline short strings. Record offsets are multiples of 4 and the empty sentinel ends in binary 11, so a 4-byte string slot whose low two bits are 01 can hold (length, up to 3 Latin-1 chars) instead of the offset of a string record; it decodes straight to AtomStringImpl::add. Applies to identifiers, string constants (CachedJSValue) and every other CachedPtr to a string. Minified code is mostly such names — on the 30 MB / 58-library bundle from #497:
| #497 | this | |
|---|---|---|
| pages touched, decode every top-level function | 49.1 MB | 47.5 MB |
| that decode | 465 ms | 359 ms |
| decode everything | 752 ms | 692 ms |
| cache size | 87.2 MB | 86.5 MB (distinct short strings were already stored once, so the disk win is small) |
Long strings alias the payload. Strings of 48+ characters decoded from a persistent payload use StringImpl::createWithoutCopying over the cached bytes (adopted in place by AtomStringImpl::add), so only the header is allocated and the characters stay in clean file-backed pages; refcounting never touches the payload. ~2–3 MB of string bytes on the bundle above; shorter strings still copy.
Follow-ups from the #497 review (CodeRabbit, post-merge):
CachedJSValue::encode:RELEASE_ASSERTthattryGetValue()resolved (pre-existing// TODO: This seems wrong? What if this fails.) instead of dereferencing a possibly-null impl.bytecodeCachePageTouch(jsc shell test helper): stop when the frontier is empty; clamp the residency delta at 0; keep taking the process-lifetime ref before decoding (a rejected decode may already have borrowers), with a comment saying so.
JSTests/stress through the disk cache (persistent + lean, non-persistent + non-lean decoder): clean.