Skip to content

Commit

Permalink
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.
  • Loading branch information
brixen committed Aug 6, 2015
1 parent 5f3b880 commit 678570c
Show file tree
Hide file tree
Showing 50 changed files with 261 additions and 353 deletions.
10 changes: 2 additions & 8 deletions vm/builtin/autoload.cpp
Expand Up @@ -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"));
Expand All @@ -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"));
Expand Down
4 changes: 2 additions & 2 deletions vm/builtin/autoload.hpp
Expand Up @@ -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 */

Expand Down
29 changes: 9 additions & 20 deletions vm/builtin/block_environment.cpp
Expand Up @@ -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"));
Expand All @@ -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,
Expand All @@ -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");
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions vm/builtin/block_environment.hpp
Expand Up @@ -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 */
Expand All @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions vm/builtin/channel.cpp
Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions vm/builtin/channel.hpp
Expand Up @@ -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:
Expand Down
22 changes: 11 additions & 11 deletions vm/builtin/class.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
}

Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions vm/builtin/class.hpp
Expand Up @@ -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:
Expand Down
19 changes: 8 additions & 11 deletions vm/builtin/compiled_code.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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_;
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions vm/builtin/compiled_code.hpp
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions vm/builtin/constant_scope.cpp
Expand Up @@ -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"));
Expand Down

0 comments on commit 678570c

Please sign in to comment.