On Thu, Jan 09, 2020 at 05:30:32PM -0800, Sadasiva Gujjarlapudi wrote:
> After applying Willy's patch
> 
> requests/sec 7181.9869
> $time haproxy
>   real 0m10.304s
>   user 0m1.112s
>   sys 0m1.184s
> 
> After applying Willy's patch and the following patch
> 
> @@ -955,10 +956,14 @@ void hlua_ctx_destroy(struct hlua *lua)
>          * the garbage collection.
>          */
>         if (lua->flags & HLUA_MUST_GC) {
> -               if (!SET_SAFE_LJMP(gL.T))
> -                       return;
> -               lua_gc(gL.T, LUA_GCCOLLECT, 0);
> -               RESET_SAFE_LJMP(gL.T);
> +               if (lua->gc_tune_count++ > 1000) {
> +                       lua->gc_tune_count = 0;
> +                   if (!SET_SAFE_LJMP(gL.T))
> +                           return;
> +                       lua_gc(gL.T, LUA_GCCOLLECT, 0);
> +                   RESET_SAFE_LJMP(gL.T);
> +               }
> +
>         }
> 
> requests/sec 8635.0402
>   real 0m10.969s
>   user 0m0.744s
>   sys 0m1.116s

OK this is promising, thanks for the tests!

> Most of the time the CPU is the bottleneck and it may be ok to consume
> little bit more memory on dedicated servers running haproxy.
> Our use case/Lua application required to use as little CPU as possible.

Of course, but according to Thierry the problem is not so much about
memory, but rather about connections/file descriptors.

Well, I'm thinking about something. Since this HLUA_MUST_GC flag is used
only for connections, we could probably proceed differently. Instead of
a flag we could have a connection counter, which gets incremented for
each connection we create and decremented for each closed connection.
Then we'd just run the GC in hlua_destroy_ctx() if the connection count
indicates we did not kill all of them. This way, the GC will never run
for normal situations where connections were cleanly closed, and it will
only be run if absolutely needed.

I can propose you to give a try to the attached patch. I have no idea if
it works or does something correct. It's only build-tested.

Willy
diff --git a/include/types/hlua.h b/include/types/hlua.h
index fb1796c..b541e15 100644
--- a/include/types/hlua.h
+++ b/include/types/hlua.h
@@ -37,7 +37,7 @@ struct stream;
 #define HLUA_WAKERESWR 0x00000004
 #define HLUA_WAKEREQWR 0x00000008
 #define HLUA_EXIT      0x00000010
-#define HLUA_MUST_GC   0x00000020
+/* unused:   0x00000020 */
 #define HLUA_STOP      0x00000040
 
 #define HLUA_F_AS_STRING    0x01
@@ -93,6 +93,7 @@ struct hlua {
        struct list com; /* The list head of the signals attached to this task. 
*/
        struct ebpt_node node;
        struct hlua_consistency cons; /* Store data consistency check. */
+       int conn_count;  /* outgoing connections count */
 };
 
 /* This is a part of the list containing references to functions
diff --git a/src/hlua.c b/src/hlua.c
index 37f7866..b60e8ba 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -905,6 +905,7 @@ int hlua_ctx_init(struct hlua *lua, struct task *task, int 
already_safe)
        }
        lua->Mref = LUA_REFNIL;
        lua->flags = 0;
+       lua->conn_count = 0;
        LIST_INIT(&lua->com);
        lua->T = lua_newthread(gL.T);
        if (!lua->T) {
@@ -954,7 +955,7 @@ void hlua_ctx_destroy(struct hlua *lua)
         * NOTE: maybe this action locks all the Lua threads untiml the en of
         * the garbage collection.
         */
-       if (lua->flags & HLUA_MUST_GC) {
+       if (lua->conn_count) {
                if (!SET_SAFE_LJMP(gL.T))
                        return;
                lua_gc(gL.T, LUA_GCCOLLECT, 0);
@@ -1194,11 +1195,6 @@ resume_execution:
                break;
        }
 
-       /* This GC permits to destroy some object when a Lua timeout strikes. */
-       if (lua->flags & HLUA_MUST_GC &&
-           ret != HLUA_E_AGAIN)
-               lua_gc(lua->T, LUA_GCCOLLECT, 0);
-
        switch (ret) {
        case HLUA_E_AGAIN:
                break;
@@ -1699,6 +1695,7 @@ __LJMP static int hlua_socket_close_helper(lua_State *L)
        struct hlua_socket *socket;
        struct appctx *appctx;
        struct xref *peer;
+       struct hlua *hlua = hlua_gethlua(L);
 
        socket = MAY_LJMP(hlua_checksocket(L, 1));
 
@@ -1711,6 +1708,8 @@ __LJMP static int hlua_socket_close_helper(lua_State *L)
        peer = xref_get_peer_and_lock(&socket->xref);
        if (!peer)
                return 0;
+
+       hlua->conn_count--;
        appctx = container_of(peer, struct appctx, ctx.hlua_cosocket.xref);
 
        /* Set the flag which destroy the session. */
@@ -2463,7 +2462,7 @@ __LJMP static int hlua_socket_connect(struct lua_State *L)
        si_rx_endp_more(&s->si[0]);
        appctx_wakeup(appctx);
 
-       hlua->flags |= HLUA_MUST_GC;
+       hlua->conn_count++;
 
        if (!notification_new(&hlua->com, 
&appctx->ctx.hlua_cosocket.wake_on_write, hlua->task)) {
                xref_unlock(&socket->xref, peer);

Reply via email to