Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Removed GCToken.
Browse files Browse the repository at this point in the history
This was a well-intentioned idea but not practical or useful.

The idea was to have the compiler help check where in call paths a
garbage-collection cycle could run. Unfortnately, adding this in as an
after-thought resulted in all the places where GCTokens are created from thin
air deep in some call path. It didn't change the fact that GC could happen
pretty much anywhere.

In a managed runtime, either GC can happen everywhere or it should only happen
at a very small number of extremely well-defined points.  The middle ground of
"it can happen at all these places" is an invitation for a low budge horror
movie, dismembered objects strewn throughout the code.

Along with the rework of the stop-the-world mechanism, the removal of GCToken
and restricting the invocation of a GC cycle to a single well-defined method
call in a few well-defined locations, and finally, making all allocation paths
GC-safe (ie GC will NOT run when allocating an object), Rubinius will have much
better defined GC behavior.

The GC safe allocation path is important for cases like the string_dup
instruction, where a young GC cycle could run when allocating the dup and the
original String (eg a literal String in a method) is in the young generation
and moved. Since the original String is on the C stack and not in a GC root
object, the dup fails when copying the contents of the original String. It's
better to make allocation GC-safe than to accept the performance cost of the GC
root in these sorts of cases. Also, that case is only one well-defined instance
of the issue. There are more complicated ones.
brixen committed Aug 6, 2015
1 parent 5f3b880 commit 678570c
Showing 50 changed files with 261 additions and 353 deletions.
10 changes: 2 additions & 8 deletions vm/builtin/autoload.cpp
Original file line number Diff line number Diff line change
@@ -14,13 +14,7 @@ namespace rubinius {
G(autoload)->set_object_type(state, AutoloadType);
}

/*
* We added GCToken because we use send(). send() itself doesn't accept
* a GCToken because it was tried and impacted performance severely. We
* include it here anyway so methods using autoload resolving can see
* it can trigger a GC due to calling back into Ruby.
*/
Object* Autoload::resolve(STATE, GCToken gct, CallFrame* call_frame, Module* under, bool honor_require) {
Object* Autoload::resolve(STATE, CallFrame* call_frame, Module* under, bool honor_require) {
Autoload* self = this;
OnStack<1> os(state, self);
Object* res = send(state, call_frame, state->symbol("resolve"));
@@ -41,7 +35,7 @@ namespace rubinius {
return cNil;
}

Object* Autoload::resolve(STATE, GCToken gct, CallFrame* call_frame, bool honor_require) {
Object* Autoload::resolve(STATE, CallFrame* call_frame, bool honor_require) {
Autoload* self = this;
OnStack<1> os(state, self);
Object* res = send(state, call_frame, state->symbol("resolve"));
4 changes: 2 additions & 2 deletions vm/builtin/autoload.hpp
Original file line number Diff line number Diff line change
@@ -33,8 +33,8 @@ namespace rubinius {
// Rubinius.primitive :autoload_allocate
static Autoload* create(STATE);

Object* resolve(STATE, GCToken gct, CallFrame* call_frame, Module* under, bool honor_require = false);
Object* resolve(STATE, GCToken gct, CallFrame* call_frame, bool honor_require = false);
Object* resolve(STATE, CallFrame* call_frame, Module* under, bool honor_require = false);
Object* resolve(STATE, CallFrame* call_frame, bool honor_require = false);

public: /* TypeInfo */

29 changes: 9 additions & 20 deletions vm/builtin/block_environment.cpp
Original file line number Diff line number Diff line change
@@ -37,8 +37,7 @@ namespace rubinius {
}

void BlockEnvironment::bootstrap_methods(STATE) {
GCTokenImpl gct;
System::attach_primitive(state, gct,
System::attach_primitive(state,
G(blokenv), false,
state->symbol("call_under"),
state->symbol("block_call_under"));
@@ -58,8 +57,8 @@ namespace rubinius {
return env;
}

MachineCode* BlockEnvironment::machine_code(STATE, GCToken gct, CallFrame* call_frame) {
return compiled_code_->internalize(state, gct, call_frame);
MachineCode* BlockEnvironment::machine_code(STATE, CallFrame* call_frame) {
return compiled_code_->internalize(state, call_frame);
}

Object* BlockEnvironment::invoke(STATE, CallFrame* previous,
@@ -73,9 +72,8 @@ namespace rubinius {
OnStack<3> iv(state, invocation.self, invocation.constant_scope, invocation.module);
VariableRootBuffer vrb(state->vm()->current_root_buffers(),
&args.arguments_location(), args.total());
GCTokenImpl gct;

mcode = env->machine_code(state, gct, previous);
mcode = env->machine_code(state, previous);

if(!mcode) {
Exception::internal_error(state, previous, "invalid bytecode method");
@@ -471,10 +469,7 @@ namespace rubinius {

// Check the stack and interrupts here rather than in the interpreter
// loop itself.

GCTokenImpl gct;

if(!state->check_interrupts(gct, frame, frame)) return NULL;
if(!state->check_interrupts(frame, frame)) return NULL;

state->vm()->checkpoint(state);

@@ -483,21 +478,15 @@ namespace rubinius {
} else {
// Check the stack and interrupts here rather than in the interpreter
// loop itself.

GCTokenImpl gct;

if(!state->check_interrupts(gct, frame, frame)) return NULL;
if(!state->check_interrupts(frame, frame)) return NULL;

state->vm()->checkpoint(state);
return (*mcode->run)(state, mcode, frame);
}
#else
// Check the stack and interrupts here rather than in the interpreter
// loop itself.

GCTokenImpl gct;

if(!state->check_interrupts(gct, frame, frame)) return NULL;
if(!state->check_interrupts(frame, frame)) return NULL;

state->vm()->checkpoint(state);
return (*mcode->run)(state, mcode, frame);
@@ -559,15 +548,15 @@ namespace rubinius {
}


BlockEnvironment* BlockEnvironment::under_call_frame(STATE, GCToken gct,
BlockEnvironment* BlockEnvironment::under_call_frame(STATE,
CompiledCode* ccode, MachineCode* caller,
CallFrame* call_frame)
{

MachineCode* mcode = ccode->machine_code();
if(!mcode) {
OnStack<1> os(state, ccode);
mcode = ccode->internalize(state, gct, call_frame);
mcode = ccode->internalize(state, call_frame);
if(!mcode) {
Exception::internal_error(state, call_frame, "invalid bytecode method");
return 0;
4 changes: 2 additions & 2 deletions vm/builtin/block_environment.hpp
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ namespace rubinius {
ConstantScope* constant_scope_; // slot
Module* module_; // slot

MachineCode* machine_code(STATE, GCToken gct, CallFrame* call_frame);
MachineCode* machine_code(STATE, CallFrame* call_frame);

public:
/* accessors */
@@ -64,7 +64,7 @@ namespace rubinius {
BlockEnvironment* env, Arguments& args,
BlockInvocation& invocation);

static BlockEnvironment* under_call_frame(STATE, GCToken gct, CompiledCode* cm,
static BlockEnvironment* under_call_frame(STATE, CompiledCode* cm,
MachineCode* caller, CallFrame* call_frame);

static Object* execute_interpreter(STATE, CallFrame* previous,
10 changes: 5 additions & 5 deletions vm/builtin/channel.cpp
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ namespace rubinius {
return chan;
}

Object* Channel::send(STATE, GCToken gct, Object* val, CallFrame* calling_environment) {
Object* Channel::send(STATE, Object* val, CallFrame* calling_environment) {
Channel* self = this;

OnStack<2> os(state, val, self);
@@ -62,7 +62,7 @@ namespace rubinius {
return cNil;
}

Object* Channel::try_receive(STATE, GCToken gct, CallFrame* calling_environment) {
Object* Channel::try_receive(STATE, CallFrame* calling_environment) {
Channel* self = this;
OnStack<1> os(state, self);

@@ -77,12 +77,12 @@ namespace rubinius {
return self->value_->shift(state);
}

Object* Channel::receive(STATE, GCToken gct, CallFrame* call_frame) {
return receive_timeout(state, gct, cNil, call_frame);
Object* Channel::receive(STATE, CallFrame* call_frame) {
return receive_timeout(state, cNil, call_frame);
}

#define NANOSECONDS 1000000000
Object* Channel::receive_timeout(STATE, GCToken gct, Object* duration, CallFrame* call_frame) {
Object* Channel::receive_timeout(STATE, Object* duration, CallFrame* call_frame) {
// Passing control away means that the GC might run. So we need
// to stash this into a root, and read it back out again after
// control is returned.
8 changes: 4 additions & 4 deletions vm/builtin/channel.hpp
Original file line number Diff line number Diff line change
@@ -37,16 +37,16 @@ namespace rubinius {
static Channel* create(STATE);

// Rubinius.primitive :channel_send
Object* send(STATE, GCToken gct, Object* val, CallFrame* calling_environment);
Object* send(STATE, Object* val, CallFrame* calling_environment);

// Rubinius.primitive :channel_receive
Object* receive(STATE, GCToken gct, CallFrame* calling_environment);
Object* receive(STATE, CallFrame* calling_environment);

// Rubinius.primitive :channel_try_receive
Object* try_receive(STATE, GCToken gct, CallFrame* calling_environment);
Object* try_receive(STATE, CallFrame* calling_environment);

// Rubinius.primitive :channel_receive_timeout
Object* receive_timeout(STATE, GCToken gct, Object* duration, CallFrame* calling_environment);
Object* receive_timeout(STATE, Object* duration, CallFrame* calling_environment);

class Info : public TypeInfo {
public:
22 changes: 11 additions & 11 deletions vm/builtin/class.cpp
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ namespace rubinius {
}

namespace {
Object* collect_and_allocate(STATE, GCToken gct, Class* self,
Object* collect_and_allocate(STATE, Class* self,
CallFrame* calling_environment)
{
OnStack<1> os(state, self);
@@ -91,7 +91,7 @@ namespace rubinius {
return obj;
}

Object* allocate_packed(STATE, GCToken gct, Class* self,
Object* allocate_packed(STATE, Class* self,
CallFrame* calling_environment)
{
uint32_t size = self->packed_size();
@@ -113,7 +113,7 @@ namespace rubinius {
state->memory()->new_object_typed(state, self, size, PackedObject::type));
}
} else {
return collect_and_allocate(state, gct, self, calling_environment);
return collect_and_allocate(state, self, calling_environment);
}
}

@@ -128,11 +128,11 @@ namespace rubinius {
}
}

Object* Class::allocate(STATE, GCToken gct, CallFrame* calling_environment) {
Object* Class::allocate(STATE, CallFrame* calling_environment) {
object_type obj_type = type_info_->type;

if(obj_type == PackedObject::type) {
Object* new_obj = allocate_packed(state, gct, this, calling_environment);
Object* new_obj = allocate_packed(state, this, calling_environment);
#ifdef RBX_ALLOC_TRACKING
if(unlikely(state->vm()->allocation_tracking())) {
new_obj->setup_allocation_site(state, calling_environment);
@@ -157,8 +157,8 @@ namespace rubinius {
Class* self = this;
OnStack<1> os(state, self);

auto_pack(state, gct, calling_environment);
Object* new_obj = allocate_packed(state, gct, self, calling_environment);
auto_pack(state, calling_environment);
Object* new_obj = allocate_packed(state, self, calling_environment);
#ifdef RBX_ALLOC_TRACKING
if(unlikely(state->vm()->allocation_tracking())) {
new_obj->setup_allocation_site(state, calling_environment);
@@ -236,16 +236,16 @@ namespace rubinius {
*
* This locks the class so that construction is serialized.
*/
void Class::auto_pack(STATE, GCToken gct, CallFrame* call_frame) {
void Class::auto_pack(STATE, CallFrame* call_frame) {
Class* self = this;
OnStack<1> os(state, self);

hard_lock(state, gct, call_frame);
hard_lock(state, call_frame);

// If another thread did this work while we were waiting on the lock,
// don't redo it.
if(self->type_info_->type == PackedObject::type) {
self->hard_unlock(state, gct, call_frame);
self->hard_unlock(state, call_frame);
return;
}

@@ -295,7 +295,7 @@ namespace rubinius {
atomic::memory_barrier();
self->set_object_type(state, PackedObject::type);

self->hard_unlock(state, gct, call_frame);
self->hard_unlock(state, call_frame);
}

Class* Class::real_class(STATE, Class* klass) {
4 changes: 2 additions & 2 deletions vm/builtin/class.hpp
Original file line number Diff line number Diff line change
@@ -96,12 +96,12 @@ namespace rubinius {
static Class* s_allocate(STATE);

// Rubinius.primitive+ :class_allocate
Object* allocate(STATE, GCToken gct, CallFrame* calling_environment);
Object* allocate(STATE, CallFrame* calling_environment);

// Rubinius.primitive :class_set_superclass
Object* set_superclass(STATE, Object* sup);

void auto_pack(STATE, GCToken gct, CallFrame* call_frame);
void auto_pack(STATE, CallFrame* call_frame);

class Info : public Module::Info {
public:
19 changes: 8 additions & 11 deletions vm/builtin/compiled_code.cpp
Original file line number Diff line number Diff line change
@@ -65,24 +65,22 @@ namespace rubinius {
}

Tuple* CompiledCode::call_sites(STATE, CallFrame* calling_environment) {
GCTokenImpl gct;
CompiledCode* self = this;
OnStack<1> os(state, self);

if(self->machine_code_ == NULL) {
if(!self->internalize(state, gct, calling_environment)) return force_as<Tuple>(Primitives::failure());
if(!self->internalize(state, calling_environment)) return force_as<Tuple>(Primitives::failure());
}
MachineCode* mcode = self->machine_code_;
return mcode->call_sites(state);
}

Tuple* CompiledCode::constant_caches(STATE, CallFrame* calling_environment) {
GCTokenImpl gct;
CompiledCode* self = this;
OnStack<1> os(state, self);

if(self->machine_code_ == NULL) {
if(!self->internalize(state, gct, calling_environment)) return force_as<Tuple>(Primitives::failure());
if(!self->internalize(state, calling_environment)) return force_as<Tuple>(Primitives::failure());
}
MachineCode* mcode = self->machine_code_;
return mcode->constant_caches(state);
@@ -121,7 +119,7 @@ namespace rubinius {
return as<Fixnum>(lines_->at(fin+1))->to_native();
}

MachineCode* CompiledCode::internalize(STATE, GCToken gct, CallFrame* call_frame,
MachineCode* CompiledCode::internalize(STATE, CallFrame* call_frame,
const char** reason, int* ip)
{
MachineCode* mcode = machine_code_;
@@ -133,7 +131,7 @@ namespace rubinius {
CompiledCode* self = this;
OnStack<1> os(state, self);

self->hard_lock(state, gct, call_frame);
self->hard_lock(state, call_frame);

mcode = self->machine_code_;
if(!mcode) {
@@ -164,7 +162,7 @@ namespace rubinius {
set_executor(mcode->fallback);
}

self->hard_unlock(state, gct, call_frame);
self->hard_unlock(state, call_frame);
return mcode;
}

@@ -213,9 +211,8 @@ namespace rubinius {

VariableRootBuffer vrb(state->vm()->current_root_buffers(),
&args.arguments_location(), args.total());
GCTokenImpl gct;

if(!code->internalize(state, gct, call_frame, &reason, &ip)) {
if(!code->internalize(state, call_frame, &reason, &ip)) {
Exception::bytecode_error(state, call_frame, code, ip, reason);
return 0;
}
@@ -356,13 +353,13 @@ namespace rubinius {
return false;
}

Object* CompiledCode::set_breakpoint(STATE, GCToken gct, Fixnum* ip, Object* bp, CallFrame* calling_environment) {
Object* CompiledCode::set_breakpoint(STATE, Fixnum* ip, Object* bp, CallFrame* calling_environment) {
CompiledCode* self = this;
OnStack<3> os(state, self, ip, bp);

int i = ip->to_native();
if(self->machine_code_ == NULL) {
if(!self->internalize(state, gct, calling_environment)) return Primitives::failure();
if(!self->internalize(state, calling_environment)) return Primitives::failure();
}

if(!self->machine_code_->validate_ip(state, i)) return Primitives::failure();
4 changes: 2 additions & 2 deletions vm/builtin/compiled_code.hpp
Original file line number Diff line number Diff line change
@@ -100,14 +100,14 @@ namespace rubinius {

void post_marshal(STATE);
size_t number_of_locals();
MachineCode* internalize(STATE, GCToken gct, CallFrame* call_frame, const char** failure_reason=0, int* ip=0);
MachineCode* internalize(STATE, CallFrame* call_frame, const char** failure_reason=0, int* ip=0);
void specialize(STATE, TypeInfo* ti);

static Object* default_executor(STATE, CallFrame*, Executable* exec, Module* mod, Arguments& args);
static Object* specialized_executor(STATE, CallFrame*, Executable* exec, Module* mod, Arguments& args);

// Rubinius.primitive :compiledcode_set_breakpoint
Object* set_breakpoint(STATE, GCToken gct, Fixnum* ip, Object* bp, CallFrame* calling_environment);
Object* set_breakpoint(STATE, Fixnum* ip, Object* bp, CallFrame* calling_environment);

// Rubinius.primitive :compiledcode_clear_breakpoint
Object* clear_breakpoint(STATE, Fixnum* ip);
3 changes: 1 addition & 2 deletions vm/builtin/constant_scope.cpp
Original file line number Diff line number Diff line change
@@ -12,8 +12,7 @@ namespace rubinius {
}

void ConstantScope::bootstrap_methods(STATE) {
GCTokenImpl gct;
System::attach_primitive(state, gct,
System::attach_primitive(state,
G(constantscope), false,
state->symbol("const_set"),
state->symbol("constant_scope_const_set"));
Loading

0 comments on commit 678570c

Please sign in to comment.