History

Created multiple pointer field iterators based on application Fix bug in vm_string class related to appending null at the end of the current string - This was recently introduced because I didn't test the case :( Scheduler had an invalid usage of std::mutex that was found using the debug version of CRT Fix heinous perf issue in GC where it was taking longer than code execution - Switch from std::queue to std::stack -> perf gain of ~20%

arr committed 9 years ago

Commit 30c4c581 parent 6f29a816 files · +102 −39Browse files

Unified · Split

6 files
  1. src/include/vm_memory.h +7 −1
  2. src/vm/garbage_collector.cpp +37 −20
  3. src/vm/scheduler.cpp +3 −3
  4. src/vm/scheduler.h +1 −1
  5. src/vm/string.cpp +21 −10
  6. src/vm/vm_memory.cpp +33 −4
src/include/vm_memory.h+7−1
@@ -50,11 +50,17 @@ namespace disvm · 49 unmodified lines
5050 void dec_ref_count_and_free(vm_alloc_t *alloc);
5151
5252 // Callback for a pointer field supplying the pointer
53− using pointer_field_callback_t = std::function<void(pointer_t)>;
53+ using pointer_field_callback_t = void(*)(pointer_t);
5454
5555 // The callback will be called on all non-null pointer fields.
5656 void enum_pointer_fields(const type_descriptor_t &type_desc, void *data, pointer_field_callback_t callback);
5757
58+ // Callback for a pointer field supplying the pointer and a calling context
59+ using pointer_field_callback_ext_t = void(*)(pointer_t, void *cxt);
60+
61+ // The callback will be called on all non-null pointer fields.
62+ void enum_pointer_fields(const type_descriptor_t &type_desc, void *data, pointer_field_callback_ext_t callback, void *cxt);
63+
5864 // Callback for a pointer field supplying the pointer and byte offset
5965 using pointer_field_offset_callback_t = std::function<void(pointer_t, std::size_t)>;
6066
src/vm/garbage_collector.cpp+37−20
@@ -6,7 +6,7 @@ · 5 unmodified lines
66
77 #include <cinttypes>
88 #include <array>
9−#include <queue>
9+#include <stack>
1010 #include <debug.h>
1111 #include <vm_memory.h>
1212 #include <exceptions.h>
@@ -124,20 +124,37 @@ void default_garbage_collector_t::enum_tracked_allocations(vm_alloc_callback_t c · 111 unmodified lines
124124
125125 namespace
126126 {
127− void mark(const std::vector<std::shared_ptr<const vm_thread_t>> &threads, const gc_colour_t curr_colour)
127+ struct mark_cxt_t final : public std::stack<vm_alloc_t *, std::vector<vm_alloc_t *>>
128128 {
129− std::queue<vm_alloc_t *> allocs;
129+ mark_cxt_t(const type_descriptor_t *string_type, const gc_colour_t curr_colour)
130+ : std::stack<vm_alloc_t *, std::vector<vm_alloc_t *>>{}
131+ , string_type{ string_type }
132+ , curr_colour{ curr_colour }
133+ { }
134+
135+ const type_descriptor_t *string_type;
136+ const gc_colour_t curr_colour;
137+ };
130138
131− // Making this raw since it is a built-in type and has unlimited lifetime.
132− const auto string_type_raw = intrinsic_type_desc::type<vm_string_t>().get();
133− auto add_pointer_to_queue = std::function<void(pointer_t p)>{ [&allocs, string_type_raw, curr_colour](pointer_t p) -> void
134− {
135− auto a = vm_alloc_t::from_allocation(p);
139+ void mark_pointers(pointer_t p, void *cxt)
140+ {
141+ auto a = vm_alloc_t::from_allocation(p);
142+
143+ assert(cxt != nullptr);
144+ auto c = reinterpret_cast<mark_cxt_t *>(cxt);
136145
137− // Don't examine vm strings or allocations with the current colour
138− if (curr_colour != get_gc_colour(a) && a->alloc_type.get() != string_type_raw)
139− allocs.push(a);
140− } };
146+ // Don't examine vm strings or allocations with the current colour
147+ if (c->curr_colour != get_gc_colour(a) && a->alloc_type.get() != c->string_type)
148+ c->push(a);
149+ }
150+
151+ void mark(const std::vector<std::shared_ptr<const vm_thread_t>> &threads, const gc_colour_t curr_colour)
152+ {
153+ mark_cxt_t mark_cxt
154+ {
155+ intrinsic_type_desc::type<vm_string_t>().get(), // Using raw pointer since it is a built-in type and has unlimited lifetime
156+ curr_colour
157+ };
141158
142159 // Get roots from threads
143160 for (auto &t : threads)
@@ -145,7 +162,7 @@ namespace · 1 unmodified line
145162 auto &r = t->get_registers();
146163
147164 // Current MP
148− allocs.push(r.mp_base);
165+ mark_cxt.push(r.mp_base);
149166
150167 // Traverse the stack for roots
151168 for (auto frame = r.stack.peek_frame(); frame != nullptr; frame = frame->prev_frame())
@@ -154,29 +171,29 @@ namespace · 2 unmodified lines
154171 if (frame->prev_module_ref() != nullptr)
155172 {
156173 auto prev_mp = frame->prev_module_ref()->mp_base;
157− allocs.push(prev_mp);
174+ mark_cxt.push(prev_mp);
158175 }
159176
160177 if (frame->frame_type->map_in_bytes > 0)
161− enum_pointer_fields(*frame->frame_type, frame->base(), add_pointer_to_queue);
178+ enum_pointer_fields(*frame->frame_type, frame->base(), mark_pointers, &mark_cxt);
162179 }
163180 }
164181
165182 const auto log_enabled = debug::is_component_tracing_enabled<debug::component_trace_t::garbage_collector>();
166183 if (log_enabled)
167− debug::log_msg(debug::component_trace_t::garbage_collector, debug::log_level_t::debug, "gc: roots found: %" PRIuPTR, allocs.size());
184+ debug::log_msg(debug::component_trace_t::garbage_collector, debug::log_level_t::debug, "gc: roots found: %" PRIuPTR, mark_cxt.size());
168185
169186 // Traverse the graph
170− while (!allocs.empty())
187+ while (!mark_cxt.empty())
171188 {
172− auto curr = allocs.front();
173− allocs.pop();
189+ auto curr = mark_cxt.top();
190+ mark_cxt.pop();
174191
175192 if (log_enabled)
176193 debug::log_msg(debug::component_trace_t::garbage_collector, debug::log_level_t::debug, "gc: mark: alloc: %#" PRIxPTR, curr);
177194
178195 if (curr->alloc_type->map_in_bytes > 0)
179− enum_pointer_fields(*curr->alloc_type, curr->get_allocation(), add_pointer_to_queue);
196+ enum_pointer_fields(*curr->alloc_type, curr->get_allocation(), mark_pointers, &mark_cxt);
180197
181198 set_gc_colour(curr, curr_colour);
182199 }
src/vm/scheduler.cpp+3−3
@@ -254,7 +254,7 @@ void default_scheduler_t::enqueue_blocked_thread(uint32_t thread_id) · 253 unmodified lines
254254 if (debug::is_component_tracing_enabled<debug::component_trace_t::scheduler>())
255255 debug::log_msg(debug::component_trace_t::scheduler, debug::log_level_t::debug, "scheduler: enqueue blocked: %d", thread_id);
256256
257− const bool runnable_thread = enqueue_thread_unsafe(std::move(thread_container), vm_thread_state_t::ready);
257+ const bool runnable_thread = enqueue_thread_unsafe(thread_container.get(), vm_thread_state_t::ready);
258258 if (runnable_thread)
259259 {
260260 lock_all.unlock();
@@ -294,7 +294,7 @@ std::shared_ptr<default_scheduler_t::thread_instance_t> default_scheduler_t::nex · 33 unmodified lines
294294
295295 std::unique_lock<std::mutex> lock_all{ _vm_threads_lock };
296296 --_running_vm_thread_count;
297− const bool runnable_thread = enqueue_thread_unsafe(std::move(prev_thread), current_state);
297+ const bool runnable_thread = enqueue_thread_unsafe(prev_thread.get(), current_state);
298298 if (runnable_thread)
299299 {
300300 lock_all.unlock();
@@ -380,7 +380,7 @@ void default_scheduler_t::perform_gc(bool is_gc_thread, std::unique_lock<std::mu · 79 unmodified lines
380380 }
381381 }
382382
383−bool default_scheduler_t::enqueue_thread_unsafe(std::shared_ptr<thread_instance_t> thread_instance, runtime::vm_thread_state_t current_state)
383+bool default_scheduler_t::enqueue_thread_unsafe(thread_instance_t *thread_instance, runtime::vm_thread_state_t current_state)
384384 {
385385 assert(thread_instance != nullptr);
386386
src/vm/scheduler.h+1−1
@@ -73,7 +73,7 @@ namespace disvm · 72 unmodified lines
7373
7474 // Add the thread to the queue in a non-thread safe manner.
7575 // Returns 'true' if the runnable thread queue has been updated, otherwise 'false'.
76− bool enqueue_thread_unsafe(std::shared_ptr<thread_instance_t> thread, runtime::vm_thread_state_t current_state);
76+ bool enqueue_thread_unsafe(thread_instance_t *thread, runtime::vm_thread_state_t current_state);
7777
7878 private:
7979 vm_t &_vm;
src/vm/string.cpp+21−10
@@ -123,14 +123,14 @@ namespace · 122 unmodified lines
123123 template<>
124124 void copy_characters_to(char *dest, word_t dest_start, const char *source, word_t source_length)
125125 {
126− assert(dest != nullptr && source != nullptr);
126+ assert(dest != nullptr && source != nullptr && dest != source);
127127 std::memcpy((dest + dest_start), source, source_length);
128128 }
129129
130130 template<>
131131 void copy_characters_to(runtime::rune_t *dest, word_t dest_start, const runtime::rune_t *source, word_t source_length)
132132 {
133− assert(dest != nullptr && source != nullptr);
133+ assert(dest != nullptr && source != nullptr && dest != source);
134134 std::memcpy((dest + dest_start), source, (source_length * sizeof(runtime::rune_t)));
135135 }
136136
@@ -286,17 +286,19 @@ vm_string_t::vm_string_t(const vm_string_t &other, word_t begin_index, word_t en · 149 unmodified lines
286286 // Allocate memory based on character count and size
287287 auto src = other._mem.local;
288288 auto dest = _mem.local;
289− const auto length_in_bytes = _length * _character_size;
290289 if (_is_alloc)
291290 {
292291 src = other._mem.alloc;
293− _mem.alloc = alloc_memory<uint8_t>(_length_max);
292+ _mem.alloc = alloc_memory<uint8_t>(_length_max * _character_size);
294293 dest = _mem.alloc;
295294 }
296295
297296 // Copy over string content starting at the correct index (char vs. rune_t)
297+ const auto length_in_bytes = _length * _character_size;
298298 std::memcpy(dest, (src + (begin_index * _character_size)), length_in_bytes);
299− debug::log_msg(debug::component_trace_t::memory, debug::log_level_t::debug, "copy: vm string: %#" PRIxPTR " %d %d %#" PRIxPTR, &other, begin_index, end_index, this);
299+
300+ if (debug::is_component_tracing_enabled<debug::component_trace_t::memory>())
301+ debug::log_msg(debug::component_trace_t::memory, debug::log_level_t::debug, "copy: vm string: %#" PRIxPTR " %d %d %#" PRIxPTR, &other, begin_index, end_index, this);
300302 }
301303
302304 vm_string_t::~vm_string_t()
@@ -309,10 +311,11 @@ vm_string_t::~vm_string_t() · 6 unmodified lines
309311 if (_is_alloc)
310312 free_memory(source);
311313
312− debug::log_msg(debug::component_trace_t::memory, debug::log_level_t::debug, "destroy: vm string");
313−
314314 debug::assign_debug_pointer(&source);
315315 debug::assign_debug_pointer(&_encoded_str);
316+
317+ if (debug::is_component_tracing_enabled<debug::component_trace_t::memory>())
318+ debug::log_msg(debug::component_trace_t::memory, debug::log_level_t::debug, "destroy: vm string");
316319 }
317320
318321 vm_string_t &vm_string_t::append(const vm_string_t &other)
@@ -324,6 +327,7 @@ vm_string_t &vm_string_t::append(const vm_string_t &other) · 5 unmodified lines
324327
325328 // Check if the new string has any length at all.
326329 const auto other_length = other._length;
330+ assert(other_length >= 0);
327331 if (other_length == 0)
328332 return *this;
329333
@@ -342,7 +346,7 @@ vm_string_t &vm_string_t::append(const vm_string_t &other) · 12 unmodified lines
342346
343347 // Define the values for the appended string
344348 const auto new_length = local_length + other_length;
345− const auto new_length_max = (new_length == 0) ? static_cast<word_t>(compute_max_length(_mem.local)) : compute_max_length(new_length);
349+ const auto new_length_max = compute_max_length(new_length);
346350 const auto new_is_rune = local_is_rune || other_is_rune;
347351 uint8_t *new_source = nullptr;
348352
@@ -497,8 +501,15 @@ const char *vm_string_t::str() const · 148 unmodified lines
497501 // If the string is ASCII then return the memory
498502 if (_character_size == sizeof(char))
499503 {
500− _encoded_str = (_is_alloc) ? reinterpret_cast<char *>(_mem.alloc) : reinterpret_cast<char *>(_mem.local);
501− _encoded_str[_length] = '\0';
504+ auto str = (_is_alloc) ? _mem.alloc : _mem.local;
505+ if (_length_max <= _length)
506+ {
507+ auto tmp = alloc_memory<char>(_length + 1); // +1 for null
508+ str = static_cast<uint8_t *>(std::memcpy(tmp, str, _length));
509+ }
510+
511+ _encoded_str = reinterpret_cast<char *>(str);
512+ assert(_encoded_str[_length] == '\0');
502513 return _encoded_str;
503514 }
504515
src/vm/vm_memory.cpp+33−4
@@ -81,13 +81,17 @@ void disvm::runtime::destroy_memory(const type_descriptor_t &type_desc, void *da · 80 unmodified lines
8181
8282 void disvm::runtime::inc_ref_count_in_memory(const type_descriptor_t &type_desc, void *data)
8383 {
84− assert(data != nullptr);
84+ if (type_desc.size_in_bytes == 0)
85+ return;
86+
8587 enum_pointer_fields(type_desc, data, add_ref_pointer_field);
8688 }
8789
8890 void disvm::runtime::dec_ref_count_in_memory(const type_descriptor_t &type_desc, void *data)
8991 {
90− assert(data != nullptr);
92+ if (type_desc.size_in_bytes == 0)
93+ return;
94+
9195 enum_pointer_fields(type_desc, data, dec_ref_and_free_pointer_field);
9296 }
9397
@@ -104,8 +108,6 @@ void disvm::runtime::dec_ref_count_and_free(vm_alloc_t *alloc) · 10 unmodified lines
104108 void disvm::runtime::enum_pointer_fields(const type_descriptor_t &type_desc, void *data, pointer_field_callback_t callback)
105109 {
106110 assert(data != nullptr && callback != nullptr);
107− if (type_desc.size_in_bytes == 0)
108− return;
109111
110112 auto offset_accum = std::size_t{ 0 };
111113 auto memory = reinterpret_cast<word_t *>(data);
@@ -130,6 +132,33 @@ void disvm::runtime::enum_pointer_fields(const type_descriptor_t &type_desc, voi · 18 unmodified lines
130132 }
131133 }
132134
135+void disvm::runtime::enum_pointer_fields(const type_descriptor_t &type_desc, void *data, pointer_field_callback_ext_t callback, void *cxt)
136+{
137+ assert(data != nullptr && callback != nullptr);
138+
139+ auto offset_accum = std::size_t{ 0 };
140+ auto memory = reinterpret_cast<word_t *>(data);
141+ for (auto i = word_t{ 0 }; i < type_desc.map_in_bytes; ++i, memory += 8, offset_accum += (8 * sizeof(pointer_t)))
142+ {
143+ const auto words8 = type_desc.pointer_map[i];
144+ assert(sizeof(words8) == 1);
145+ if (words8 == 0)
146+ continue;
147+
148+ const auto flags = std::bitset<sizeof(words8) * 8>{ words8 };
149+
150+ // Enumerating the flags in reverse order so memory access is sequential.
151+ if (flags[7] && (memory[0] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[0]), cxt);
152+ if (flags[6] && (memory[1] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[1]), cxt);
153+ if (flags[5] && (memory[2] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[2]), cxt);
154+ if (flags[4] && (memory[3] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[3]), cxt);
155+ if (flags[3] && (memory[4] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[4]), cxt);
156+ if (flags[2] && (memory[5] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[5]), cxt);
157+ if (flags[1] && (memory[6] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[6]), cxt);
158+ if (flags[0] && (memory[7] != runtime_constants::nil)) callback(reinterpret_cast<pointer_t>(memory[7]), cxt);
159+ }
160+}
161+
133162 void disvm::runtime::enum_pointer_fields_with_offset(const type_descriptor_t &type_desc, void *data, pointer_field_offset_callback_t callback)
134163 {
135164 assert(data != nullptr && callback != nullptr);