[Core] Avoid O(n^2) header dedup when cloning EF_UNIQ_HEADERS events (#3061)

* events: avoid O(n^2) header dedup when cloning EF_UNIQ_HEADERS events

switch_event_dup() copies a source event header by header via
switch_event_add_header_string(). When the source carries
EF_UNIQ_HEADERS (SWITCH_EVENT_CHANNEL_DATA / REQUEST_PARAMS / MESSAGE --
e.g. a channel's variable list), the destination inherits the flag, so
switch_event_base_add_header() runs a full switch_event_del_header()
linear scan of the partially-built list on every add to enforce
uniqueness. Cloning an N-header event is therefore O(n^2).

That scan is redundant while cloning: the source already enforced
uniqueness on insert, so copying it cannot introduce duplicates
regardless of the per-add scan. Clear EF_UNIQ_HEADERS on the
destination for the duration of the copy loop, then restore it.

This is a hot path. switch_channel_execute_on() -- invoked on ring,
answer, media, transfer, park, record and playback -- calls
switch_core_get_variables() and switch_channel_get_variables(), each of
which dup()s an EF_UNIQ_HEADERS event. On a production voicemail server
carrying ~191 channel variables per call, switch_event_del_header_val()
accounted for 47-69% of all FreeSWITCH CPU time in on-box perf profiles.

Microbenchmark (tests/unit/switch_event.c, switch_event_dup of an
N-header CHANNEL_DATA event, 4000 iterations, time per dup):

    N      before     after
    50     4.80 us    3.19 us
    100   11.34 us    5.88 us
    191   28.29 us   10.99 us   (2.6x)
    400  107.05 us   23.27 us   (4.6x)

"before" scales ~O(n^2); "after" is linear. Behaviour is unchanged: the
clone has identical headers, values and order, keeps EF_UNIQ_HEADERS,
and still enforces uniqueness on subsequent adds (covered by the test).

Signed-off-by: Calvin Ellison <cellison@youmail.com>

* tests: cover switch_event_dup uniqueness and add a dup benchmark

Adds a dup_uniq_bench case to the switch_event unit test. For an
EF_UNIQ_HEADERS (CHANNEL_DATA) source event it verifies the clone
preserves header count, values and the EF_UNIQ_HEADERS flag, and that
re-setting an existing key does not produce a duplicate in the clone.
It also prints a switch_event_dup timing sweep across header counts,
used to validate the O(n^2) -> O(n) change in switch_event_dup().

Signed-off-by: Calvin Ellison <cellison@youmail.com>

* tests: add dup_faithful_copy regression for switch_event_dup

Covers the EF_UNIQ_HEADERS edge case for the O(n^2)->O(n) dup change: a well-formed EF_UNIQ source stays unique through dup, and a malformed source already holding duplicate names (only possible if headers were added before the flag was set) is copied faithfully rather than silently collapsed to the last value.

Signed-off-by: Calvin Ellison <cellison@youmail.com>

* tests: gate dup_uniq_bench behind #ifdef BENCHMARK

The dup_uniq_bench timing sweep is a benchmark rather than a pass/fail correctness test. Guard the whole test with #ifdef BENCHMARK (matching the existing benchmark in this file) so it stays out of normal test runs; dup_faithful_copy continues to cover correctness of the switch_event_dup change.

Signed-off-by: Calvin Ellison <cellison@youmail.com>

* switch_event: clarify EF_UNIQ_HEADERS dup comment, brace one-line test loops

Address review feedback on the switch_event_dup change:

- src/switch_event.c: the fast-path comment claimed todup's headers are "already unique" unconditionally. That only holds when EF_UNIQ_HEADERS is set (names are deduped on insert); reword to scope the claim to that case and to the verbatim-copy intent.

- tests/unit/switch_event.c: expand one-line loops to braced form per the SignalWire coding guidelines.

Signed-off-by: Calvin Ellison <cellison@youmail.com>

---------

Signed-off-by: Calvin Ellison <cellison@youmail.com>
Co-authored-by: Calvin Ellison <cellison@youmail.com>

src/switch_event.c

@@ -1335,6 +1335,7 @@ SWITCH_DECLARE(void) switch_event_merge(switch_event_t *event, switch_event_t *t
 SWITCH_DECLARE(switch_status_t) switch_event_dup(switch_event_t **event, switch_event_t *todup)
 {
 	switch_event_header_t *hp;
+	int restore_uniq_headers = 0;
 
 	if (switch_event_create_subclass(event, SWITCH_EVENT_CLONE, todup->subclass_name) != SWITCH_STATUS_SUCCESS) {
 		return SWITCH_STATUS_GENERR;
@@ -1344,6 +1345,19 @@ SWITCH_DECLARE(switch_status_t) switch_event_dup(switch_event_t **event, switch_
 	(*event)->event_user_data = todup->event_user_data;
 	(*event)->bind_user_data = todup->bind_user_data;
 	(*event)->flags = todup->flags;
+
+	/* The destination inherited todup's flags above. When EF_UNIQ_HEADERS is set,
+	 * switch_event_base_add_header() rescans the partial header list on every add
+	 * to keep names unique, making this copy loop O(n^2). A clone should reproduce
+	 * todup's list verbatim rather than run a fresh uniqueness pass, and a
+	 * well-formed EF_UNIQ_HEADERS source is already unique (its names were deduped
+	 * on insert), so that scan only adds cost here. Suppress it for the copy, then
+	 * restore the flag so later adds enforce uniqueness again. */
+	if (switch_test_flag((*event), EF_UNIQ_HEADERS)) {
+		switch_clear_flag((*event), EF_UNIQ_HEADERS);
+		restore_uniq_headers = 1;
+	}
+
 	for (hp = todup->headers; hp; hp = hp->next) {
 		if (todup->subclass_name && !strcmp(hp->name, "Event-Subclass")) {
 			continue;
@@ -1359,6 +1373,10 @@ SWITCH_DECLARE(switch_status_t) switch_event_dup(switch_event_t **event, switch_
 		}
 	}
 
+	if (restore_uniq_headers) {
+		switch_set_flag((*event), EF_UNIQ_HEADERS);
+	}
+
 	if (todup->body) {
 		(*event)->body = DUP(todup->body);
 	}

tests/unit/switch_event.c

@@ -161,6 +161,160 @@ FST_TEST_BEGIN(expand_headers_offset)
 }
 FST_TEST_END()
 
+#ifdef BENCHMARK
+FST_TEST_BEGIN(dup_uniq_bench)
+{
+  switch_event_t *src = NULL, *dup = NULL;
+  switch_status_t status;
+  const int loops = 4000;
+  const int Ns[4] = {50, 100, 191, 400};
+  int i, j, count, k, S, Nv;
+  char name[32], val[64];
+  switch_event_header_t *hp;
+  switch_time_t start_ts, end_ts;
+  double per_us;
+
+  /* A channel's variables are a SWITCH_EVENT_CHANNEL_DATA event => EF_UNIQ_HEADERS.
+   * switch_event_create() also adds the standard event headers, so the source
+   * count is (191 added + standard); assert relative to the actual source count. */
+  status = switch_event_create(&src, SWITCH_EVENT_CHANNEL_DATA);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "create CHANNEL_DATA event");
+  fst_xcheck(switch_test_flag(src, EF_UNIQ_HEADERS) != 0, "CHANNEL_DATA event is EF_UNIQ_HEADERS");
+
+  for (i = 0; i < 191; i++) {
+    switch_snprintf(name, sizeof(name), "var_%d", i);
+    switch_snprintf(val, sizeof(val), "value_%d_some_padding_payload", i);
+    switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, name, val);
+  }
+  S = 0;
+  for (hp = src->headers; hp; hp = hp->next) {
+    S++;
+  }
+  fst_xcheck(S >= 191, "source has at least the 191 added headers");
+
+  /* correctness: dup preserves header count, the uniq flag, and values */
+  status = switch_event_dup(&dup, src);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok");
+  count = 0;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    count++;
+  }
+  fst_xcheck(count == S, "dup header count equals source");
+  fst_xcheck(switch_test_flag(dup, EF_UNIQ_HEADERS) != 0, "dup keeps EF_UNIQ_HEADERS");
+  fst_check_string_equals(switch_event_get_header(dup, "var_0"), "value_0_some_padding_payload");
+  fst_check_string_equals(switch_event_get_header(dup, "var_190"), "value_190_some_padding_payload");
+  switch_event_destroy(&dup);
+
+  /* correctness: uniqueness preserved (re-setting a key must not duplicate it in the dup) */
+  switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "var_0", "REPLACED");
+  status = switch_event_dup(&dup, src);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok (2)");
+  k = 0;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "var_0")) {
+      k++;
+    }
+  }
+  fst_xcheck(k == 1, "var_0 present exactly once after re-set + dup");
+  fst_check_string_equals(switch_event_get_header(dup, "var_0"), "REPLACED");
+  switch_event_destroy(&dup);
+  switch_event_destroy(&src);
+
+  /* timing sweep: dup cost vs header count (baseline ~O(n^2), patched ~O(n)) */
+  for (j = 0; j < 4; j++) {
+    Nv = Ns[j];
+    switch_event_create(&src, SWITCH_EVENT_CHANNEL_DATA);
+    for (i = 0; i < Nv; i++) {
+      switch_snprintf(name, sizeof(name), "var_%d", i);
+      switch_snprintf(val, sizeof(val), "value_%d_some_padding_payload", i);
+      switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, name, val);
+    }
+    start_ts = switch_time_now();
+    for (i = 0; i < loops; i++) {
+      switch_event_dup(&dup, src);
+      switch_event_destroy(&dup);
+    }
+    end_ts = switch_time_now();
+    per_us = (double)(end_ts - start_ts) / (double)loops;
+    printf("DUP_BENCH N=%d loops=%d total=%" SWITCH_UINT64_T_FMT "us per_dup=%.3f us\n",
+           Nv, loops, (uint64_t)(end_ts - start_ts), per_us);
+    switch_event_destroy(&src);
+  }
+}
+FST_TEST_END()
+#endif /* BENCHMARK */
+
+FST_TEST_BEGIN(dup_faithful_copy)
+{
+  /* Regression for the switch_event_dup O(n^2)->O(n) change (suppressing the
+   * per-add EF_UNIQ_HEADERS dedup scan while cloning). dup must reproduce the
+   * source faithfully:
+   *   (1) a well-formed EF_UNIQ source stays unique through dup (the real case);
+   *   (2) a malformed source that already holds duplicate names (only possible
+   *       if headers were added before EF_UNIQ_HEADERS was set) is copied
+   *       faithfully -- dup PRESERVES the duplicates rather than silently
+   *       collapsing to the last value. A dup-bearing EF_UNIQ event is already
+   *       a bug upstream of here; a copy must not silently drop data. */
+  switch_event_t *src = NULL, *dup = NULL;
+  switch_event_header_t *hp;
+  switch_status_t status;
+  int n;
+
+  /* (1) well-formed EF_UNIQ source -> unique dup */
+  status = switch_event_create(&src, SWITCH_EVENT_CHANNEL_DATA);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "create CHANNEL_DATA");
+  fst_xcheck(switch_test_flag(src, EF_UNIQ_HEADERS) != 0, "CHANNEL_DATA is EF_UNIQ");
+  switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "k", "v1");
+  switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "k", "v2");
+  n = 0;
+  for (hp = src->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "k")) {
+      n++;
+    }
+  }
+  fst_xcheck(n == 1, "EF_UNIQ source keeps 'k' unique (collapsed to last)");
+  status = switch_event_dup(&dup, src);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok (unique)");
+  n = 0;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "k")) {
+      n++;
+    }
+  }
+  fst_xcheck(n == 1, "dup of unique source stays unique");
+  fst_check_string_equals(switch_event_get_header(dup, "k"), "v2");
+  switch_event_destroy(&dup);
+  switch_event_destroy(&src);
+
+  /* (2) malformed source (duplicate names under EF_UNIQ) -> faithful copy */
+  status = switch_event_create(&src, SWITCH_EVENT_GENERAL);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "create GENERAL");
+  fst_xcheck(switch_test_flag(src, EF_UNIQ_HEADERS) == 0, "GENERAL is not EF_UNIQ");
+  switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "dupkey", "first");
+  switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "dupkey", "second");
+  switch_set_flag(src, EF_UNIQ_HEADERS);
+  n = 0;
+  for (hp = src->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "dupkey")) {
+      n++;
+    }
+  }
+  fst_xcheck(n == 2, "malformed source holds 2 'dupkey' headers");
+  status = switch_event_dup(&dup, src);
+  fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok (malformed)");
+  fst_xcheck(switch_test_flag(dup, EF_UNIQ_HEADERS) != 0, "dup keeps EF_UNIQ flag");
+  n = 0;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "dupkey")) {
+      n++;
+    }
+  }
+  fst_xcheck(n == 2, "dup faithfully preserves both 'dupkey' headers (no silent collapse)");
+  switch_event_destroy(&dup);
+  switch_event_destroy(&src);
+}
+FST_TEST_END()
+
 FST_SUITE_END()
 
 FST_MINCORE_END()