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 761386EC58; Wed, 4 Aug 2021 19:15:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 761386EC58 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628093702; bh=KpD5plRTM/NHt/hJjtwxz7l8am2w0XIv5vflOoElt60=; 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=TkN+Jbd7pVNqnPeBtmi4jaHlktdSdR5z/biwjTkTglaRO/MF+cbkTzovGNEIBWobX 0GCgm0MqaxnDQOr433NpdPtJt5YJZmdCVsAANm9hL5D/TMFwsTFf3jx0oqJVaGphql PunrN/B4zWMjj9qBkhern5nDPv9idcqE0murkXHQ= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 7BCED6EC58 for ; Wed, 4 Aug 2021 19:15:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7BCED6EC58 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mBJXn-0007oW-OU; Wed, 04 Aug 2021 19:15:00 +0300 Date: Wed, 4 Aug 2021 19:14:58 +0300 To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210804161458.ma42lpoh3lh5x3fi@esperanza> References: <903f6291-bab5-985e-f922-042453e4d146@tarantool.org> <20210804123001.3cwudybvkw6dds74@esperanza> <20210804153526.gr2uyri5dg52zw4b@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210804153526.gr2uyri5dg52zw4b@esperanza> X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD941C43E597735A9C351B198F4576AC7B20CA14D9DFB46B94A182A05F538085040B4D19DF1E8C99B7AD6D876BB33FE9D491313167CEFB9E71E322E58FEBD8DB122 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72E4E5201E1C2E308EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC18FED211962C318638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D827F586F6CD92B021791030A38B0EF02C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B65D56369A3576CBA5089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5E45637E152A07416FF39909C40A13E528EE2A6D0CB8543A8D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7501A9DF589746230F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34A63B03BCD35E0C0A99981491B17D1A91CCB60799DB5B9DA64E218622DFF11245AE1EE49809DF8A1B1D7E09C32AA3244C1409A499A92E6AD47985AD6E0E867672408A6A02710B7304FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojeDyvyeZJDJHi7DLw66LOsQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D174B6E487E4E232073ABFEA03B346C00274CEFED1673C562683ABF942079399BFB559BB5D741EB966A65DFF43FF7BE03240331F90058701C67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 15/20] net.box: rewrite request implementation in C 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: Vladimir Davydov via Tarantool-patches Reply-To: Vladimir Davydov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Wed, Aug 04, 2021 at 06:35:26PM +0300, Vladimir Davydov wrote: > On Wed, Aug 04, 2021 at 03:30:01PM +0300, Vladimir Davydov wrote: > > On Mon, Aug 02, 2021 at 11:54:43PM +0200, Vladislav Shpilevoy wrote: > > > > + lua_pushcclosure(L, luaT_netbox_request_iterator_next, 2); > > > > > > 8. Push of cfunctions, especially closures, on a regular basis might > > > be expensive. Could you please try to make it a non-closure function > > > and cache its reference like I showed in the proposal about request __gc? > > > > My test doesn't check performance of the request iterator so I need to > > come up with a new test first to check this hypothesis. I'll reply to > > this email with the results once they're ready. > > Using getref instead of pushcclosure does improve performance of > the final version by about 10%: > > KRPS (WALL TIME) KRPS (PROC TIME) > pushcclosure : 233 +- 3 : 344 +- 7 > getref : 259 +- 4 : 404 +- 10 > > The test is here: > > https://gist.github.com/locker/9df4eaaf1bc889b16706640711c946f7 > > Amended the patch, thanks. The incremental diff is below: > -- > diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c > index 7ef40792320e..fadd71a6813b 100644 > --- a/src/box/lua/net_box.c > +++ b/src/box/lua/net_box.c > @@ -131,11 +131,20 @@ struct netbox_request { > * the response hasn't been received yet. > */ > struct error *error; > + /** Timeout for luaT_netbox_request_iterator_next(). */ > + double iterator_timeout; > }; It's totally wrong to store iterator_timeout in netbox_request, because there may be more than one iterator open for the same request. Instead we should push a table with {request, timeout}, just like the Lua implementation did. This is a bit slower, but still faster than using pushcclosure: KRPS (WALL TIME) KRPS (PROC TIME) pushcclosure : 233 +- 3 : 344 +- 7 getref : 259 +- 4 : 404 +- 10 getref + table : 257 +- 7 : 397 +- 15 The new incremental diff is below: -- diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c index de9c11736603..45b0c7f3bac3 100644 --- a/src/box/lua/net_box.c +++ b/src/box/lua/net_box.c @@ -136,6 +136,13 @@ struct netbox_request { static const char netbox_registry_typename[] = "net.box.registry"; static const char netbox_request_typename[] = "net.box.request"; +/** + * Instead of pushing luaT_netbox_request_iterator_next with lua_pushcclosure + * in luaT_netbox_request_pairs, we get it by reference, because it works + * faster. + */ +static int luaT_netbox_request_iterator_next_ref; + static void netbox_request_destroy(struct netbox_request *request) { @@ -1488,8 +1495,8 @@ luaT_netbox_request_discard(struct lua_State *L) /** * Gets the next message or the final result. Takes the index of the last - * returned message as a second argument (the first argument is ignored). - * The request and timeout are passed as up-values (see request.pairs()). + * returned message as a second argument. The request and timeout are passed in + * the first argument as a table (see request.pairs()). * * On success returns the index of the current message (used by the iterator * implementation to continue iteration) and an object, which is either the @@ -1504,9 +1511,12 @@ luaT_netbox_request_discard(struct lua_State *L) static int luaT_netbox_request_iterator_next(struct lua_State *L) { - struct netbox_request *request = luaT_check_netbox_request( - L, lua_upvalueindex(1)); - double timeout = lua_tonumber(L, lua_upvalueindex(2)); + /* The first argument is a table: {request, timeout}. */ + lua_rawgeti(L, 1, 1); + struct netbox_request *request = luaT_check_netbox_request(L, -1); + lua_rawgeti(L, 1, 2); + double timeout = lua_tonumber(L, -1); + /* The second argument is the index of the last returned message. */ if (luaL_isnull(L, 2)) { /* The previous call returned an error. */ goto stop; @@ -1582,8 +1592,17 @@ luaT_netbox_request_pairs(struct lua_State *L) lua_pop(L, 1); lua_pushnumber(L, TIMEOUT_INFINITY); } - lua_pushcclosure(L, luaT_netbox_request_iterator_next, 2); - lua_pushnil(L); + lua_settop(L, 2); + /* Create a table passed to next(): {request, timeout}. */ + lua_createtable(L, 2, 0); + lua_insert(L, 1); + lua_rawseti(L, 1, 2); /* timeout */ + lua_rawseti(L, 1, 1); /* request */ + /* Push the next() function. It must go first. */ + lua_rawgeti(L, LUA_REGISTRYINDEX, + luaT_netbox_request_iterator_next_ref); + lua_insert(L, 1); + /* Push the iterator index. */ lua_pushinteger(L, 0); return 3; } @@ -1749,6 +1768,9 @@ netbox_dispatch_response_console(struct lua_State *L) int luaopen_net_box(struct lua_State *L) { + lua_pushcfunction(L, luaT_netbox_request_iterator_next); + luaT_netbox_request_iterator_next_ref = luaL_ref(L, LUA_REGISTRYINDEX); + static const struct luaL_Reg netbox_registry_meta[] = { { "__gc", luaT_netbox_registry_gc }, { "reset", luaT_netbox_registry_reset },