From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 14CAB6C7D4; Wed, 3 Feb 2021 01:15:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 14CAB6C7D4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1612304144; bh=pUAPqS8SwBD3NQ+vpRLfIhgy4EG6Mb7xI8l+02w2Aow=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=fGMa0F8gbiWuhYXLt71h5hzhGkCkaPobiSaV7E6fOC71w4/ZIJV+LNZyGLskDCFWe 84BSYItB0jIRHHGVF4HQsWQAdqaYGcxTlwyU47e6I38G0y3Zkq+j5DwBsivuf2wVGN zQXLs+LTHOBeoBWiPNQY2VQnGl4ktHOmv5Yo3hNc= Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2823882060 for ; Wed, 3 Feb 2021 01:13:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2823882060 Received: by mail-lf1-f41.google.com with SMTP id e15so10197964lft.13 for ; Tue, 02 Feb 2021 14:13:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=1XTFtw7GsWt673Dk7sE2PxW8WEhFHGXr2bYj9mIclw4=; b=cHPDdjKEyHc99xiFdjAPZ/tlc2S6Dt1wI89UkBMLV8QtnJoHdNeM60yjN91J0gLawT wOQmvdYrxCdIPZR2HEwPwcqITOB83bRSu9IJrwK6kI8l1dB47n+iyL760UH3Z2kOwdEI Pr5JaJiU4Z/uHSQ+ttxfhy3C/tY8abVdCnt21Qo/g4cnjIYUMP7EGOk7M7UarHKM2LUL ukb3Fx464W10+R6pdSDyxXo0SkUIyZUsbi83S3ZyQduQgsUd6As0rkL11ad+skSOlEl6 l1QLwlT76CEfMS5DlGiontTmM2+usQWxNd3OA1649319w1Cg+j1FL/jV5lo7zkCTcbKB HEhg== X-Gm-Message-State: AOAM533UdwI1b/QSCAtnyzlXCBTZaWGMsVJGYe/or/8Q2q8TES5+rc6h a01eKRoX33/Hx7L4GAvgN7DcDkQB9e4= X-Google-Smtp-Source: ABdhPJwA63eCnsKHIpIc8AzPPU0WQQoZPCk4wssWJzoyt5S6V1+PNkhDAUSmdZjOa3eijTCFpY1+1w== X-Received: by 2002:a19:5056:: with SMTP id z22mr47718lfj.226.1612304016817; Tue, 02 Feb 2021 14:13:36 -0800 (PST) Received: from grain.localdomain ([5.18.103.226]) by smtp.gmail.com with ESMTPSA id d18sm28011lfm.285.2021.02.02.14.13.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Feb 2021 14:13:35 -0800 (PST) Received: by grain.localdomain (Postfix, from userid 1000) id 93DED5602A0; Wed, 3 Feb 2021 01:12:08 +0300 (MSK) To: tml Date: Wed, 3 Feb 2021 01:12:02 +0300 Message-Id: <20210202221207.383101-8-gorcunov@gmail.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20210202221207.383101-1-gorcunov@gmail.com> References: <20210202221207.383101-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v14 07/12] module_cache: use references as a main usage counter X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" We use module:funcs_list as implicit reference counter, lets get rid of it and use explicit counting with module_ref/unref help. An interesting moment is module reload: we can't drop old instance from hash because hash now points to a new module. Part-of #4642 Suggested-by: Vladislav Shpilevoy Signed-off-by: Cyrill Gorcunov --- src/box/module_cache.c | 137 ++++++++++++++++++++++++++--------------- 1 file changed, 87 insertions(+), 50 deletions(-) diff --git a/src/box/module_cache.c b/src/box/module_cache.c index 4c64a6dc5..bb6ef39cd 100644 --- a/src/box/module_cache.c +++ b/src/box/module_cache.c @@ -129,6 +129,24 @@ module_cache_del(const char *name, const char *name_end) mh_strnptr_del(mod_hash, e, NULL); } +/** + * Mark module as out of cache. + */ +static void +module_set_orphan(struct module *module) +{ + module->package[0] = '\0'; +} + +/** + * Test if module is out of cache. + */ +static bool +module_is_orphan(struct module *module) +{ + return module->package[0] == '\0'; +} + /** * Arguments for luaT_module_find used by lua_cpcall(). */ @@ -206,6 +224,47 @@ module_find(const char *package, const char *package_end, return 0; } +/** + * Delete a module. + */ +static void +module_delete(struct module *module) +{ + struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT); + if (e != NULL) + --e->iparam; + dlclose(module->handle); + TRASH(module); + free(module); +} + +/** + * Increase reference to a module. + */ +static void +module_ref(struct module *module) +{ + assert(module->refs >= 0); + module->refs++; +} + +/** + * Decrease module reference and delete it if last one. + */ +static void +module_unref(struct module *module) +{ + assert(module->refs > 0); + if (module->refs-- == 1) { + if (!module_is_orphan(module)) { + size_t len = strlen(module->package); + module_cache_del(module->package, + &module->package[len]); + } + module_delete(module); + } +} + /** * Load dynamic shared object, ie module library. * @@ -301,6 +360,7 @@ module_load(const char *package, const char *package_end) struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT); if (e != NULL) ++e->iparam; + module_ref(module); return module; error: @@ -308,30 +368,6 @@ module_load(const char *package, const char *package_end) return NULL; } -/** - * Delete a module. - */ -static void -module_delete(struct module *module) -{ - struct errinj *e = errinj(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT); - if (e != NULL) - --e->iparam; - dlclose(module->handle); - TRASH(module); - free(module); -} - -/** - * Check if a module is unused and delete it then. - */ -static void -module_gc(struct module *module) -{ - if (rlist_empty(&module->funcs_list) && module->refs == 0) - module_delete(module); -} - /** * Import a function from a module. */ @@ -366,28 +402,17 @@ module_sym_load(struct module_sym *mod_sym) if (module == NULL) return -1; if (module_cache_add(module) != 0) { - module_delete(module); + module_unref(module); return -1; } } else { + module_ref(cached); module = cached; } mod_sym->addr = module_sym(module, name.sym); if (mod_sym->addr == NULL) { - if (cached == NULL) { - /* - * In case if it was a first load we should - * clean the cache immediately otherwise - * the module continue being referenced even - * if there will be no use of it. - * - * Note the module_sym set an error thus be - * careful to not wipe it. - */ - module_cache_del(name.package, name.package_end); - module_delete(module); - } + module_unref(module); return -1; } @@ -403,12 +428,11 @@ module_sym_unload(struct module_sym *mod_sym) return; rlist_del(&mod_sym->item); - if (rlist_empty(&mod_sym->module->funcs_list)) { - struct func_name name; - func_split_name(mod_sym->name, &name); - module_cache_del(name.package, name.package_end); - } - module_gc(mod_sym->module); + /* + * Unref action might delete module + * so call it after rlist_del. + */ + module_unref(mod_sym->module); mod_sym->module = NULL; mod_sym->addr = NULL; @@ -454,10 +478,9 @@ module_sym_call(struct module_sym *mod_sym, struct port *args, */ struct module *module = mod_sym->module; assert(module != NULL); - ++module->refs; + module_ref(module); int rc = mod_sym->addr(&ctx, data, data + data_sz); - --module->refs; - module_gc(module); + module_unref(module); region_truncate(region, region_svp); if (rc != 0) { @@ -485,6 +508,9 @@ module_reload(const char *package, const char *package_end) if (new == NULL) return -1; + /* Extra ref until cache get updated */ + module_ref(old); + struct module_sym *mod_sym, *tmp; rlist_foreach_entry_safe(mod_sym, &old->funcs_list, item, tmp) { struct func_name name; @@ -499,6 +525,10 @@ module_reload(const char *package, const char *package_end) mod_sym->module = new; rlist_move(&new->funcs_list, &mod_sym->item); + + /* Move reference to a new place */ + module_ref(new); + module_unref(old); } if (module_cache_update(package, package_end, new) != 0) { @@ -511,10 +541,15 @@ module_reload(const char *package, const char *package_end) panic("module: can't update module cache (%s)", package); } - module_gc(old); + module_set_orphan(old); + module_unref(old); + + /* From explicit load above */ + module_unref(new); return 0; restore: + module_set_orphan(new); /* * Some old-dso func can't be load from new module, * restore old functions. @@ -533,11 +568,13 @@ module_reload(const char *package, const char *package_end) } mod_sym->module = old; rlist_move(&old->funcs_list, &mod_sym->item); + module_ref(old); + module_unref(new); } while (mod_sym != rlist_first_entry(&old->funcs_list, struct module_sym, item)); assert(rlist_empty(&new->funcs_list)); - module_delete(new); + module_unref(new); return -1; } @@ -559,7 +596,7 @@ module_free(void) while (mh_size(mod_hash) > 0) { mh_int_t i = mh_first(mod_hash); struct module *m = mh_strnptr_node(mod_hash, i)->val; - module_gc(m); + module_unref(m); } mh_strnptr_delete(mod_hash); mod_hash = NULL; -- 2.29.2