Skip to content

Commit

Permalink
Revert "LibWasm: Some more performance stuff (#8812)"
Browse files Browse the repository at this point in the history
This reverts commit 35394db.
I pushed the wrong button again, hopefully this will be the last of
such incidents.
  • Loading branch information
alimpfard committed Jul 16, 2021
1 parent 35394db commit 23b48f8
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 49 deletions.
1 change: 0 additions & 1 deletion Tests/LibWasm/test-wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class WebAssemblyModule final : public JS::Object {
explicit WebAssemblyModule(JS::Object& prototype)
: JS::Object(prototype)
{
m_machine.enable_instruction_count_limit();
}

static Wasm::AbstractMachine& machine() { return m_machine; }
Expand Down
10 changes: 0 additions & 10 deletions Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
module.for_each_section_of_type<GlobalSection>([&](auto& global_section) {
for (auto& entry : global_section.entries()) {
Configuration config { m_store };
if (m_should_limit_instruction_count)
config.enable_instruction_count_limit();
config.set_frame(Frame {
auxiliary_instance,
Vector<Value> {},
Expand All @@ -155,8 +153,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
Vector<Reference> references;
for (auto& entry : segment.init) {
Configuration config { m_store };
if (m_should_limit_instruction_count)
config.enable_instruction_count_limit();
config.set_frame(Frame {
main_module_instance,
Vector<Value> {},
Expand Down Expand Up @@ -208,8 +204,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
return IterationDecision::Break;
}
Configuration config { m_store };
if (m_should_limit_instruction_count)
config.enable_instruction_count_limit();
config.set_frame(Frame {
main_module_instance,
Vector<Value> {},
Expand Down Expand Up @@ -268,8 +262,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
segment.value().visit(
[&](DataSection::Data::Active const& data) {
Configuration config { m_store };
if (m_should_limit_instruction_count)
config.enable_instruction_count_limit();
config.set_frame(Frame {
main_module_instance,
Vector<Value> {},
Expand Down Expand Up @@ -447,8 +439,6 @@ Result AbstractMachine::invoke(FunctionAddress address, Vector<Value> arguments)
Result AbstractMachine::invoke(Interpreter& interpreter, FunctionAddress address, Vector<Value> arguments)
{
Configuration configuration { m_store };
if (m_should_limit_instruction_count)
configuration.enable_instruction_count_limit();
return configuration.call(interpreter, address, move(arguments));
}

Expand Down
13 changes: 5 additions & 8 deletions Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,34 +130,34 @@ class Value {
}
}

ALWAYS_INLINE Value(Value const& value)
Value(Value const& value)
: m_value(AnyValueType { value.m_value })
, m_type(value.m_type)
{
}

ALWAYS_INLINE Value(Value&& value)
Value(Value&& value)
: m_value(move(value.m_value))
, m_type(move(value.m_type))
{
}

ALWAYS_INLINE Value& operator=(Value&& value)
Value& operator=(Value&& value)
{
m_value = move(value.m_value);
m_type = move(value.m_type);
return *this;
}

ALWAYS_INLINE Value& operator=(Value const& value)
Value& operator=(Value const& value)
{
m_value = value.m_value;
m_type = value.m_type;
return *this;
}

template<typename T>
ALWAYS_INLINE Optional<T> to()
Optional<T> to()
{
Optional<T> result;
m_value.visit(
Expand Down Expand Up @@ -505,13 +505,10 @@ class AbstractMachine {
auto& store() const { return m_store; }
auto& store() { return m_store; }

void enable_instruction_count_limit() { m_should_limit_instruction_count = true; }

private:
Optional<InstantiationError> allocate_all_initial_phase(Module const&, ModuleInstance&, Vector<ExternValue>&, Vector<Value>& global_values);
Optional<InstantiationError> allocate_all_final_phase(Module const&, ModuleInstance&, Vector<Vector<Reference>>& elements);
Store m_store;
bool m_should_limit_instruction_count { false };
};

class Linker {
Expand Down
27 changes: 13 additions & 14 deletions Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,12 @@ void BytecodeInterpreter::interpret(Configuration& configuration)
auto& instructions = configuration.frame().expression().instructions();
auto max_ip_value = InstructionPointer { instructions.size() };
auto& current_ip_value = configuration.ip();
auto const should_limit_instruction_count = configuration.should_limit_instruction_count();
u64 executed_instructions = 0;

while (current_ip_value < max_ip_value) {
if (should_limit_instruction_count) {
if (executed_instructions++ >= Constants::max_allowed_executed_instructions_per_call) [[unlikely]] {
m_trap = Trap { "Exceeded maximum allowed number of instructions" };
return;
}
if (executed_instructions++ >= Constants::max_allowed_executed_instructions_per_call) [[unlikely]] {
m_trap = Trap { "Exceeded maximum allowed number of instructions" };
return;
}
auto& instruction = instructions[current_ip_value.value()];
auto old_ip = current_ip_value;
Expand Down Expand Up @@ -1126,15 +1123,17 @@ void DebuggerBytecodeInterpreter::interpret(Configuration& configuration, Instru
}
}

BytecodeInterpreter::interpret(configuration, ip, instruction);

if (post_interpret_hook) {
auto result = post_interpret_hook(configuration, ip, instruction, *this);
if (!result) {
m_trap = Trap { "Trapped by user request" };
return;
ScopeGuard guard { [&] {
if (post_interpret_hook) {
auto result = post_interpret_hook(configuration, ip, instruction, *this);
if (!result) {
m_trap = Trap { "Trapped by user request" };
return;
}
}
}
} };

BytecodeInterpreter::interpret(configuration, ip, instruction);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ struct BytecodeInterpreter : public Interpreter {
T read_value(ReadonlyBytes data);

Vector<Value> pop_values(Configuration& configuration, size_t count);
ALWAYS_INLINE bool trap_if_not(bool value, StringView reason)
bool trap_if_not(bool value, StringView reason)
{
if (!value)
m_trap = Trap { reason };
Expand Down
4 changes: 0 additions & 4 deletions Userland/Libraries/LibWasm/AbstractMachine/Configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ class Configuration {
Result call(Interpreter&, FunctionAddress, Vector<Value> arguments);
Result execute(Interpreter&);

void enable_instruction_count_limit() { m_should_limit_instruction_count = true; }
bool should_limit_instruction_count() const { return m_should_limit_instruction_count; }

void dump_stack();

private:
Expand All @@ -72,7 +69,6 @@ class Configuration {
Stack m_stack;
size_t m_depth { 0 };
InstructionPointer m_ip;
bool m_should_limit_instruction_count { false };
};

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ namespace Web::Bindings {
WebAssemblyObject::WebAssemblyObject(JS::GlobalObject& global_object)
: Object(*global_object.object_prototype())
{
s_abstract_machine.enable_instruction_count_limit();
}

void WebAssemblyObject::initialize(JS::GlobalObject& global_object)
Expand Down
18 changes: 8 additions & 10 deletions Userland/Utilities/wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,16 +512,14 @@ int main(int argc, char* argv[])
if (debug)
launch_repl();

if (result.is_trap()) {
warnln("Execution trapped: {}", result.trap().reason);
} else {
if (!result.values().is_empty())
warnln("Returned:");
for (auto& value : result.values()) {
Wasm::Printer printer { stream };
g_stdout.write(" -> "sv.bytes());
g_printer.print(value);
}
if (result.is_trap())
warnln("Execution trapped!");
if (!result.values().is_empty())
warnln("Returned:");
for (auto& value : result.values()) {
Wasm::Printer printer { stream };
g_stdout.write(" -> "sv.bytes());
g_printer.print(value);
}
}
}
Expand Down

0 comments on commit 23b48f8

Please sign in to comment.