Skip to content

Commit

Permalink
[JSC] sourceURLStripped should be cached
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259969
rdar://113616170

Reviewed by Keith Miller.

262420@main introduced sourceURLStripped for error stack URLs. However this is really costly operation,
as a result, it makes error stack string generation slower. JetStream2/chai-wtb is frequently creating
this error stack string, so this is affected by this change by up to 5%.

This patch fixes the above performance issue by caching sourceURLStripped in SourceProvider.

* Source/JavaScriptCore/parser/SourceProvider.cpp:
(JSC::SourceProvider::sourceURLStripped):
* Source/JavaScriptCore/parser/SourceProvider.h:
* Source/JavaScriptCore/runtime/ScriptExecutable.h:
(JSC::ScriptExecutable::sourceURLStripped const):
* Source/JavaScriptCore/runtime/StackFrame.cpp:
(JSC::processSourceURL):
(JSC::StackFrame::sourceURL const):
(JSC::StackFrame::sourceURLStripped const):

Canonical link: https://commits.webkit.org/266728@main
  • Loading branch information
Constellation committed Aug 9, 2023
1 parent 9790360 commit 6fb2754
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 11 deletions.
10 changes: 10 additions & 0 deletions Source/JavaScriptCore/parser/SourceProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ void SourceProvider::getID()
}
}

const String& SourceProvider::sourceURLStripped()
{
if (UNLIKELY(m_sourceURL.isNull()))
return m_sourceURLStripped;
if (LIKELY(!m_sourceURLStripped.isNull()))
return m_sourceURLStripped;
m_sourceURLStripped = URL(m_sourceURL).strippedForUseAsReport();
return m_sourceURLStripped;
}

#if ENABLE(WEBASSEMBLY)
BaseWebAssemblySourceProvider::BaseWebAssemblySourceProvider(const SourceOrigin& sourceOrigin, String&& sourceURL)
: SourceProvider(sourceOrigin, WTFMove(sourceURL), String(), TextPosition(), SourceProviderSourceType::WebAssembly)
Expand Down
2 changes: 2 additions & 0 deletions Source/JavaScriptCore/parser/SourceProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class UnlinkedFunctionCodeBlock;

// This is NOT the path that should be used for computing relative paths from a script. Use SourceOrigin's URL for that, the values may or may not be the same...
const String& sourceURL() const { return m_sourceURL; }
const String& sourceURLStripped();
const String& preRedirectURL() const { return m_preRedirectURL; }
const String& sourceURLDirective() const { return m_sourceURLDirective; }
const String& sourceMappingURLDirective() const { return m_sourceMappingURLDirective; }
Expand All @@ -98,6 +99,7 @@ class UnlinkedFunctionCodeBlock;
SourceProviderSourceType m_sourceType;
SourceOrigin m_sourceOrigin;
String m_sourceURL;
String m_sourceURLStripped;
String m_preRedirectURL;
String m_sourceURLDirective;
String m_sourceMappingURLDirective;
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/runtime/ScriptExecutable.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ScriptExecutable : public ExecutableBase {
const SourceOrigin& sourceOrigin() const { return m_source.provider()->sourceOrigin(); }
// This is NOT the path that should be used for computing relative paths from a script. Use SourceOrigin's URL for that, the values may or may not be the same... This should only be used for `error.sourceURL` and stack traces.
const String& sourceURL() const { return m_source.provider()->sourceURL(); }
const String& sourceURLStripped() const { return m_source.provider()->sourceURLStripped(); }
const String& preRedirectURL() const { return m_source.provider()->preRedirectURL(); }
int firstLine() const { return m_source.firstLine().oneBasedInt(); }
JS_EXPORT_PRIVATE int lastLine() const;
Expand Down
31 changes: 20 additions & 11 deletions Source/JavaScriptCore/runtime/StackFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,10 @@ SourceID StackFrame::sourceID() const
return m_codeBlock->ownerExecutable()->sourceID();
}

String StackFrame::sourceURL(VM& vm) const
static String processSourceURL(VM& vm, const JSC::StackFrame& frame, const String& sourceURL)
{
if (m_isWasmFrame)
return "[wasm code]"_s;

if (!m_codeBlock)
return "[native code]"_s;

String sourceURL = m_codeBlock->ownerExecutable()->sourceURL();

if (vm.clientData && !sourceURL.startsWithIgnoringASCIICase("http"_s)) {
String overrideURL = vm.clientData->overrideSourceURL(*this, sourceURL);
String overrideURL = vm.clientData->overrideSourceURL(frame, sourceURL);
if (!overrideURL.isNull())
return overrideURL;
}
Expand All @@ -79,9 +71,26 @@ String StackFrame::sourceURL(VM& vm) const
return emptyString();
}

String StackFrame::sourceURL(VM& vm) const
{
if (m_isWasmFrame)
return "[wasm code]"_s;

if (!m_codeBlock)
return "[native code]"_s;

return processSourceURL(vm, *this, m_codeBlock->ownerExecutable()->sourceURL());
}

String StackFrame::sourceURLStripped(VM& vm) const
{
return URL(sourceURL(vm)).strippedForUseAsReport();
if (m_isWasmFrame)
return "[wasm code]"_s;

if (!m_codeBlock)
return "[native code]"_s;

return processSourceURL(vm, *this, m_codeBlock->ownerExecutable()->sourceURLStripped());
}

String StackFrame::functionName(VM& vm) const
Expand Down

0 comments on commit 6fb2754

Please sign in to comment.