[PATCH] variable.c: remove custom locking for autoload

[PATCH] variable.c: remove custom locking for autoload

[Eric Wong]

This reverts r52446, r52335, r52332
---
 variable.c | 122 +++++++++++++++----------------------------------------------
 1 file changed, 29 insertions(+), 93 deletions(-)

diff --git a/variable.c b/variable.c
index ec6924a..2a8991b 100644
--- a/variable.c
+++ b/variable.c
@@ -16,7 +16,6 @@
 #include "ruby/util.h"
 #include "constant.h"
 #include "id.h"
-#include "ccan/list/list.h"
 #include "id_table.h"
 
 struct rb_id_table *rb_global_tbl;
@@ -1891,24 +1890,11 @@ autoload_data(VALUE mod, ID id)
     return (VALUE)val;
 }
 
-/* always on stack, no need to mark */
-struct autoload_state {
-    struct autoload_data_i *ele;
-    VALUE mod;
-    VALUE result;
-    ID id;
-    VALUE thread;
-    union {
-	struct list_node node;
-	struct list_head head;
-    } waitq;
-};
-
 struct autoload_data_i {
     VALUE feature;
     int safe_level;
+    VALUE thread;
     VALUE value;
-    struct autoload_state *state; /* points to on-stack struct */
 };
 
 static void
@@ -1916,6 +1902,7 @@ autoload_i_mark(void *ptr)
 {
     struct autoload_data_i *p = ptr;
     rb_gc_mark(p->feature);
+    rb_gc_mark(p->thread);
     rb_gc_mark(p->value);
 }
 
@@ -1975,8 +1962,8 @@ rb_autoload(VALUE mod, ID id, const char *file)
     ad = TypedData_Make_Struct(0, struct autoload_data_i, &autoload_data_i_type, ele);
     ele->feature = fn;
     ele->safe_level = rb_safe_level();
+    ele->thread = Qnil;
     ele->value = Qundef;
-    ele->state = 0;
     st_insert(tbl, (st_data_t)id, (st_data_t)ad);
 }
 
@@ -2049,7 +2036,7 @@ rb_autoloading_value(VALUE mod, ID id, VALUE* value)
     if (!(load = autoload_data(mod, id)) || !(ele = check_autoload_data(load))) {
 	return 0;
     }
-    if (ele->state && ele->state->thread == rb_thread_current()) {
+    if (ele->thread == rb_thread_current()) {
 	if (ele->value != Qundef) {
 	    if (value) {
 		*value = ele->value;
@@ -2092,58 +2079,17 @@ autoload_const_set(VALUE arg)
 static VALUE
 autoload_require(VALUE arg)
 {
-    struct autoload_state *state = (struct autoload_state *)arg;
-
-    /* this may release GVL and switch threads: */
-    state->result = rb_funcall(rb_vm_top_self(), rb_intern("require"), 1,
-			       state->ele->feature);
-
-    return state->result;
+    struct autoload_data_i *ele = (struct autoload_data_i *)arg;
+    return rb_funcall(rb_vm_top_self(), rb_intern("require"), 1, ele->feature);
 }
 
 static VALUE
 autoload_reset(VALUE arg)
 {
-    struct autoload_state *state = (struct autoload_state *)arg;
-    int need_wakeups = 0;
-
-    if (state->ele->state == state) {
-	need_wakeups = 1;
-	state->ele->state = 0;
-    }
-
-    /* At the last, move a value defined in autoload to constant table */
-    if (RTEST(state->result) && state->ele->value != Qundef) {
-	int safe_backup;
-	struct autoload_const_set_args args;
-
-	args.mod = state->mod;
-	args.id = state->id;
-	args.value = state->ele->value;
-	safe_backup = rb_safe_level();
-	rb_set_safe_level_force(state->ele->safe_level);
-	rb_ensure(autoload_const_set, (VALUE)&args,
-	          reset_safe, (VALUE)safe_backup);
-    }
-
-    /* wakeup any waiters we had */
-    if (need_wakeups) {
-	struct autoload_state *cur = 0, *nxt;
-
-	list_for_each_safe(&state->waitq.head, cur, nxt, waitq.node) {
-	    VALUE th = cur->thread;
-
-	    cur->thread = Qfalse;
-	    list_del(&cur->waitq.node);
-
-	    /*
-	     * cur is stored on the stack of cur->waiting_th,
-	     * do not touch cur after waking up waiting_th
-	     */
-	    rb_thread_wakeup_alive(th);
-	}
+    struct autoload_data_i *ele = (struct autoload_data_i *)arg;
+    if (ele->thread == rb_thread_current()) {
+	ele->thread = Qnil;
     }
-
     return 0;			/* ignored */
 }
 
@@ -2153,7 +2099,6 @@ rb_autoload_load(VALUE mod, ID id)
     VALUE load, result;
     const char *loading = 0, *src;
     struct autoload_data_i *ele;
-    struct autoload_state state;
 
     if (!autoload_defined_p(mod, id)) return Qfalse;
     load = check_autoload_required(mod, id, &loading);
@@ -2161,39 +2106,30 @@ rb_autoload_load(VALUE mod, ID id)
     src = rb_sourcefile();
     if (src && loading && strcmp(src, loading) == 0) return Qfalse;
 
-    /* set ele->state for a marker of autoloading thread */
+    /* set ele->thread for a marker of autoloading thread */
     if (!(ele = check_autoload_data(load))) {
 	return Qfalse;
     }
-
-    state.ele = ele;
-    state.mod = mod;
-    state.id = id;
-    state.thread = rb_thread_current();
-    if (!ele->state) {
-	ele->state = &state;
-
-	/*
-	 * autoload_reset will wake up any threads added to this
-	 * iff the GVL is released during autoload_require
-	 */
-	list_head_init(&state.waitq.head);
+    if (ele->thread == Qnil) {
+	ele->thread = rb_thread_current();
     }
-    else {
-	list_add_tail(&ele->state->waitq.head, &state.waitq.node);
-	/*
-	 * autoload_reset in other thread will resume us and remove us
-	 * from the waitq list
-	 */
-	do {
-	    rb_thread_sleep_deadly();
-	} while (state.thread != Qfalse);
-    }
-
     /* autoload_data_i can be deleted by another thread while require */
-    result = rb_ensure(autoload_require, (VALUE)&state,
-		       autoload_reset, (VALUE)&state);
+    result = rb_ensure(autoload_require, (VALUE)ele, autoload_reset, (VALUE)ele);
 
+    if (RTEST(result)) {
+	/* At the last, move a value defined in autoload to constant table */
+	if (ele->value != Qundef) {
+	    int safe_backup;
+	    struct autoload_const_set_args args;
+	    args.mod = mod;
+	    args.id = id;
+	    args.value = ele->value;
+	    safe_backup = rb_safe_level();
+	    rb_set_safe_level_force(ele->safe_level);
+	    rb_ensure(autoload_const_set, (VALUE)&args,
+		      reset_safe, (VALUE)safe_backup);
+	}
+    }
     RB_GC_GUARD(load);
     return result;
 }
@@ -2594,8 +2530,8 @@ const_update(st_data_t *key, st_data_t *value, st_data_t arg, int existing)
 
 		load = autoload_data(klass, id);
 		/* for autoloading thread, keep the defined value to autoloading storage */
-		if (load && (ele = check_autoload_data(load)) && ele->state &&
-			    (ele->state->thread == rb_thread_current())) {
+		if (load && (ele = check_autoload_data(load)) &&
+			    (ele->thread == rb_thread_current())) {
 		    rb_clear_constant_cache();
 
 		    ele->value = val; /* autoload_i is non-WB-protected */
-- 
EW