From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS60781 185.101.107.0/24 X-Spam-Status: No, score=-1.7 required=3.0 tests=AWL,BAYES_00,RCVD_IN_XBL, RDNS_NONE,URIBL_BLOCKED shortcircuit=no autolearn=no version=3.3.2 X-Original-To: spew@80x24.org Received: from 80x24.org (unknown [185.101.107.136]) by dcvr.yhbt.net (Postfix) with ESMTP id 109071F45E for ; Thu, 5 Nov 2015 21:24:11 +0000 (UTC) From: Eric Wong To: spew@80x24.org Subject: [PATCH] variable.c: remove custom locking for autoload Date: Thu, 5 Nov 2015 21:24:09 +0000 Message-Id: <20151105212409.23859-1-e@80x24.org> List-Id: 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