From 8da6c01d8f7b24d63431ceef1db62c2e2cdcc2bd Mon Sep 17 00:00:00 2001 From: davidot Date: Fri, 4 Feb 2022 16:12:39 +0100 Subject: [PATCH] LibJS: Remove the JS_TRACK_ZOMBIE_CELLS option This feature had bitrotted somewhat and would trigger errors because PrimitiveStrings were "destroyed" but because of this mode they were not removed from the string cache. Even fixing that case running test-js with the options still failed in more places. --- Userland/Libraries/LibJS/Heap/Cell.h | 9 --------- Userland/Libraries/LibJS/Heap/Heap.cpp | 19 +------------------ Userland/Libraries/LibJS/Heap/Heap.h | 11 ----------- .../LibJS/Runtime/FinalizationRegistry.h | 7 ------- Userland/Libraries/LibJS/Runtime/Shape.cpp | 7 ------- Userland/Libraries/LibJS/Runtime/Shape.h | 4 ---- Userland/Libraries/LibJS/Runtime/WeakMap.h | 7 ------- Userland/Libraries/LibJS/Runtime/WeakRef.h | 7 ------- Userland/Libraries/LibJS/Runtime/WeakSet.h | 7 ------- .../Libraries/LibTest/JavaScriptTestRunner.h | 7 ------- .../LibTest/JavaScriptTestRunnerMain.cpp | 6 ------ Userland/Libraries/LibWeb/Bindings/Wrapper.h | 7 ------- Userland/Utilities/js.cpp | 10 ---------- 13 files changed, 1 insertion(+), 107 deletions(-) diff --git a/Userland/Libraries/LibJS/Heap/Cell.h b/Userland/Libraries/LibJS/Heap/Cell.h index 4e6701e6cae..c7f4a7c645e 100644 --- a/Userland/Libraries/LibJS/Heap/Cell.h +++ b/Userland/Libraries/LibJS/Heap/Cell.h @@ -24,18 +24,9 @@ public: bool is_marked() const { return m_mark; } void set_marked(bool b) { m_mark = b; } -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() - { - } -#endif - enum class State { Live, Dead, -#ifdef JS_TRACK_ZOMBIE_CELLS - Zombie, -#endif }; State state() const { return m_state; } diff --git a/Userland/Libraries/LibJS/Heap/Heap.cpp b/Userland/Libraries/LibJS/Heap/Heap.cpp index cf792ba04a5..3fd89637c5e 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.cpp +++ b/Userland/Libraries/LibJS/Heap/Heap.cpp @@ -190,14 +190,6 @@ public: return; dbgln_if(HEAP_DEBUG, " ! {}", &cell); -#ifdef JS_TRACK_ZOMBIE_CELLS - if (cell.state() == Cell::State::Zombie) { - dbgln("BUG! Marking a zombie cell, {} @ {:p}", cell.class_name(), &cell); - cell.vm().dump_backtrace(); - VERIFY_NOT_REACHED(); - } -#endif - cell.set_marked(true); cell.visit_edges(*this); } @@ -234,16 +226,7 @@ void Heap::sweep_dead_cells(bool print_report, const Core::ElapsedTimer& measure block.template for_each_cell_in_state([&](Cell* cell) { if (!cell->is_marked()) { dbgln_if(HEAP_DEBUG, " ~ {}", cell); -#ifdef JS_TRACK_ZOMBIE_CELLS - if (m_zombify_dead_cells) { - cell->set_state(Cell::State::Zombie); - cell->did_become_zombie(); - } else { -#endif - block.deallocate(cell); -#ifdef JS_TRACK_ZOMBIE_CELLS - } -#endif + block.deallocate(cell); ++collected_cells; collected_cell_bytes += block.cell_size(); } else { diff --git a/Userland/Libraries/LibJS/Heap/Heap.h b/Userland/Libraries/LibJS/Heap/Heap.h index 6b2deb7335d..ddb189581ec 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.h +++ b/Userland/Libraries/LibJS/Heap/Heap.h @@ -63,13 +63,6 @@ public: bool should_collect_on_every_allocation() const { return m_should_collect_on_every_allocation; } void set_should_collect_on_every_allocation(bool b) { m_should_collect_on_every_allocation = b; } -#ifdef JS_TRACK_ZOMBIE_CELLS - void set_zombify_dead_cells(bool b) - { - m_zombify_dead_cells = b; - } -#endif - void did_create_handle(Badge, HandleImpl&); void did_destroy_handle(Badge, HandleImpl&); @@ -132,10 +125,6 @@ private: bool m_should_gc_when_deferral_ends { false }; bool m_collecting_garbage { false }; - -#ifdef JS_TRACK_ZOMBIE_CELLS - bool m_zombify_dead_cells { false }; -#endif }; } diff --git a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h index 9fca911f34d..53f008f51e8 100644 --- a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h +++ b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h @@ -35,13 +35,6 @@ public: private: virtual void visit_edges(Visitor& visitor) override; -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() override - { - deregister(); - } -#endif - FunctionObject* m_cleanup_callback { nullptr }; struct FinalizationRecord { diff --git a/Userland/Libraries/LibJS/Runtime/Shape.cpp b/Userland/Libraries/LibJS/Runtime/Shape.cpp index 0b0f2f96b7b..b7e51fc9e3c 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.cpp +++ b/Userland/Libraries/LibJS/Runtime/Shape.cpp @@ -247,11 +247,4 @@ FLATTEN void Shape::add_property_without_transition(PropertyKey const& property_ add_property_without_transition(property_name.to_string_or_symbol(), attributes); } -#ifdef JS_TRACK_ZOMBIE_CELLS -void Shape::did_become_zombie() -{ - revoke_weak_ptrs(); -} -#endif - } diff --git a/Userland/Libraries/LibJS/Runtime/Shape.h b/Userland/Libraries/LibJS/Runtime/Shape.h index b2712880c56..4691e330708 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.h +++ b/Userland/Libraries/LibJS/Runtime/Shape.h @@ -89,10 +89,6 @@ private: virtual const char* class_name() const override { return "Shape"; } virtual void visit_edges(Visitor&) override; -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() override; -#endif - Shape* get_or_prune_cached_forward_transition(TransitionKey const&); Shape* get_or_prune_cached_prototype_transition(Object* prototype); diff --git a/Userland/Libraries/LibJS/Runtime/WeakMap.h b/Userland/Libraries/LibJS/Runtime/WeakMap.h index 0d966381576..27872c6dfd3 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakMap.h +++ b/Userland/Libraries/LibJS/Runtime/WeakMap.h @@ -30,13 +30,6 @@ public: virtual void remove_dead_cells(Badge) override; private: -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() override - { - deregister(); - } -#endif - void visit_edges(Visitor&) override; HashMap m_values; // This stores Cell pointers instead of Object pointers to aide with sweeping diff --git a/Userland/Libraries/LibJS/Runtime/WeakRef.h b/Userland/Libraries/LibJS/Runtime/WeakRef.h index 8bc2b9ad307..35be9d8e325 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRef.h +++ b/Userland/Libraries/LibJS/Runtime/WeakRef.h @@ -32,13 +32,6 @@ public: private: virtual void visit_edges(Visitor&) override; -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() override - { - deregister(); - } -#endif - Object* m_value { nullptr }; u32 m_last_execution_generation { 0 }; }; diff --git a/Userland/Libraries/LibJS/Runtime/WeakSet.h b/Userland/Libraries/LibJS/Runtime/WeakSet.h index 6d777cb97cb..eaa9e6cda0d 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakSet.h +++ b/Userland/Libraries/LibJS/Runtime/WeakSet.h @@ -30,13 +30,6 @@ public: virtual void remove_dead_cells(Badge) override; private: -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() override - { - deregister(); - } -#endif - HashTable m_values; // This stores Cell pointers instead of Object pointers to aide with sweeping }; diff --git a/Userland/Libraries/LibTest/JavaScriptTestRunner.h b/Userland/Libraries/LibTest/JavaScriptTestRunner.h index 6dff4cb26d3..f126409ca91 100644 --- a/Userland/Libraries/LibTest/JavaScriptTestRunner.h +++ b/Userland/Libraries/LibTest/JavaScriptTestRunner.h @@ -116,9 +116,6 @@ static consteval size_t __testjs_last() static constexpr auto TOP_LEVEL_TEST_NAME = "__$$TOP_LEVEL$$__"; extern RefPtr g_vm; extern bool g_collect_on_every_allocation; -#ifdef JS_TRACK_ZOMBIE_CELLS -extern bool g_zombify_dead_cells; -#endif extern bool g_run_bytecode; extern String g_currently_running_test; struct FunctionWithLength { @@ -291,10 +288,6 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path) interpreter->heap().set_should_collect_on_every_allocation(g_collect_on_every_allocation); -#ifdef JS_TRACK_ZOMBIE_CELLS - interpreter->heap().set_zombify_dead_cells(g_zombify_dead_cells); -#endif - if (g_run_file) { auto result = g_run_file(test_path, *interpreter); if (result.is_error() && result.error() == RunFileHookResult::SkipFile) { diff --git a/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp b/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp index a4aa7f7e449..0910486bb26 100644 --- a/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp +++ b/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp @@ -19,9 +19,6 @@ namespace JS { RefPtr<::JS::VM> g_vm; bool g_collect_on_every_allocation = false; -#ifdef JS_TRACK_ZOMBIE_CELLS -bool g_zombify_dead_cells = false; -#endif bool g_run_bytecode = false; String g_currently_running_test; HashMap s_exposed_global_functions; @@ -112,9 +109,6 @@ int main(int argc, char** argv) }); args_parser.add_option(print_json, "Show results as JSON", "json", 'j'); args_parser.add_option(g_collect_on_every_allocation, "Collect garbage after every allocation", "collect-often", 'g'); -#ifdef JS_TRACK_ZOMBIE_CELLS - args_parser.add_option(g_zombify_dead_cells, "Zombify dead cells (to catch missing GC marks)", "zombify-dead-cells", 'z'); -#endif args_parser.add_option(g_run_bytecode, "Use the bytecode interpreter", "run-bytecode", 'b'); args_parser.add_option(JS::Bytecode::g_dump_bytecode, "Dump the bytecode", "dump-bytecode", 'd'); args_parser.add_option(test_glob, "Only run tests matching the given glob", "filter", 'f', "glob"); diff --git a/Userland/Libraries/LibWeb/Bindings/Wrapper.h b/Userland/Libraries/LibWeb/Bindings/Wrapper.h index 11da182ea6d..2ac7c4851cc 100644 --- a/Userland/Libraries/LibWeb/Bindings/Wrapper.h +++ b/Userland/Libraries/LibWeb/Bindings/Wrapper.h @@ -24,13 +24,6 @@ protected: : Object(prototype) { } - -#ifdef JS_TRACK_ZOMBIE_CELLS - virtual void did_become_zombie() override - { - revoke_weak_ptrs(); - } -#endif }; } diff --git a/Userland/Utilities/js.cpp b/Userland/Utilities/js.cpp index f2d99ac71b9..3b98ceeb12f 100644 --- a/Userland/Utilities/js.cpp +++ b/Userland/Utilities/js.cpp @@ -1370,10 +1370,6 @@ ErrorOr serenity_main(Main::Arguments arguments) args_parser.add_option(s_strip_ansi, "Disable ANSI colors", "disable-ansi-colors", 'c'); args_parser.add_option(s_disable_source_location_hints, "Disable source location hints", "disable-source-location-hints", 'h'); args_parser.add_option(gc_on_every_allocation, "GC on every allocation", "gc-on-every-allocation", 'g'); -#ifdef JS_TRACK_ZOMBIE_CELLS - bool zombify_dead_cells = false; - args_parser.add_option(zombify_dead_cells, "Zombify dead cells (to catch missing GC marks)", "zombify-dead-cells", 'z'); -#endif args_parser.add_option(disable_syntax_highlight, "Disable live syntax highlighting", "no-syntax-highlight", 's'); args_parser.add_positional_argument(script_paths, "Path to script files", "scripts", Core::ArgsParser::Required::No); args_parser.parse(arguments); @@ -1416,9 +1412,6 @@ ErrorOr serenity_main(Main::Arguments arguments) ReplConsoleClient console_client(interpreter->global_object().console()); interpreter->global_object().console().set_client(console_client); interpreter->heap().set_should_collect_on_every_allocation(gc_on_every_allocation); -#ifdef JS_TRACK_ZOMBIE_CELLS - interpreter->heap().set_zombify_dead_cells(zombify_dead_cells); -#endif auto& global_environment = interpreter->realm().global_environment(); @@ -1630,9 +1623,6 @@ ErrorOr serenity_main(Main::Arguments arguments) ReplConsoleClient console_client(interpreter->global_object().console()); interpreter->global_object().console().set_client(console_client); interpreter->heap().set_should_collect_on_every_allocation(gc_on_every_allocation); -#ifdef JS_TRACK_ZOMBIE_CELLS - interpreter->heap().set_zombify_dead_cells(zombify_dead_cells); -#endif signal(SIGINT, [](int) { sigint_handler();