Skip to content

Commit 23b48f8

Browse files
alimpfardalimpfard
committed
Revert "LibWasm: Some more performance stuff (#8812)"
This reverts commit 35394db. I pushed the wrong button again, hopefully this will be the last of such incidents.
1 parent 35394db commit 23b48f8

File tree

8 files changed

+27
-49
lines changed

8 files changed

+27
-49
lines changed

‎Tests/LibWasm/test-wasm.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ class WebAssemblyModule final : public JS::Object {
3434
explicit WebAssemblyModule(JS::Object& prototype)
3535
: JS::Object(prototype)
3636
{
37-
m_machine.enable_instruction_count_limit();
3837
}
3938

4039
static Wasm::AbstractMachine& machine() { return m_machine; }

‎Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
128128
module.for_each_section_of_type<GlobalSection>([&](auto& global_section) {
129129
for (auto& entry : global_section.entries()) {
130130
Configuration config { m_store };
131-
if (m_should_limit_instruction_count)
132-
config.enable_instruction_count_limit();
133131
config.set_frame(Frame {
134132
auxiliary_instance,
135133
Vector<Value> {},
@@ -155,8 +153,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
155153
Vector<Reference> references;
156154
for (auto& entry : segment.init) {
157155
Configuration config { m_store };
158-
if (m_should_limit_instruction_count)
159-
config.enable_instruction_count_limit();
160156
config.set_frame(Frame {
161157
main_module_instance,
162158
Vector<Value> {},
@@ -208,8 +204,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
208204
return IterationDecision::Break;
209205
}
210206
Configuration config { m_store };
211-
if (m_should_limit_instruction_count)
212-
config.enable_instruction_count_limit();
213207
config.set_frame(Frame {
214208
main_module_instance,
215209
Vector<Value> {},
@@ -268,8 +262,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector<Ex
268262
segment.value().visit(
269263
[&](DataSection::Data::Active const& data) {
270264
Configuration config { m_store };
271-
if (m_should_limit_instruction_count)
272-
config.enable_instruction_count_limit();
273265
config.set_frame(Frame {
274266
main_module_instance,
275267
Vector<Value> {},
@@ -447,8 +439,6 @@ Result AbstractMachine::invoke(FunctionAddress address, Vector<Value> arguments)
447439
Result AbstractMachine::invoke(Interpreter& interpreter, FunctionAddress address, Vector<Value> arguments)
448440
{
449441
Configuration configuration { m_store };
450-
if (m_should_limit_instruction_count)
451-
configuration.enable_instruction_count_limit();
452442
return configuration.call(interpreter, address, move(arguments));
453443
}
454444

‎Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,34 +130,34 @@ class Value {
130130
}
131131
}
132132

133-
ALWAYS_INLINE Value(Value const& value)
133+
Value(Value const& value)
134134
: m_value(AnyValueType { value.m_value })
135135
, m_type(value.m_type)
136136
{
137137
}
138138

139-
ALWAYS_INLINE Value(Value&& value)
139+
Value(Value&& value)
140140
: m_value(move(value.m_value))
141141
, m_type(move(value.m_type))
142142
{
143143
}
144144

145-
ALWAYS_INLINE Value& operator=(Value&& value)
145+
Value& operator=(Value&& value)
146146
{
147147
m_value = move(value.m_value);
148148
m_type = move(value.m_type);
149149
return *this;
150150
}
151151

152-
ALWAYS_INLINE Value& operator=(Value const& value)
152+
Value& operator=(Value const& value)
153153
{
154154
m_value = value.m_value;
155155
m_type = value.m_type;
156156
return *this;
157157
}
158158

159159
template<typename T>
160-
ALWAYS_INLINE Optional<T> to()
160+
Optional<T> to()
161161
{
162162
Optional<T> result;
163163
m_value.visit(
@@ -505,13 +505,10 @@ class AbstractMachine {
505505
auto& store() const { return m_store; }
506506
auto& store() { return m_store; }
507507

508-
void enable_instruction_count_limit() { m_should_limit_instruction_count = true; }
509-
510508
private:
511509
Optional<InstantiationError> allocate_all_initial_phase(Module const&, ModuleInstance&, Vector<ExternValue>&, Vector<Value>& global_values);
512510
Optional<InstantiationError> allocate_all_final_phase(Module const&, ModuleInstance&, Vector<Vector<Reference>>& elements);
513511
Store m_store;
514-
bool m_should_limit_instruction_count { false };
515512
};
516513

517514
class Linker {

‎Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,12 @@ void BytecodeInterpreter::interpret(Configuration& configuration)
3737
auto& instructions = configuration.frame().expression().instructions();
3838
auto max_ip_value = InstructionPointer { instructions.size() };
3939
auto& current_ip_value = configuration.ip();
40-
auto const should_limit_instruction_count = configuration.should_limit_instruction_count();
4140
u64 executed_instructions = 0;
4241

4342
while (current_ip_value < max_ip_value) {
44-
if (should_limit_instruction_count) {
45-
if (executed_instructions++ >= Constants::max_allowed_executed_instructions_per_call) [[unlikely]] {
46-
m_trap = Trap { "Exceeded maximum allowed number of instructions" };
47-
return;
48-
}
43+
if (executed_instructions++ >= Constants::max_allowed_executed_instructions_per_call) [[unlikely]] {
44+
m_trap = Trap { "Exceeded maximum allowed number of instructions" };
45+
return;
4946
}
5047
auto& instruction = instructions[current_ip_value.value()];
5148
auto old_ip = current_ip_value;
@@ -1126,15 +1123,17 @@ void DebuggerBytecodeInterpreter::interpret(Configuration& configuration, Instru
11261123
}
11271124
}
11281125

1129-
BytecodeInterpreter::interpret(configuration, ip, instruction);
1130-
1131-
if (post_interpret_hook) {
1132-
auto result = post_interpret_hook(configuration, ip, instruction, *this);
1133-
if (!result) {
1134-
m_trap = Trap { "Trapped by user request" };
1135-
return;
1126+
ScopeGuard guard { [&] {
1127+
if (post_interpret_hook) {
1128+
auto result = post_interpret_hook(configuration, ip, instruction, *this);
1129+
if (!result) {
1130+
m_trap = Trap { "Trapped by user request" };
1131+
return;
1132+
}
11361133
}
1137-
}
1134+
} };
1135+
1136+
BytecodeInterpreter::interpret(configuration, ip, instruction);
11381137
}
11391138

11401139
}

‎Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ struct BytecodeInterpreter : public Interpreter {
5050
T read_value(ReadonlyBytes data);
5151

5252
Vector<Value> pop_values(Configuration& configuration, size_t count);
53-
ALWAYS_INLINE bool trap_if_not(bool value, StringView reason)
53+
bool trap_if_not(bool value, StringView reason)
5454
{
5555
if (!value)
5656
m_trap = Trap { reason };

‎Userland/Libraries/LibWasm/AbstractMachine/Configuration.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ class Configuration {
6161
Result call(Interpreter&, FunctionAddress, Vector<Value> arguments);
6262
Result execute(Interpreter&);
6363

64-
void enable_instruction_count_limit() { m_should_limit_instruction_count = true; }
65-
bool should_limit_instruction_count() const { return m_should_limit_instruction_count; }
66-
6764
void dump_stack();
6865

6966
private:
@@ -72,7 +69,6 @@ class Configuration {
7269
Stack m_stack;
7370
size_t m_depth { 0 };
7471
InstructionPointer m_ip;
75-
bool m_should_limit_instruction_count { false };
7672
};
7773

7874
}

‎Userland/Libraries/LibWeb/WebAssembly/WebAssemblyObject.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ namespace Web::Bindings {
2525
WebAssemblyObject::WebAssemblyObject(JS::GlobalObject& global_object)
2626
: Object(*global_object.object_prototype())
2727
{
28-
s_abstract_machine.enable_instruction_count_limit();
2928
}
3029

3130
void WebAssemblyObject::initialize(JS::GlobalObject& global_object)

‎Userland/Utilities/wasm.cpp‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -512,16 +512,14 @@ int main(int argc, char* argv[])
512512
if (debug)
513513
launch_repl();
514514

515-
if (result.is_trap()) {
516-
warnln("Execution trapped: {}", result.trap().reason);
517-
} else {
518-
if (!result.values().is_empty())
519-
warnln("Returned:");
520-
for (auto& value : result.values()) {
521-
Wasm::Printer printer { stream };
522-
g_stdout.write(" -> "sv.bytes());
523-
g_printer.print(value);
524-
}
515+
if (result.is_trap())
516+
warnln("Execution trapped!");
517+
if (!result.values().is_empty())
518+
warnln("Returned:");
519+
for (auto& value : result.values()) {
520+
Wasm::Printer printer { stream };
521+
g_stdout.write(" -> "sv.bytes());
522+
g_printer.print(value);
525523
}
526524
}
527525
}

0 commit comments

Comments
 (0)