← Commit list

eab7d91ade

Max Dymond
Max Dymond
2026-09-04
SUCCESS Duration: 393s

Subject

multi: reduce struct size with small cleanups

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

docs/internals/UINT_SETS.md +1 -1lib/api.c +11 -4lib/api.h +4 -4lib/multi.c +4 -5lib/multihandle.h +1 -1lib/multiif.h +3 -4lib/uint-table.c +1 -5lib/uint-table.h +3 -10tests/data/Makefile.am +1 -0tests/data/test3230 +19 -0tests/unit/Makefile.inc +1 -0tests/unit/unit3212.c +1 -1tests/unit/unit3230.c +140 -0
docs/internals/UINT_SETS.md+1 -1
@@ -51,7 +51,7 @@ This iteration has the following properties:
### Memory
For storing 1000 entries, the table would allocate one block of 8KB on
-a 64-bit system, plus the 2 pointers and 3 `uint32_t` in its base `struct
+a 64-bit system, plus the pointer and 3 `uint32_t` in its base `struct
uint32_tbl`. A resize allocates a completely new pointer array, copy
the existing entries and free the previous one.
lib/api.c+11 -4
@@ -29,6 +29,13 @@
#include "multiif.h"
#include "vtls/vtls_scache.h"
+typedef char mapi_fn_ids_fit_uint8[
+ (CURL_MAPI_FN_LAST <= CURL_CBAPI_FN_START) ? 1 : -1];
+typedef char cbapi_fn_ids_fit_uint8[
+ ((CURL_CBAPI_FN_LAST - 1) <= UINT8_MAX) ? 1 : -1];
+typedef char mapi_depth_fits_uint8[
+ (CURL_MAPI_MAX_RECURSION <= UINT8_MAX) ? 1 : -1];
+
struct Curl_eapi_fn_props {
Curl_eapi_fn fn;
uint8_t data_is_killed; /* easy handle is killed in call */
@@ -135,7 +142,7 @@ static bool eapi_in_event_cb(struct Curl_easy *data)
size_t i;
for(i = 0; i < multi->callstack.count; ++i) {
if(multi->callstack.calls[i] >= CURL_CBAPI_FN_START) {
- uint16_t fn = multi->callstack.calls[i];
+ uint8_t fn = multi->callstack.calls[i];
if((fn < CURL_CBAPI_FN_LAST) &&
cbapi_fn_props[fn - CURL_CBAPI_FN_START].is_event_cb)
return TRUE;
@@ -376,7 +383,7 @@ bool Curl_mapi_enter(struct Curl_mapi_guard *guard,
mresult = CURLM_RECURSIVE_API_CALL;
goto out;
}
- multi->callstack.calls[multi->callstack.count] = (uint16_t)fn;
+ multi->callstack.calls[multi->callstack.count] = (uint8_t)fn;
++multi->callstack.count;
guard->depth = multi->callstack.count;
guard->multi = fn_props->multi_is_killed ? NULL : multi;
@@ -400,7 +407,7 @@ void Curl_mapi_leave(struct Curl_mapi_guard *guard)
DEBUGASSERT(0); /* someone forgot to clean up */
}
/* reset to depth the guard was entered in */
- guard->multi->callstack.count = (uint16_t)(guard->depth - 1);
+ guard->multi->callstack.count = (uint8_t)(guard->depth - 1);
}
}
}
@@ -439,7 +446,7 @@ void Curl_cbapi_enter(struct Curl_mapi_guard *guard,
/* if multi callstack already at max depth, leave */
if(multi->callstack.count >= CURL_MAPI_MAX_RECURSION)
return;
- multi->callstack.calls[multi->callstack.count] = (uint16_t)fn;
+ multi->callstack.calls[multi->callstack.count] = (uint8_t)fn;
++multi->callstack.count;
guard->depth = multi->callstack.count;
guard->multi = multi;
lib/api.h+4 -4
@@ -99,7 +99,7 @@ typedef enum {
CURL_MAPI_FN_LAST
} Curl_mapi_fn;
-#define CURL_CBAPI_FN_START (16 * 1024)
+#define CURL_CBAPI_FN_START 128
/* the callback functions */
typedef enum {
@@ -169,13 +169,13 @@ CURLHcode Curl_eapi_hcode(CURLcode result);
#define CURL_MAPI_MAX_RECURSION 15
struct Curl_mapi_stack {
- uint16_t count;
- uint16_t calls[CURL_MAPI_MAX_RECURSION];
+ uint8_t count;
+ uint8_t calls[CURL_MAPI_MAX_RECURSION];
};
struct Curl_mapi_guard {
struct Curl_multi *multi; /* != NULL if handle stays */
- uint16_t depth; /* > 0 if this guard was entered */
+ uint8_t depth; /* > 0 if this guard was entered */
};
bool Curl_mapi_enter(struct Curl_mapi_guard *guard,
lib/multi.c+4 -5
@@ -254,7 +254,7 @@ struct Curl_multi *Curl_multi_handle(uint32_t xfer_table_size,
Curl_dnscache_init(&multi->dnscache, dnssize);
Curl_mntfy_init(multi);
Curl_multi_ev_init(multi, ev_hashsize);
- Curl_uint32_tbl_init(&multi->xfers, NULL);
+ Curl_uint32_tbl_init(&multi->xfers);
Curl_uint32_bset_init(&multi->process);
Curl_uint32_bset_init(&multi->dirty);
Curl_uint32_bset_init(&multi->pending);
@@ -264,7 +264,6 @@ struct Curl_multi *Curl_multi_handle(uint32_t xfer_table_size,
multi->multiplexing = TRUE;
multi->max_concurrent_streams = 100;
- multi->last_timeout_ms = -1;
#ifdef ENABLE_WAKEUP
multi->wakeup_pair[0] = CURL_SOCKET_BAD;
multi->wakeup_pair[1] = CURL_SOCKET_BAD;
@@ -3575,7 +3574,7 @@ CURLMcode Curl_update_timer(struct Curl_multi *multi)
return CURLM_OK;
multi_timeout(multi, &timeouts_offset_us, &timeout_ms);
- if(timeout_ms < 0 && multi->last_timeout_ms < 0) {
+ if(timeout_ms < 0 && !multi->last_timeout_set) {
/* nothing to do */
}
else if(timeout_ms < 0) {
@@ -3584,7 +3583,7 @@ CURLMcode Curl_update_timer(struct Curl_multi *multi)
timeout_ms = -1; /* normalize */
set_value = TRUE;
}
- else if(multi->last_timeout_ms < 0) {
+ else if(!multi->last_timeout_set) {
CURL_TRC_M(multi->admin, "[TIMER] set %dms, none before", timeout_ms);
set_value = TRUE;
}
@@ -3606,7 +3605,7 @@ CURLMcode Curl_update_timer(struct Curl_multi *multi)
struct Curl_mapi_guard guard;
multi->last_expire_offset_us = timeouts_offset_us;
- multi->last_timeout_ms = timeout_ms;
+ multi->last_timeout_set = timeout_ms >= 0;
CURL_CBAPI_MULTI_START(&guard, multi, multi_timer_cb);
rc = multi->timer_cb(multi, timeout_ms, multi->timer_userp);
CURL_CBAPI_MULTI_END(&guard);
lib/multihandle.h+1 -1
@@ -158,7 +158,6 @@ struct Curl_multi {
/* timer callback and user data pointer for the *socket() API */
curl_multi_timer_callback timer_cb;
void *timer_userp;
- timediff_t last_timeout_ms; /* the last timeout value set via timer_cb */
#ifdef USE_WINSOCK
WSAEVENT wsa_event; /* Winsock event used for waits */
@@ -191,6 +190,7 @@ struct Curl_multi {
#endif
BIT(dead); /* a callback returned error, everything needs to crash and
burn */
+ BIT(last_timeout_set); /* last timer callback value was nonnegative */
BIT(xfer_buf_borrowed); /* xfer_buf is currently being borrowed */
BIT(xfer_ulbuf_borrowed); /* xfer_ulbuf is currently being borrowed */
BIT(xfer_sockbuf_borrowed); /* xfer_sockbuf is currently being borrowed */
lib/multiif.h+3 -4
@@ -89,7 +89,7 @@ CURLMcode Curl_multi_pollset(struct Curl_easy *data,
* @param pbuf on return, the buffer to use or NULL on error
* @param pbuflen on return, the size of *pbuf or 0 on error
* @return CURLE_OK when buffer is available and is returned.
- * CURLE_OUT_OF_MEMORy on failure to allocate the buffer,
+ * CURLE_OUT_OF_MEMORY on failure to allocate the buffer,
* CURLE_FAILED_INIT if the easy handle is without multi.
* CURLE_AGAIN if the buffer is borrowed already.
*/
@@ -113,7 +113,7 @@ void Curl_multi_xfer_buf_release(struct Curl_easy *data, char *buf);
* @param pbuf on return, the buffer to use or NULL on error
* @param pbuflen on return, the size of *pbuf or 0 on error
* @return CURLE_OK when buffer is available and is returned.
- * CURLE_OUT_OF_MEMORy on failure to allocate the buffer,
+ * CURLE_OUT_OF_MEMORY on failure to allocate the buffer,
* CURLE_FAILED_INIT if the easy handle is without multi.
* CURLE_AGAIN if the buffer is borrowed already.
*/
@@ -138,8 +138,7 @@ void Curl_multi_xfer_ulbuf_release(struct Curl_easy *data, char *buf);
* @param blen requested length of the buffer
* @param pbuf on return, the buffer to use or NULL on error
* @return CURLE_OK when buffer is available and is returned.
- * CURLE_OUT_OF_MEMORy on failure to allocate the buffer,
- * CURLE_FAILED_INIT if the easy handle is without multi.
+ * CURLE_OUT_OF_MEMORY on failure to allocate the buffer,
* CURLE_AGAIN if the buffer is borrowed already.
*/
CURLcode Curl_multi_xfer_sockbuf_borrow(struct Curl_easy *data,
lib/uint-table.c+1 -5
@@ -29,11 +29,9 @@
#define CURL_UINT32_TBL_MAGIC 0x62757473
#endif
-void Curl_uint32_tbl_init(struct uint32_tbl *tbl,
- Curl_uint32_tbl_entry_dtor *entry_dtor)
+void Curl_uint32_tbl_init(struct uint32_tbl *tbl)
{
memset(tbl, 0, sizeof(*tbl));
- tbl->entry_dtor = entry_dtor;
tbl->last_key_added = UINT32_MAX;
#ifdef DEBUGBUILD
tbl->init = CURL_UINT32_TBL_MAGIC;
@@ -49,8 +47,6 @@ static void uint32_tbl_clear_rows(struct uint32_tbl *tbl,
end = CURLMIN(upto_excluding, tbl->nrows);
for(i = from; i < end; ++i) {
if(tbl->rows[i]) {
- if(tbl->entry_dtor)
- tbl->entry_dtor(i, tbl->rows[i]);
tbl->rows[i] = NULL;
tbl->nentries--;
}
lib/uint-table.h+3 -10
@@ -25,12 +25,8 @@
***************************************************************************/
#include "curl_setup.h"
-/* Destructor for a single table entry */
-typedef void Curl_uint32_tbl_entry_dtor(uint32_t key, void *entry);
-
struct uint32_tbl {
void **rows; /* array of void* holding entries */
- Curl_uint32_tbl_entry_dtor *entry_dtor;
uint32_t nrows; /* length of `rows` array */
uint32_t nentries; /* entries in table */
uint32_t last_key_added; /* UINT_MAX or last key added */
@@ -39,17 +35,14 @@ struct uint32_tbl {
#endif
};
-/* Initialize the table with 0 capacity.
- * The optional `entry_dtor` is called when a table entry is removed,
- * Passing NULL means no action is taken on removal. */
-void Curl_uint32_tbl_init(struct uint32_tbl *tbl,
- Curl_uint32_tbl_entry_dtor *entry_dtor);
+/* Initialize the table with 0 capacity. */
+void Curl_uint32_tbl_init(struct uint32_tbl *tbl);
/* Resize the table to change capacity `nmax`. When `nmax` is reduced,
* all present entries with key equal or larger to `nmax` are removed. */
CURLcode Curl_uint32_tbl_resize(struct uint32_tbl *tbl, uint32_t nrows);
-/* Destroy the table, freeing all entries. */
+/* Destroy the table, freeing its storage. */
void Curl_uint32_tbl_destroy(struct uint32_tbl *tbl);
/* Get the table capacity. */
tests/data/Makefile.am+1 -0
@@ -2081,6 +2081,7 @@ TESTCASES = \
test3227 \
test3228 \
test3229 \
+ test3230 \
\
test3300 \
test3301 \
tests/data/test3230+19 -0
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+unittest
+multi
+</keywords>
+</info>
+
+# Client-side
+<client>
+<features>
+unittest
+</features>
+<name>
+multi timer unit tests
+</name>
+</client>
+</testcase>
tests/unit/Makefile.inc+1 -0
@@ -102,6 +102,7 @@ TESTS_C = \
unit3216.c \
unit3219.c \
unit3227.c \
+ unit3230.c \
unit3300.c \
unit3301.c \
unit3302.c \
tests/unit/unit3212.c+1 -1
@@ -30,7 +30,7 @@
static CURLcode t3212_setup(struct uint32_tbl *tbl)
{
- Curl_uint32_tbl_init(tbl, NULL);
+ Curl_uint32_tbl_init(tbl);
return Curl_uint32_tbl_resize(tbl, TBL_SIZE);
}
tests/unit/unit3230.c+140 -0
@@ -0,0 +1,140 @@
+/***************************************************************************
+ * _ _ ____ _
+ * Project ___| | | | _ \| |
+ * / __| | | | |_) | |
+ * | (__| |_| | _ <| |___
+ * \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <[email protected]>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "unitcheck.h"
+#include "urldata.h"
+#include "multiif.h"
+
+#define T3230_TIMER_CALLS 6
+
+struct t3230_timer_ctx {
+ long timeout_ms[T3230_TIMER_CALLS];
+ size_t count;
+};
+
+static int t3230_timer_cb(CURLM *multi, long timeout_ms, void *userp)
+{
+ struct t3230_timer_ctx *ctx = userp;
+ (void)multi;
+
+ if(ctx->count < CURL_ARRAYSIZE(ctx->timeout_ms))
+ ctx->timeout_ms[ctx->count] = timeout_ms;
+ ++ctx->count;
+ return 0;
+}
+
+static CURLcode t3230_setup(void)
+{
+ CURLcode result = CURLE_OK;
+ global_init(CURL_GLOBAL_ALL);
+ return result;
+}
+
+static void t3230_test_timer_updates(void)
+{
+ CURLM *handle = curl_multi_init();
+ struct Curl_multi *multi = handle;
+ struct t3230_timer_ctx ctx;
+ struct curltime now;
+ timediff_t expire_offset_us;
+ CURLMcode mresult;
+ int running;
+
+ memset(&ctx, 0, sizeof(ctx));
+ if(!multi) {
+ fail("multi handle creation failed");
+ return;
+ }
+
+ mresult = curl_multi_setopt(handle, CURLMOPT_TIMERDATA, &ctx);
+ fail_unless(mresult == CURLM_OK, "setting timer data failed");
+ mresult = curl_multi_setopt(handle, CURLMOPT_TIMERFUNCTION,
+ t3230_timer_cb);
+ fail_unless(mresult == CURLM_OK, "setting timer callback failed");
+
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "initial timer update failed");
+ fail_unless(!ctx.count, "callback invoked without a timeout");
+ fail_unless(!multi->last_timeout_set, "initial timer marked as set");
+
+ now = curlx_now();
+ Curl_expire_set(multi->admin, EXPIRE_TIMEOUT, 600000, &now);
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "setting timer failed");
+ fail_unless(ctx.count == 1, "setting timer did not invoke callback");
+ fail_unless(multi->last_timeout_set, "timer not marked as set");
+
+ Curl_expire_set(multi->admin, EXPIRE_TIMEOUT, 1200000, &now);
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "replacing timer failed");
+ fail_unless(ctx.count == 2, "replacing timer did not invoke callback");
+
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "unchanged timer update failed");
+ fail_unless(ctx.count == 2, "unchanged timer invoked callback");
+
+ expire_offset_us = multi->last_expire_offset_us;
+ mresult = curl_multi_socket_action(handle, CURL_SOCKET_TIMEOUT, 0, &running);
+ fail_unless(mresult == CURLM_OK, "forced timer refresh failed");
+ fail_unless(ctx.count == 3, "timer refresh did not invoke callback");
+ fail_unless(multi->last_expire_offset_us == expire_offset_us,
+ "timer refresh changed expiry offset");
+ fail_unless(multi->last_timeout_set, "refreshed timer not marked as set");
+
+ Curl_expire_clear_all(multi->admin);
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "timer clear update failed");
+ fail_unless(ctx.count == 4, "clearing timer did not invoke callback");
+ fail_unless(!multi->last_timeout_set, "cleared timer marked as set");
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "repeated timer clear update failed");
+
+ Curl_multi_mark_dirty(multi->admin);
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "zero timer update failed");
+ fail_unless(multi->last_timeout_set, "zero timer not marked as set");
+ Curl_multi_clear_dirty(multi->admin);
+ fail_unless(Curl_update_timer(multi) == CURLM_OK,
+ "clearing zero timer failed");
+ fail_unless(!multi->last_timeout_set, "zero timer clear left timer set");
+
+ fail_unless(ctx.count == T3230_TIMER_CALLS, "wrong timer callback count");
+ fail_unless(ctx.timeout_ms[0] >= 0, "wrong initial timeout");
+ fail_unless(ctx.timeout_ms[1] > ctx.timeout_ms[0],
+ "wrong replacement timeout");
+ fail_unless(ctx.timeout_ms[2] >= 0, "wrong refreshed timeout");
+ fail_unless(ctx.timeout_ms[3] == -1, "wrong cleared timeout");
+ fail_unless(ctx.timeout_ms[4] == 0, "wrong zero timeout");
+ fail_unless(ctx.timeout_ms[5] == -1, "wrong zero timeout clear");
+
+ curl_multi_cleanup(handle);
+}
+
+static CURLcode test_unit3230(const char *arg)
+{
+ UNITTEST_BEGIN(t3230_setup())
+
+ t3230_test_timer_updates();
+
+ UNITTEST_END(curl_global_cleanup())
+}