Skip to content

Commit

Permalink
[my_runtime] Ensure that the from space is zero-initialized.
Browse files Browse the repository at this point in the history
Collect() maintains the invariant we set up in Init().
  • Loading branch information
Andy Chu committed Nov 26, 2020
1 parent 2800584 commit 43f4e25
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 14 deletions.
2 changes: 2 additions & 0 deletions mycpp/gc_heap.cc
Expand Up @@ -139,6 +139,8 @@ void Heap::Collect(bool must_grow) {
// Swap spaces for next collection
char* tmp = from_space_;
from_space_ = to_space_;
// Maintain invariant of the space we will allocate from.
memset(from_space_, 0, space_size_);
to_space_ = tmp;

#if GC_DEBUG
Expand Down
15 changes: 8 additions & 7 deletions mycpp/gc_heap.h
Expand Up @@ -150,7 +150,9 @@ class Heap {

roots_top_ = 0;

// slab scanning relies on 0 bytes (nullptr)
// Slab scanning relies on 0 bytes (nullptr). e.g. for a List<Token*>*.
// Note: I noticed that memset() of say 400 MiB is pretty expensive. Does
// it makes sense to zero the slabs instead?
memset(from_space_, 0, num_bytes);
memset(to_space_, 0, num_bytes);

Expand Down Expand Up @@ -527,8 +529,6 @@ class Str : public gc_heap::Obj {
DISALLOW_COPY_AND_ASSIGN(Str)
};

constexpr int kStrHeaderSize = offsetof(Str, data_);

template <int N>
class GlobalStr {
// A template type with the same layout as Str with length N-1 (which needs a
Expand All @@ -542,6 +542,10 @@ class GlobalStr {
DISALLOW_COPY_AND_ASSIGN(GlobalStr)
};

// This is the same as offsetof(Str, data_), but doesn't give a warning,
// because of the inheritance?
constexpr int kStrHeaderSize = offsetof(GlobalStr<1>, data_);

// This macro is a workaround for the fact that it's impossible to have a
// a constexpr initializer for char[N]. The "String Literals as Non-Type
// Template Parameters" feature of C++ 20 would have done it, but it's not
Expand Down Expand Up @@ -582,10 +586,7 @@ inline Str* NewStr(const char* data, int len) {
// log("NewStr s->data_ %p len = %d", s->data_, len);
// log("sizeof(Str) = %d", sizeof(Str));
memcpy(s->data_, data, len);

// So we can pass it directly to C functions. TODO: is this redundant
// because the heap is zero'd?
s->data_[len] = '\0';
assert(s->data_[len] == '\0'); // should be true because Heap was zeroed

s->SetCellLength(obj_len); // So the GC can copy it
return s;
Expand Down
9 changes: 2 additions & 7 deletions mycpp/my_runtime.cc
Expand Up @@ -49,7 +49,7 @@ Str* Str::replace(Str* old, Str* new_str) {

// Second pass to copy into new 'result'
Str* result = NewStr(length);
p_this = data_; // back to beginning
p_this = data_; // back to beginning
char* p_result = result->data_; // advances through 'result'

while (p_this < p_end) {
Expand All @@ -63,12 +63,7 @@ Str* Str::replace(Str* old, Str* new_str) {
p_result++;
}
}
if (0) {
log("length = %d", length);
log("result->data_[length-1] = %c", result->data_[length-1]);
log("result->data_[length] = %c", result->data_[length]);
assert(result->data_[length] == '\0'); // buffer should have been zero'd
}
assert(result->data_[length] == '\0'); // buffer should have been zero'd
return result;
}

Expand Down
3 changes: 3 additions & 0 deletions mycpp/my_runtime_test.cc
Expand Up @@ -68,6 +68,9 @@ TEST collect_test() {
auto s = NewStr("abcdefg");
for (int i = 0; i < 40; ++i) {
s = s->replace(b, bb);

Str* t = NewStr("NUL");

// log("i = %d", i);
// log("len(s) = %d", len(s));
}
Expand Down

0 comments on commit 43f4e25

Please sign in to comment.