Skip to content

multi_ntfy: replace enabled bitset with inline flags - #22804

Merged
cmeister2 merged 1 commit into
masterfrom
cm2/multi-notify-inline-flags
Sep 3, 2026
Merged

cmeister2 merged 1 commit into
masterfrom
cm2/multi-notify-inline-flags

Conversation

@cmeister2

Copy link
Copy Markdown
Contributor

Store enabled notification types in the low bits of a uint32_t and use the high bit for the pending-entry state.

This reduces curl_multi_ntfy to 40 bytes and removes an allocation from multi-handle initialization.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The refactor cleanly removes the allocation-backed bitset, updates all internal references, and includes a compile-time guard to prevent flag-bit overlap.

Pull request overview

This PR optimizes the internal multi-notification bookkeeping by replacing the dynamically sized uint32_bset “enabled” bitset with an inline uint32_t flags field, using low bits for enabled notification types and bit 31 for the “has pending entries” state. This matches the goal of shrinking curl_multi_ntfy and removing an allocation during multi-handle initialization.

Changes:

  • Removed Curl_mntfy_resize() and the associated allocation/resizing of the enabled-notification bitset during Curl_multi_handle() initialization.
  • Replaced multi->ntfy.enabled and multi->ntfy.has_entries with a single multi->ntfy.flags word, including a compile-time guard that notification types stay below bit 31.
  • Updated notification enable/disable checks and dispatch/add logic to use flag bit tests instead of Curl_uint32_bset_*.
File summaries
File Description
lib/multi.c Drops Curl_mntfy_resize() from multi-handle initialization and keeps existing notification dispatch checks.
lib/multi_ntfy.h Reworks curl_multi_ntfy to store state in uint32_t flags and updates the “has entries” accessor macro.
lib/multi_ntfy.c Implements enabled-type and pending-entry tracking via bit flags, removes bitset init/resize/destroy, and adds a compile-time bound check.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@testclutch

Copy link
Copy Markdown

Analysis of PR #22804 at 8b455d24:

Test ../../tests/http/test_02_download.py::TestDownload::test_02_33_max_host_conns[True-0-h2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_02_download.py::TestDownload::test_02_34_max_total_conns[True-0-h2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Generated by Testclutch

cmeister2 added a commit that referenced this pull request Sep 2, 2026
Store enabled notification types in the low bits of a uint32_t and use
the high bit for the pending-entry state.

This reduces curl_multi_ntfy to 40 bytes and removes an allocation from
multi-handle initialization.

Closes #22804
@cmeister2
cmeister2 force-pushed the cm2/multi-notify-inline-flags branch from 8b455d2 to 3d61ab3 Compare September 2, 2026 16:52
Store enabled notification types in the low bits of a uint32_t and use
the high bit for the pending-entry state.

This reduces curl_multi_ntfy to 40 bytes and removes an allocation from
multi-handle initialization.

Closes #22804
@cmeister2
cmeister2 force-pushed the cm2/multi-notify-inline-flags branch from 3d61ab3 to e82ae66 Compare September 2, 2026 20:14
@cmeister2
cmeister2 requested a review from bagder September 2, 2026 21:18
@bagder
bagder requested a review from icing September 3, 2026 06:19
@cmeister2
cmeister2 merged commit b01224a into master Sep 3, 2026
365 of 370 checks passed
@cmeister2
cmeister2 deleted the cm2/multi-notify-inline-flags branch September 3, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants