Skip to content

CachedTypes: stop encoding source text into CachedSourceCodeKey #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 2 commits into from
Apr 21, 2026
Merged

CachedTypes: stop encoding source text into CachedSourceCodeKey #186

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3a0896
CachedTypes: stop encoding source text into CachedSourceCodeKey
dylan-conway Apr 20, 2026
60f2994
CachedTypes: return SourceProvider* and skip Base::decode for reuse path
dylan-conway Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions Source/JavaScriptCore/runtime/CachedTypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1644,12 +1644,49 @@ class CachedStringSourceProvider : public CachedSourceProviderShape<StringSource
void encode(Encoder& encoder, const StringSourceProvider& sourceProvider)
{
Base::encode(encoder, sourceProvider);
#if USE(BUN_JSC_ADDITIONS)
// SourceCodeKey::operator== under BUN_JSC_ADDITIONS does not compare source
// text, so encoding it here only wastes ~source_size bytes of bytecode and
// forces a ~source_size heap allocation at decode time. Store length only —
// the comparison still validates length() and host().
m_sourceLength = sourceProvider.source().length();
#else
m_source.encode(encoder, sourceProvider.source().toString());
#endif
}

#if USE(BUN_JSC_ADDITIONS)
// The caller (CachedSourceProvider::decode) returns SourceProvider*, so the
// BUN reuse path can return the runtime provider as its base type without
// any reinterpret_cast through the StringSourceProvider sibling.
SourceProvider* decode(Decoder& decoder, SourceProviderSourceType sourceType) const
#else
StringSourceProvider* decode(Decoder& decoder, SourceProviderSourceType sourceType) const
#endif
{
#if USE(BUN_JSC_ADDITIONS)
// Reuse the runtime SourceProvider the Decoder was constructed with rather
// than allocating a fresh StringSourceProvider holding a heap copy of the
// source. The decoded key is only used for SourceCodeKey equality, which
// under BUN_JSC_ADDITIONS does not look at source bytes.
//
// Base::decode is intentionally skipped: the runtime provider already has
// its sourceURLDirective / sourceMappingURLDirective / sourceTaintedOrigin
// set, and the decoded key only needs sourceOrigin().url().host() and
// length() for equality. CachedSourceProviderShape fields are offset-based
// (not stream-based), so leaving them undecoded does not affect later reads.
if (RefPtr<SourceProvider> provider = decoder.provider()) {
if (provider->sourceType() == sourceType && provider->source().length() == m_sourceLength)
return provider.leakRef();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Fallback for callers that did not supply a provider: decode without source
// bytes. SourceCodeKey::operator== ignores string(), but length() is compared,
// so synthesize a provider whose source() is empty — length() will mismatch
// and the cache entry will be rejected, which is the conservative behaviour.
String decodedSource;
Comment on lines +1682 to +1686

@coderabbitai coderabbitai Bot Apr 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't make key-only decode paths synthesize a zero-length provider.

When decoder.provider() is absent, Lines 1682-1686 fall back to an empty StringSourceProvider, so source().length() becomes 0. That breaks local no-provider key comparisons under USE(BUN_JSC_ADDITIONS): isCachedBytecodeStillValid() still constructs its Decoder without a provider on Line 2779, so decodedKey == key now becomes an unconditional cache miss for every non-empty source. decodeSourceCodeKey() on Line 2740 is lossy for the same reason.

At minimum, thread key.source().provider() into the validity-check path; otherwise this fallback needs to preserve m_sourceLength instead of hard-coding 0.

🔧 Minimal fix for the validity-check path 
 bool isCachedBytecodeStillValid(VM& vm, Ref<CachedBytecode> cachedBytecode, const SourceCodeKey& key, SourceCodeType type)
 {
     auto span = cachedBytecode->span();
     if (span.empty())
         return false;
     auto* cachedEntry = std::bit_cast<const GenericCacheEntry*>(span.data());
-    Ref decoder = Decoder::create(vm, WTF::move(cachedBytecode));
+    Ref decoder = Decoder::create(vm, WTF::move(cachedBytecode), &key.source().provider());
     return cachedEntry->isStillValid(decoder.get(), key, tagFromSourceCodeType(type));
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Source/JavaScriptCore/runtime/CachedTypes.cpp` around lines 1682 - 1686, The
fallback in decodeSourceCodeKey() creates an empty StringSourceProvider (String
decodedSource) so provider()->source().length() becomes 0, causing decodedKey ==
key to always miss when decoder.provider() is absent (affecting
isCachedBytecodeStillValid() and decodeSourceCodeKey()); instead either thread
the original key.source().provider() into the Decoder constructed in the
validity-check path (use key.source().provider() when decoder.provider() is
missing) or, if synthesising a provider, preserve the original source length by
setting m_sourceLength to key.source().length() rather than hard-coding 0;
update decodeSourceCodeKey(), the Decoder construction sites used by
isCachedBytecodeStillValid(), and any StringSourceProvider fallback logic to use
key.source().provider() or preserve m_sourceLength accordingly.

#else
String decodedSource = m_source.decode(decoder);
#endif
SourceOrigin decodedSourceOrigin = m_sourceOrigin.decode(decoder);
String decodedSourceURL = m_sourceURL.decode(decoder);
TextPosition decodedStartPosition = m_startPosition.decode(decoder);
Expand All @@ -1660,7 +1697,11 @@ class CachedStringSourceProvider : public CachedSourceProviderShape<StringSource
}

private:
#if USE(BUN_JSC_ADDITIONS)
unsigned m_sourceLength;
#else
CachedString m_source;
#endif
};

#if ENABLE(WEBASSEMBLY)
Expand Down
Loading