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 A3B447030E; Mon, 25 Jan 2021 11:52:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A3B447030E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1611564744; bh=jFbWwwKbfKfUc6TCSsLjnNSgaH1zyHzivXvB9p0hnEw=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=jtLG9MlLzU4OAzjXy4/zTtTMAO+xZ5TGUMrXdhI4/sPyV07U/TGHBLcllfvcwo97N 1dyR+XCuNe8FU+xsqpOwBxVRVklB/fYCRSv49PQGh3HGNmYBubVCjoycYyvU184goW FuShiF6D3EtDmD5TTjXZgaFofHx1tSRdrm7Gff6c= Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) (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 849457030E for ; Mon, 25 Jan 2021 11:52:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 849457030E Received: by mail-lf1-f50.google.com with SMTP id q8so16515118lfm.10 for ; Mon, 25 Jan 2021 00:52:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=yW+gU6UBy359Xs0qLMfKK11sOujXbWozTUCZsau5YAo=; b=SuPjW3QjFAqlWUjkfrjYK9SZNceAJQtqDb5D6/m74Zx6sN2E3PXwcpjPs1joLZbcZW KS/G8iEzNSeyLmPsXi2Y0eqB1V6RP4qPMxjI1adU8n68ubaVeEe7JT66Kyn//v6+Dipf paDG2HsU+IYinQl4PVw7vNz6IcZrdFRmL3IN+Xbgq6hBXMeV8woi+PZnMfsko4cFZLso Px+dcnVSTTYprpZMFUISd3i/mPevkdp/AdwbEeP/z9MqmB3sZH7BfUsNW8rU1RAqOoRl EN/3NA9KoS73cXaPA6DvHLu5e4MUSFKNNCLrQGnSzvYRR+MFf0RQDqfjrewOkeTAoXts yXdQ== X-Gm-Message-State: AOAM5309tPt2d5W6Te7KnoE/NjTEPw1kIlOy104ncxdCqpkC1YG2s++E Nvt5GCh7C39RBSb9Fv7VeiuDkAx1lMs= X-Google-Smtp-Source: ABdhPJwwZZv8p//shsiFniP1ybzN/Uu4+k76qLAHGdVXjxZgCaGCfweVtS31cLQ2YxhYUqYkFgL+3g== X-Received: by 2002:ac2:5d6c:: with SMTP id h12mr2802740lft.482.1611564741404; Mon, 25 Jan 2021 00:52:21 -0800 (PST) Received: from grain.localdomain ([5.18.91.94]) by smtp.gmail.com with ESMTPSA id l4sm734911lfh.96.2021.01.25.00.52.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Jan 2021 00:52:19 -0800 (PST) Received: by grain.localdomain (Postfix, from userid 1000) id E1CA8560113; Mon, 25 Jan 2021 11:52:18 +0300 (MSK) Date: Mon, 25 Jan 2021 11:52:18 +0300 To: Vladislav Shpilevoy Cc: tml Message-ID: <20210125085218.GE2174@grain> References: <20210118203556.281700-1-gorcunov@gmail.com> <20210118203556.281700-3-gorcunov@gmail.com> <136716fc-7e19-6515-d18c-c0d3d0576a96@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <136716fc-7e19-6515-d18c-c0d3d0576a96@tarantool.org> User-Agent: Mutt/1.14.6 (2020-07-11) Subject: Re: [Tarantool-patches] [PATCH v12 2/8] module_cache: move module handling into own subsystem 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 Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Sun, Jan 24, 2021 at 05:26:48PM +0100, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 4 comments below. > > On 18.01.2021 21:35, Cyrill Gorcunov wrote: > > The module handling should not be bound to particular > > function implementation (we will have two users: already > > existing functions for "_func" space, and a new upcoming > > one which will be serving cbox submodule in next patch). > > 1. It is now cmod, not cbox. Thanks! > > + > > +#include "box/error.h" > > 2. #include "error.h" and #include "box/error.h" are the same > files. Because you are in box/ folder. OK > > + > > + mod_sym->addr = module_sym(module, name.sym); > > + if (mod_sym->addr == NULL) > > + return -1; > > 3. Is it correct, that you second time has deleted the bugfix > which you did about module not being unloaded when first > symbol load fails? Sigh... This sneaky issue triggers for n'th time :( Thanks for catching, Vlad! > > +#pragma once > > + > > +#include > > 4. Please, use "" instead of <> for non-system headers. OK, an update on top of the patch. I'll squash it later --- src/box/module_cache.c | 30 ++++++++++++++++++++++++++---- src/box/module_cache.h | 2 +- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/box/module_cache.c b/src/box/module_cache.c index 9fe316807..a66b84efb 100644 --- a/src/box/module_cache.c +++ b/src/box/module_cache.c @@ -16,7 +16,7 @@ #include "fiber.h" #include "port.h" -#include "box/error.h" +#include "error.h" #include "lua/utils.h" #include "libeio/eio.h" @@ -338,8 +338,14 @@ module_sym_load(struct module_sym *mod_sym) struct func_name name; func_split_name(mod_sym->name, &name); - struct module *module = module_cache_find(name.package, name.package_end); - if (module == NULL) { + /* + * In case if module has been loaded already by + * some previous call we can eliminate redundant + * loading and take it from the cache. + */ + struct module *cached, *module; + cached = module_cache_find(name.package, name.package_end); + if (cached == NULL) { module = module_load(name.package, name.package_end); if (module == NULL) return -1; @@ -347,11 +353,27 @@ module_sym_load(struct module_sym *mod_sym) module_delete(module); return -1; } + } else { + module = cached; } mod_sym->addr = module_sym(module, name.sym); - if (mod_sym->addr == NULL) + 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); + } return -1; + } mod_sym->module = module; rlist_add(&module->funcs_list, &mod_sym->item); diff --git a/src/box/module_cache.h b/src/box/module_cache.h index fd789f603..0f5d2b64a 100644 --- a/src/box/module_cache.h +++ b/src/box/module_cache.h @@ -6,7 +6,7 @@ #pragma once -#include +#include "small/rlist.h" #if defined(__cplusplus) extern "C" { -- 2.29.2