system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-07-22 10:36:24 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

LibJS: Make Error stack traces lazier

Browse source 
Instead of eagerly populating the stack trace with a textual
representation of every call frame, just store the raw source code range
(code, start offset, end offset). From that, we can generate the full
rich backtrace when requested, and save ourselves the trouble otherwise.

This makes test-wasm take ~7 seconds on my machine instead of ~60. :^)
This commit is contained in:
Andreas Kling 2023-05-28 08:28:43 +02:00
parent 6c81b90e5a
commit 87ac906ee6
8 changed files with 51 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp
View file

@ -36,7 +36,7 @@ void CellSyntaxHighlighter::rehighlight(Palette const& palette)
if (m_cell && m_cell->thrown_value().has_value()) {
if (auto value = m_cell->thrown_value().value(); value.is_object() && is<JS::Error>(value.as_object())) {
auto& error = static_cast<JS::Error const&>(value.as_object());
auto& range = error.traceback().first().source_range;
auto& range = error.traceback().first().source_range();
spans.prepend({
GUI::TextRange { { range.start.line - 1, range.start.column }, { range.end.line - 1, range.end.column } },

2
Userland/Applications/Spreadsheet/Spreadsheet.cpp
View file

@ -70,7 +70,7 @@ Sheet::Sheet(Workbook& workbook)
warnln(" with message '{}'", error.get_without_side_effects(interpreter().vm().names.message));
for (auto& traceback_frame : error.traceback()) {
auto& function_name = traceback_frame.function_name;
auto& source_range = traceback_frame.source_range;
auto& source_range = traceback_frame.source_range();
dbgln(" {} at {}:{}:{}", function_name, source_range.filename(), source_range.start.line, source_range.start.column);
}
} else {

8
Userland/Applications/Spreadsheet/SpreadsheetModel.cpp
View file

@ -122,13 +122,13 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role)
StringBuilder builder;
builder.appendff("{}\n", error.get_without_side_effects(object.vm().names.message).to_string_without_side_effects().release_value_but_fixme_should_propagate_errors());
for (auto const& frame : trace.in_reverse()) {
if (frame.source_range.filename().contains("runtime.js"sv)) {
if (frame.source_range().filename().contains("runtime.js"sv)) {
if (frame.function_name == "<unknown>")
builder.appendff(" in a builtin function at line {}, column {}\n", frame.source_range.start.line, frame.source_range.start.column);
builder.appendff(" in a builtin function at line {}, column {}\n", frame.source_range().start.line, frame.source_range().start.column);
else
builder.appendff(" while evaluating builtin '{}'\n", frame.function_name);
} else if (frame.source_range.filename().starts_with("cell "sv)) {
builder.appendff(" in cell '{}', at line {}, column {}\n", frame.source_range.filename().substring_view(5), frame.source_range.start.line, frame.source_range.start.column);
} else if (frame.source_range().filename().starts_with("cell "sv)) {
builder.appendff(" in cell '{}', at line {}, column {}\n", frame.source_range().filename().substring_view(5), frame.source_range().start.line, frame.source_range().start.column);
}
}
return builder.to_deprecated_string();

3
Userland/Libraries/LibJS/AST.h
View file

@ -58,6 +58,9 @@ public:
[[nodiscard]] SourceRange source_range() const;
u32 start_offset() const { return m_start_offset; }
u32 end_offset() const { return m_end_offset; }
SourceCode const& source_code() const { return *m_source_code; }
void set_end_offset(Badge<Parser>, u32 end_offset) { m_end_offset = end_offset; }

4
Userland/Libraries/LibJS/MarkupGenerator.cpp
View file

@ -150,12 +150,12 @@ ErrorOr<void> MarkupGenerator::date_to_html(Object const& date, StringBuilder& h
ErrorOr<void> MarkupGenerator::trace_to_html(TracebackFrame const& traceback_frame, StringBuilder& html_output)
{
auto function_name = escape_html_entities(traceback_frame.function_name);
auto [line, column, _] = traceback_frame.source_range.start;
auto [line, column, _] = traceback_frame.source_range().start;
auto get_filename_from_path = [&](StringView filename) -> StringView {
auto last_slash_index = filename.find_last('/');
return last_slash_index.has_value() ? filename.substring_view(*last_slash_index + 1) : filename;
};
auto filename = escape_html_entities(get_filename_from_path(traceback_frame.source_range.filename()));
auto filename = escape_html_entities(get_filename_from_path(traceback_frame.source_range().filename()));
auto trace = TRY(String::formatted("at {} ({}:{}:{})", function_name, filename, line, column));
TRY(html_output.try_appendff("&nbsp;&nbsp;{}<br>", trace));

41
Userland/Libraries/LibJS/Runtime/Error.cpp
View file

@ -15,6 +15,15 @@
namespace JS {
SourceRange const& TracebackFrame::source_range() const
{
if (auto* unrealized = source_range_storage.get_pointer<UnrealizedSourceRange>()) {
auto source_range = unrealized->source_code->range_from_offsets(unrealized->start_offset, unrealized->end_offset);
source_range_storage = move(source_range);
}
return source_range_storage.get<SourceRange>();
}
NonnullGCPtr<Error> Error::create(Realm& realm)
{
return realm.heap().allocate<Error>(realm, realm.intrinsics().error_prototype()).release_allocated_value_but_fixme_should_propagate_errors();
@ -69,13 +78,24 @@ void Error::populate_stack()
auto function_name = context->function_name;
if (function_name.is_empty())
function_name = "<unknown>"sv;
m_traceback.empend(
move(function_name),
// We might not have an AST node associated with the execution context, e.g. in promise
// reaction jobs (which aren't called anywhere from the source code).
// They're not going to generate any _unhandled_ exceptions though, so a meaningless
// source range is fine.
context->current_node ? context->current_node->source_range() : dummy_source_range);
TracebackFrame frame {
.function_name = move(function_name),
.source_range_storage = TracebackFrame::UnrealizedSourceRange {},
};
// We might not have an AST node associated with the execution context, e.g. in promise
// reaction jobs (which aren't called anywhere from the source code).
// They're not going to generate any _unhandled_ exceptions though, so a meaningless
// source range is fine.
if (context->current_node) {
auto* unrealized = frame.source_range_storage.get_pointer<TracebackFrame::UnrealizedSourceRange>();
unrealized->source_code = context->current_node->source_code();
unrealized->start_offset = context->current_node->start_offset();
unrealized->end_offset = context->current_node->end_offset();
}
m_traceback.append(move(frame));
}
}
@ -90,13 +110,14 @@ ThrowCompletionOr<String> Error::stack_string(VM& vm) const
for (size_t i = 0; i < m_traceback.size() - 1; ++i) {
auto const& frame = m_traceback[i];
auto function_name = frame.function_name;
auto source_range = frame.source_range();
// Note: Since we don't know whether we have a valid SourceRange here we just check for some default values.
if (!frame.source_range.filename().is_empty() || frame.source_range.start.offset != 0 || frame.source_range.end.offset != 0) {
if (!source_range.filename().is_empty() || source_range.start.offset != 0 || source_range.end.offset != 0) {
if (function_name == "<unknown>"sv)
MUST_OR_THROW_OOM(stack_string_builder.appendff(" at {}:{}:{}\n", frame.source_range.filename(), frame.source_range.start.line, frame.source_range.start.column));
MUST_OR_THROW_OOM(stack_string_builder.appendff(" at {}:{}:{}\n", source_range.filename(), source_range.start.line, source_range.start.column));
else
MUST_OR_THROW_OOM(stack_string_builder.appendff(" at {} ({}:{}:{})\n", function_name, frame.source_range.filename(), frame.source_range.start.line, frame.source_range.start.column));
MUST_OR_THROW_OOM(stack_string_builder.appendff(" at {} ({}:{}:{})\n", function_name, source_range.filename(), source_range.start.line, source_range.start.column));
} else {
MUST_OR_THROW_OOM(stack_string_builder.appendff(" at {}\n", function_name.is_empty() ? "<unknown>"sv : function_name.view()));
}

9
Userland/Libraries/LibJS/Runtime/Error.h
View file

@ -17,7 +17,14 @@ namespace JS {
struct TracebackFrame {
DeprecatedFlyString function_name;
SourceRange source_range;
[[nodiscard]] SourceRange const& source_range() const;
struct UnrealizedSourceRange {
u32 start_offset { 0 };
u32 end_offset { 0 };
RefPtr<JS::SourceCode const> source_code;
};
mutable Variant<SourceRange, UnrealizedSourceRange> source_range_storage;
};
class Error : public Object {

2
Userland/Libraries/LibWeb/HTML/Scripting/ExceptionReporter.cpp
View file

@ -33,7 +33,7 @@ void report_exception_to_console(JS::Value value, JS::Realm& realm, ErrorInPromi
auto const& error_value = static_cast<JS::Error const&>(object);
for (auto& traceback_frame : error_value.traceback()) {
auto& function_name = traceback_frame.function_name;
auto& source_range = traceback_frame.source_range;
auto& source_range = traceback_frame.source_range();
dbgln(" {} at {}:{}:{}", function_name, source_range.filename(), source_range.start.line, source_range.start.column);
}
console.report_exception(error_value, error_in_promise == ErrorInPromise::Yes);