From 6840297c083eda5a0751848056ce5aabdc3e8034 Mon Sep 17 00:00:00 2001 From: valtermiari Date: Wed, 15 Mar 2023 18:03:08 +0100 Subject: [PATCH] Fixed some bugs and modifications to mark --- src/GC/Makefile | 4 ++-- src/GC/include/chunk.hpp | 14 +++++++++++- src/GC/include/heap.hpp | 1 + src/GC/lib/event.cpp | 3 ++- src/GC/lib/heap.cpp | 49 +++++++++++++++++++++++++++++++++++----- src/GC/lib/profiler.cpp | 10 ++++---- src/GC/tests/game.cpp | 33 +++++++++++++++++++++++---- src/GC/tests/h_test.cpp | 13 +++++++---- src/GC/tests/player.hpp | 9 ++++++++ src/GC/todo.md | 2 ++ 10 files changed, 114 insertions(+), 24 deletions(-) diff --git a/src/GC/Makefile b/src/GC/Makefile index 57800c4..8ce942b 100644 --- a/src/GC/Makefile +++ b/src/GC/Makefile @@ -20,7 +20,7 @@ heap: h_test: rm -f tests/h_test.out - $(CC) $(WFLAGS) $(STDFLAGS) $(LIB_INCL) tests/h_test.cpp lib/heap.cpp -o tests/h_test.out + $(CC) $(WFLAGS) $(STDFLAGS) $(LIB_INCL) tests/h_test.cpp lib/heap.cpp lib/profiler.cpp lib/event.cpp -o tests/h_test.out h_test_vg: make h_test @@ -40,7 +40,7 @@ linker_vg: game: rm -f tests/game.out - $(CC) $(WFLAGS) $(STDFLAGS) $(LIB_INCL) tests/game.cpp lib/heap.cpp -o tests/game.out + $(CC) $(WFLAGS) $(STDFLAGS) $(LIB_INCL) tests/game.cpp lib/heap.cpp lib/profiler.cpp lib/event.cpp -o tests/game.out extern_lib: rm -f lib/heap.o lib/libheap.so tests/extern_lib.out diff --git a/src/GC/include/chunk.hpp b/src/GC/include/chunk.hpp index 49858a2..fe44220 100644 --- a/src/GC/include/chunk.hpp +++ b/src/GC/include/chunk.hpp @@ -10,6 +10,18 @@ namespace GC bool marked; uintptr_t *start; size_t size; - }; + // Default constructor + Chunk() + {} + + // -- Temporary -- + // A copy constructor, keep track of how many times the vectors that hold chunks + // are copied. + // Shouldn't be all that relevant if we use vectors with Chunk-pointers. + Chunk(const Chunk& c) : marked(c.marked), start(c.start), size(c.size) + { + std::cout << "Chunk was copied" << std::endl; + } + }; } \ No newline at end of file diff --git a/src/GC/include/heap.hpp b/src/GC/include/heap.hpp index cad2773..9f5ba41 100644 --- a/src/GC/include/heap.hpp +++ b/src/GC/include/heap.hpp @@ -78,6 +78,7 @@ namespace GC void mark(uintptr_t *start, const uintptr_t *end, std::vector &worklist); void print_line(Chunk *chunk); void print_worklist(std::vector &list); + void mark_step(uintptr_t memory_location, std::vector &worklist); public: /** diff --git a/src/GC/lib/event.cpp b/src/GC/lib/event.cpp index 8fd633c..9387e49 100644 --- a/src/GC/lib/event.cpp +++ b/src/GC/lib/event.cpp @@ -24,7 +24,8 @@ namespace GC return m_chunk; } - inline const char *GCEvent::TypeToString() + // Try to remove inline + const char *GCEvent::TypeToString() { switch (m_type) { diff --git a/src/GC/lib/heap.cpp b/src/GC/lib/heap.cpp index a7d37d0..617eeab 100644 --- a/src/GC/lib/heap.cpp +++ b/src/GC/lib/heap.cpp @@ -175,13 +175,12 @@ namespace GC */ void Heap::mark(uintptr_t *start, const uintptr_t *end, vector &worklist) { + cout << "--- Marked was called ---" << endl; if (getProfilerMode()) Profiler::record(MarkStart); - int counter = 0; // To find adresses thats in the worklist for (; start < end; start++) { - counter++; auto it = worklist.begin(); auto stop = worklist.end(); // for (auto it = worklist.begin(); it != worklist.end();) { @@ -195,8 +194,7 @@ namespace GC cout << "Start points to:\t" << hex << *start << endl; cout << "Chunk start:\t\t" << hex << c_start << endl; - cout << "Chunk end:\t\t" << hex << c_end << "\n" - << endl; + cout << "Chunk end:\t\t" << hex << c_end << "\n" << endl; // Check if the stack pointer aligns with the chunk if (c_start <= *start && *start < c_end) @@ -207,10 +205,16 @@ namespace GC if (getProfilerMode()) Profiler::record(ChunkMarked, chunk); chunk->marked = true; + cout << "Marked this chunk ^\n" << endl; // Remove the marked chunk from the worklist it = worklist.erase(it); // Recursively call mark, to see if the reachable chunk further points to another chunk - mark((uintptr_t *)c_start, (uintptr_t *)c_end, worklist); + // Previously -> + // mark((uintptr_t *)c_start, (uintptr_t *)c_end, worklist); + // Testing this, it should move to the next adress and end shouldn't be of concern + // since it is a static size of pointers + // New -> mark((uintptr_t *) c_end, (uintptr_t *) (c_end + sizeof(uintptr_t)), worklist); + mark_step(c_end, worklist); } else { @@ -223,7 +227,40 @@ namespace GC } } } - cout << "Counter: " << counter << endl; + } + + // Testing a strategy where if a pointer on the stack is pointing to a chunk, nested chunks, + // that are not located on the stack frame, will possibly be adjecent to the found chunk, + // allowing for a different, more efficient strategy, that doesn't have to scan the stack frame + void Heap::mark_step(uintptr_t memory_location, vector &worklist) { + + auto it = worklist.begin(); + auto end = worklist.end(); + + while (it != end) { + + Chunk *chunk = *it; + auto c_start = reinterpret_cast(chunk->start); + auto c_size = reinterpret_cast(chunk->size); + auto c_end = reinterpret_cast(c_start + c_size); + + if (c_start <= memory_location && memory_location < c_end) { + + if (!chunk->marked) { + // Mark the chunk and erase it from the worklist + chunk->marked = true; + it = worklist.erase(it); + // Update the memory location we want to look at + memory_location = c_end; + } + else { + it++; + } + } + else { + it++; + } + } } /** diff --git a/src/GC/lib/profiler.cpp b/src/GC/lib/profiler.cpp index 2b7f5b8..c94f917 100644 --- a/src/GC/lib/profiler.cpp +++ b/src/GC/lib/profiler.cpp @@ -32,7 +32,7 @@ namespace GC auto start = profiler->m_events.begin(); auto end = profiler->m_events.end(); - std::ofstream fstr = profiler->createFileStream(); + std::ofstream fstr = profiler->createFileStream(); // this is now found char buffer[22]; std::tm *btm; std::time_t tt; @@ -42,15 +42,15 @@ namespace GC { auto event = *start++; - tt = event->getTimeStamp(); + tt = event->getTimeStamp(); // this is found btm = std::localtime(&tt); std::strftime(buffer, 22, "%a %T", btm); fstr << "--------------------------------\n" << buffer - << "\nEvent:\t" << event->TypeToString(); + << "\nEvent:\t" << event->TypeToString(); // this is not found - chunk = event->getChunk(); + chunk = event->getChunk(); // this is found if (chunk) { fstr << "\nChunk:\t" << chunk->start @@ -61,7 +61,7 @@ namespace GC } } - std::ofstream createFileStream() + std::ofstream Profiler::createFileStream() { std::time_t tt = std::time(NULL); std::tm *ptm = std::localtime(&tt); diff --git a/src/GC/tests/game.cpp b/src/GC/tests/game.cpp index 5ff8787..b4c1cc5 100644 --- a/src/GC/tests/game.cpp +++ b/src/GC/tests/game.cpp @@ -6,6 +6,20 @@ #define X_LENGTH 1000; #define Y_LENGTH 500; +/* +* Description: +* This class is designed to test the Garbage Collector with a mock game, +* that consists of several live objects in the form of players, that in +* turn consists partially of Point objects. +* +* Goal: +* to find out if all the objects are allocated successfully +* and to see if they are reachable from the stack, i.e. they can get marked. +* +* Result: +* all objects gets allocated, but only Game object gets marked. +*/ + class Game { private: @@ -22,12 +36,17 @@ public: } Player* create_player(string s, Point pos, Point size, Point dir) { - Player *p = static_cast(GC::Heap::alloc(sizeof(Player))); - p->Player(s, pos, size, dir); // This will probably both be allocated on "our" heap as well as the heap for this program - return p; + Player *p = static_cast(GC::Heap::alloc(sizeof(Player))); + /* + Cannot allocate by new, since it the allocates outside of "out" heap. That also lead so us having to + define an alternative constructor, that's actually a method. Since our "alloc" does not call the constructor + of the object + */ + p->init(s, pos, size, dir); + return p; } - std::vector create_players(int nr) { + void create_players(int nr) { for (int i = 0; i < nr; i++) { Player *p = create_player(std::to_string(i), Point(i, i), Point(2, 2), Point(0, 0)); add_player(*p); @@ -43,7 +62,11 @@ int main() { gc->check_init(); Game *game = static_cast(gc->alloc(sizeof(Game))); - game->create_players(100); + game->create_players(2); + + std::cout << "Player size: " << sizeof(Player) << std::endl; + std::cout << "Game size: " << sizeof(Game) << std::endl; + std::cout << "Point size: " << sizeof(Point) << std::endl; gc->collect(GC::MARK); gc->print_contents(); diff --git a/src/GC/tests/h_test.cpp b/src/GC/tests/h_test.cpp index 8292734..d4c5161 100644 --- a/src/GC/tests/h_test.cpp +++ b/src/GC/tests/h_test.cpp @@ -12,10 +12,12 @@ Node *create_chain(int depth) { last_node->id = depth; last_node->child = nullptr; nodes.push_back(last_node); - for (int i = 0; i < depth; i++) { + for (size_t i = 0; i < depth; i++) { Node *node = static_cast(GC::Heap::alloc(sizeof(Node))); node->id = depth-i; - node->child = nodes[i-1]; + node->child = nodes[i]; + //node->child = nodes.at(i-1); + std::cout << "Child of node: " << node << " is: " << node->child << std::endl; nodes.push_back(node); } for (size_t i = 0; i < nodes.size(); i++) { @@ -84,11 +86,14 @@ int main() { Node *root = static_cast(gc->alloc(sizeof(Node))); root = test_chain(3, false); + Node *root_child = root->child; std::cout << "Adress of root:\t" << &root << std::endl; std::cout << "Root points to:\t" << root << std::endl; - std::cout << "Root child:\t" << root->child << std::endl; + std::cout << "Root child:\t" << root_child << std::endl; + std::cout << "Root child, child:\t" << root_child->child << std::endl; - gc->collect(MARK); + + gc->collect(GC::MARK); gc->print_contents(); return 0; } \ No newline at end of file diff --git a/src/GC/tests/player.hpp b/src/GC/tests/player.hpp index d3b8fec..6a554e8 100644 --- a/src/GC/tests/player.hpp +++ b/src/GC/tests/player.hpp @@ -35,4 +35,13 @@ public: direction.y = dy; } + // This is probably neccessary to initialize an object with our GC + // Since allocation and construction cannot be done at the same time + void init(string n, Point pos, Point s, Point dir) { + name = n; + position = pos; + size = s; + direction = dir; + } + }; diff --git a/src/GC/todo.md b/src/GC/todo.md index 17f159f..d8edd62 100644 --- a/src/GC/todo.md +++ b/src/GC/todo.md @@ -10,6 +10,8 @@ Goal for next week (24/2): - Kolla linking med Valter/Victor - Fixa en a-fil/static lib till Samuel - Kolla vektor vs list complexity +- Se om det är bättre att lagra Chunk och inte Chunk* i data strukturerna, +då är alla efter varandra i minnet. ## Tests TODO - Write complex datastructures for tests with larger programs