[macruby-changes] [3918] MacRuby/trunk

source_changes at macosforge.org source_changes at macosforge.org
Sat Apr 10 18:57:41 PDT 2010


Revision: 3918
          http://trac.macosforge.org/projects/ruby/changeset/3918
Author:   lsansonetti at apple.com
Date:     2010-04-10 18:57:40 -0700 (Sat, 10 Apr 2010)
Log Message:
-----------
a better super dispatcher

Modified Paths:
--------------
    MacRuby/trunk/compiler.cpp
    MacRuby/trunk/dispatcher.cpp
    MacRuby/trunk/vm.cpp
    MacRuby/trunk/vm.h

Modified: MacRuby/trunk/compiler.cpp
===================================================================
--- MacRuby/trunk/compiler.cpp	2010-04-08 18:43:26 UTC (rev 3917)
+++ MacRuby/trunk/compiler.cpp	2010-04-11 01:57:40 UTC (rev 3918)
@@ -4230,7 +4230,7 @@
 			//	// ...
 			//	SEL super_sel = sel;
 			//  	if (super_sel == 0) {
-			//		super_sel = <hardcoded-mid>;
+			//	    super_sel = <hardcoded-mid>;
 			//	}
 			//	rb_vm_dispatch(..., super_sel, ...);
 			Function *f = bb->getParent();

Modified: MacRuby/trunk/dispatcher.cpp
===================================================================
--- MacRuby/trunk/dispatcher.cpp	2010-04-08 18:43:26 UTC (rev 3917)
+++ MacRuby/trunk/dispatcher.cpp	2010-04-11 01:57:40 UTC (rev 3918)
@@ -48,7 +48,7 @@
 	}
 	else if (i < arity.left_req + opt_args) {
 	    // optional args
-	    int opt_arg_index = i - arity.left_req;
+	    const int opt_arg_index = i - arity.left_req;
 	    if (opt_arg_index >= used_opt_args) {
 		new_argv[i] = Qundef;
 	    }
@@ -58,7 +58,7 @@
 	}
 	else if (i == rest_pos) {
 	    // rest
-	    int rest_size = argc - arity.real + 1;
+	    const int rest_size = argc - arity.real + 1;
 	    if (rest_size <= 0) {
 		new_argv[i] = rb_ary_new();
 	    }
@@ -86,7 +86,8 @@
 
     assert(pimp != NULL);
 
-    VALUE (*imp)(VALUE, SEL, VALUE, rb_vm_block_t *,  ...) = (VALUE (*)(VALUE, SEL, VALUE, rb_vm_block_t *, ...))pimp;
+    VALUE (*imp)(VALUE, SEL, VALUE, rb_vm_block_t *,  ...) =
+	(VALUE (*)(VALUE, SEL, VALUE, rb_vm_block_t *, ...))pimp;
 
     switch (argc) {
 	case 0:
@@ -98,23 +99,27 @@
 	case 3:
 	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2]);
 	case 4:
-	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2], argv[3]);
+	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2],
+		    argv[3]);
 	case 5:
-	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2], argv[3], argv[4]);
+	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2],
+		    argv[3], argv[4]);
 	case 6:
-	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5]);
+	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2],
+		    argv[3], argv[4], argv[5]);
 	case 7:
-	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6]);
+	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2],
+		    argv[3], argv[4], argv[5], argv[6]);
 	case 8:
-	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6], argv[7]);
+	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2],
+		    argv[3], argv[4], argv[5], argv[6], argv[7]);
 	case 9:
-	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6], argv[7], argv[8]);
+	    return (*imp)(self, sel, dvars, b, argv[0], argv[1], argv[2],
+		    argv[3], argv[4], argv[5], argv[6], argv[7], argv[8]);
     }
 
-    rb_vm_long_arity_bstub_t *stub;
-
-    stub = (rb_vm_long_arity_bstub_t *)GET_CORE()->gen_large_arity_stub(argc,
-	true);
+    rb_vm_long_arity_bstub_t *stub = (rb_vm_long_arity_bstub_t *)
+	GET_CORE()->gen_large_arity_stub(argc, true);
     return (*stub)(pimp, (id)self, sel, dvars, b, argc, argv);
 }
 
@@ -145,24 +150,31 @@
 	case 4:
 	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3]);
 	case 5:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4]);
 	case 6:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4], argv[5]);
 	case 7:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4], argv[5], argv[6]);
 	case 8:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6], argv[7]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4], argv[5], argv[6], argv[7]);
 	case 9:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6], argv[7], argv[8]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4], argv[5], argv[6], argv[7], argv[8]);
 	case 10:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6], argv[7], argv[8], argv[9]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4], argv[5], argv[6], argv[7], argv[8], argv[9]);
 	case 11:
-	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3], argv[4], argv[5], argv[6], argv[7], argv[8], argv[9], argv[10]);
+	    return (*imp)(self, sel, argv[0], argv[1], argv[2], argv[3],
+		    argv[4], argv[5], argv[6], argv[7], argv[8], argv[9],
+		    argv[10]);
     }
 
-    rb_vm_long_arity_stub_t *stub;
-
-    stub = (rb_vm_long_arity_stub_t *)GET_CORE()->gen_large_arity_stub(argc);
+    rb_vm_long_arity_stub_t *stub = (rb_vm_long_arity_stub_t *)
+	GET_CORE()->gen_large_arity_stub(argc);
     return (*stub)(pimp, (id)self, sel, argc, argv);
 }
 
@@ -203,7 +215,7 @@
     if (len >= 3 && isalpha(p[len - 3]) && p[len - 2] == '='
 	&& p[len - 1] == ':') {
 
-	/* foo=: -> setFoo: shortcut */
+	// foo=: -> setFoo: shortcut
 	snprintf(buf, sizeof buf, "set%s", p);
 	buf[3] = toupper(buf[3]);
 	buf[len + 1] = ':';
@@ -211,98 +223,50 @@
 	new_sel = sel_registerName(buf);
     }
     else if (len > 1 && p[len - 1] == '?') {
-	/* foo?: -> isFoo: shortcut */
+	// foo?: -> isFoo: shortcut
 	snprintf(buf, sizeof buf, "is%s", p);
 	buf[2] = toupper(buf[2]);
 	buf[len + 1] = '\0';
 	new_sel = sel_registerName(buf);
     }
     else if (strcmp(p, "[]:") == 0) {
+	// []: -> objectForKey: shortcut
 	new_sel = selObjectForKey;
     }
     else if (strcmp(p, "[]=:") == 0) {
+	// []=: -> setObjectForKey: shortcut
 	new_sel = selSetObjectForKey;
     }
 
     return new_sel;
 }
 
-static IMP
-objc_imp(IMP imp)
-{
-    rb_vm_method_node_t *node = GET_CORE()->method_node_get(imp);
-    if (node != NULL && node->ruby_imp == imp) {
-	imp = node->objc_imp;
-    }
-    return imp;
-}
-
 static Method
-rb_vm_super_lookup(VALUE klass, SEL sel)
+rb_vm_super_lookup(Class klass, SEL sel, Class *super_class_p)
 {
     // Locate the current method implementation.
-    Class k = (Class)klass;
-    IMP self;
-    while (true) {
-	Method m = class_getInstanceMethod(k, sel);
-	if (m == NULL) {
-	    // The given selector does not exist, let's go through
-	    // #method_missing...
-	    return NULL; 
-	}
-	assert(m != NULL);
-	self = method_getImplementation(m);
-	if (UNAVAILABLE_IMP(self)) {
-	    k = class_getSuperclass(k);
-	    assert(k != NULL);
-	}
-	else {
-	    break;
-	}
+    Class self_class = klass;
+    Method method = class_getInstanceMethod(self_class, sel);
+    if (method == NULL) {
+	// The given selector does not exist, let's go through
+	// #method_missing...
+	*super_class_p = NULL;
+	return NULL; 
     }
+    IMP self_imp = method_getImplementation(method);
 
-    // Compute the stack call implementations right after our current method.
-    void *callstack[128];
-    int callstack_n = backtrace(callstack, 128);
-    std::vector<void *> callstack_funcs;
-    bool skip = true;
-    for (int i = callstack_n - 1; i >= 0; i--) {
-	void *start = NULL;
-	if (GET_CORE()->symbolize_call_address(callstack[i],
-		    &start, NULL, 0, NULL, NULL, 0)) {
-	    start = (void *)objc_imp((IMP)start);
-	    if (start == (void *)self) {
-		skip = false;
-	    }
-	    if (!skip) {
-		callstack_funcs.push_back(start);
-	    }
-	}
-    }
-
-    // Iterate over ancestors and return the first method that isn't on
-    // the stack.
-    VALUE ary = rb_mod_ancestors_nocopy(klass);
+    // Iterate over ancestors, locate the current class and return the
+    // super method, if it exists.
+    VALUE ary = rb_mod_ancestors_nocopy((VALUE)klass);
     const int count = RARRAY_LEN(ary);
     bool klass_located = false;
-
 #if ROXOR_VM_DEBUG
     printf("locating super method %s of class %s in ancestor chain %s\n", 
-	    sel_getName(sel), rb_class2name(klass),
+	    sel_getName(sel), rb_class2name((VALUE)klass),
 	    RSTRING_PTR(rb_inspect(ary)));
-    printf("callstack functions: ");
-    for (std::vector<void *>::iterator iter = callstack_funcs.begin();
-	 iter != callstack_funcs.end();
-	 ++iter) {
-	printf("%p ", *iter);
-    }
-    printf("\n");
 #endif
-
-    //assert(!callstack_funcs.empty());
-
     for (int i = 0; i < count; i++) {
-        if (!klass_located && RARRAY_AT(ary, i) == klass) {
+        if (!klass_located && RARRAY_AT(ary, i) == (VALUE)self_class) {
             klass_located = true;
         }
         if (klass_located) {
@@ -310,32 +274,33 @@
                 VALUE k = RARRAY_AT(ary, i + 1);
 
 		Method method = class_getInstanceMethod((Class)k, sel);
-		VALUE super = RCLASS_SUPER(k);
-
 		if (method == NULL) {
 		    continue;
 		}
 
 		IMP imp = method_getImplementation(method);
-		if (UNAVAILABLE_IMP(imp)
-		    || (super != 0 && class_getInstanceMethod((Class)super,
-			    sel) == method)) {
+		if (imp == self_imp || UNAVAILABLE_IMP(imp)) {
 		    continue;
 		}
 
-		if (std::find(callstack_funcs.begin(), callstack_funcs.end(), 
-			    (void *)imp) == callstack_funcs.end()) {
-		    // Method is not on stack.
+		VALUE super = RCLASS_SUPER(k);
+		if (super != 0 && class_getInstanceMethod((Class)super,
+			    sel) == method) {
+		    continue;
+		}
+
 #if ROXOR_VM_DEBUG
-		    printf("returning method implementation %p " \
-		    	   "from class/module %s\n", imp, rb_class2name(k));
+		printf("returning method %p of class %s (#%d)\n",
+			method, rb_class2name(k), i + 1);
 #endif
-		    return method;
-		}
+
+		*super_class_p = (Class)k;
+		return method;
             }
         }
     }
 
+    *super_class_p = NULL;
     return NULL;
 }
 
@@ -574,6 +539,18 @@
     return true;
 }
 
+static inline bool
+sel_equal(Class klass, SEL x, SEL y)
+{
+    if (x == y) {
+	return true;
+    }
+
+    IMP x_imp = class_getMethodImplementation(klass, x);
+    IMP y_imp = class_getMethodImplementation(klass, y);
+    return x_imp == y_imp;
+}
+
 static force_inline VALUE
 __rb_vm_dispatch(RoxorVM *vm, struct mcache *cache, VALUE top, VALUE self,
 	Class klass, SEL sel, rb_vm_block_t *block, unsigned char opt,
@@ -595,6 +572,16 @@
 #endif
     bool cache_method = true;
 
+    Class current_super_class = vm->get_current_super_class();
+    SEL current_super_sel = vm->get_current_super_sel();
+
+    // XXX: super method cache is disabled because it causes random runtime
+    // bugs in rails. Instead of fixing the problem, we wait for the new
+    // method cache implementation which should be different (and thread-safe).
+    if (opt == DISPATCH_SUPER) {
+	cache->flag = 0;
+    }
+
     if (cache->flag == 0) {
 recache:
 #if ROXOR_VM_DEBUG
@@ -603,7 +590,12 @@
 
 	Method method;
 	if (opt == DISPATCH_SUPER) {
-	    method = rb_vm_super_lookup((VALUE)klass, sel);
+	    if (!sel_equal(klass, current_super_sel, sel)) {
+		current_super_sel = sel;
+		current_super_class = klass;
+	    }
+	    method = rb_vm_super_lookup(current_super_class, sel,
+		    &current_super_class);
 	}
 	else {
 	    method = class_getInstanceMethod(klass, sel);
@@ -747,7 +739,7 @@
 	}
 
 #if ROXOR_VM_DEBUG
-	printf("ruby dispatch %c[<%s %p> %s] (imp %p block %p argc %d opt %d cached %s)\n",
+	printf("ruby dispatch %c[<%s %p> %s] (imp %p block %p argc %d opt %d cache %p cached %s)\n",
 		class_isMetaClass(klass) ? '+' : '-',
 		class_getName(klass),
 		(void *)self,
@@ -756,6 +748,7 @@
 		block,
 		argc,
 		opt,
+		cache,
 		cached ? "true" : "false");
 #endif
 
@@ -766,14 +759,24 @@
 	}
 	vm->set_current_class(NULL);
 
+	Class old_current_super_class = vm->get_current_super_class();
+	vm->set_current_super_class(current_super_class);
+	SEL old_current_super_sel = vm->get_current_super_sel();
+	vm->set_current_super_sel(current_super_sel);
+
 	struct Finally {
 	    bool block_already_current;
 	    Class current_class;
+	    Class current_super_class;
+	    SEL current_super_sel;
 	    RoxorVM *vm;
 	    Finally(bool _block_already_current, Class _current_class,
+		    Class _current_super_class, SEL _current_super_sel,
 		    RoxorVM *_vm) {
 		block_already_current = _block_already_current;
 		current_class = _current_class;
+		current_super_class = _current_super_class;
+		current_super_sel = _current_super_sel;
 		vm = _vm;
 	    }
 	    ~Finally() {
@@ -782,8 +785,11 @@
 		}
 		vm->set_current_class(current_class);
 		vm->pop_broken_with();
+		vm->set_current_super_class(current_super_class);
+		vm->set_current_super_sel(current_super_sel);
 	    }
-	} finalizer(block_already_current, current_klass, vm);
+	} finalizer(block_already_current, current_klass,
+		old_current_super_class, old_current_super_sel, vm);
 
 	// DTrace probe: method__entry
 	if (MACRUBY_METHOD_ENTRY_ENABLED()) {

Modified: MacRuby/trunk/vm.cpp
===================================================================
--- MacRuby/trunk/vm.cpp	2010-04-08 18:43:26 UTC (rev 3917)
+++ MacRuby/trunk/vm.cpp	2010-04-11 01:57:40 UTC (rev 3918)
@@ -343,6 +343,8 @@
     parse_in_eval = false;
     has_ensure = false;
     return_from_block = -1;
+    current_super_class = NULL;
+    current_super_sel = 0;
 }
 
 static inline void *
@@ -402,6 +404,8 @@
     has_ensure = false;
     return_from_block = -1;
     throw_exc = NULL;
+    current_super_class = NULL;
+    current_super_sel = 0;
 }
 
 RoxorVM::~RoxorVM(void)

Modified: MacRuby/trunk/vm.h
===================================================================
--- MacRuby/trunk/vm.h	2010-04-08 18:43:26 UTC (rev 3917)
+++ MacRuby/trunk/vm.h	2010-04-11 01:57:40 UTC (rev 3918)
@@ -37,8 +37,9 @@
 				// (temporary)
 
 typedef struct rb_vm_block {
-    int flags; 	// IMPORTANT: this field should always be at the beginning.
-		// Look at how rb_vm_take_ownership() is called in compiler.cpp.
+    // IMPORTANT: the flags field should always be at the beginning.
+    // Look at how rb_vm_take_ownership() is called in compiler.cpp.
+    int flags;
     VALUE proc; // a reference to a Proc object, or nil.
     VALUE self;
     VALUE klass;
@@ -50,9 +51,9 @@
     struct rb_vm_var_uses **parent_var_uses;
     struct rb_vm_block *parent_block;
     int dvars_size;
+    // IMPORTANT: do not add fields after dvars, because it would mess with
+    // the way the structure is allocated.
     VALUE *dvars[1];
-    // ATTENTION: do not add fields here, because it would mess with the way
-    // the structure is allocated regarding the dvars buffer!
 } rb_vm_block_t;
 
 typedef struct {
@@ -932,6 +933,8 @@
 	bool parse_in_eval;
 	bool has_ensure;
 	int return_from_block;
+	Class current_super_class;
+	SEL current_super_sel;	
 
 	RoxorCatchThrowException *throw_exc;
 
@@ -956,6 +959,8 @@
 	ACCESSOR(has_ensure, bool);
 	ACCESSOR(return_from_block, int);
 	ACCESSOR(throw_exc, RoxorCatchThrowException *);
+	ACCESSOR(current_super_class, Class);
+	ACCESSOR(current_super_sel, SEL);
 
 	std::string debug_blocks(void);
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/macruby-changes/attachments/20100410/f7a0d16b/attachment-0001.html>


More information about the macruby-changes mailing list