Skip to content

perf: only compute the duplicate message's MessageId when it is used - #11075

Open
hpeebles wants to merge 1 commit into
dfinity:masterfrom
hpeebles:hpeebles/ingress_skip_duplicate_message_id
Open

perf: only compute the duplicate message's MessageId when it is used#11075
hpeebles wants to merge 1 commit into
dfinity:masterfrom
hpeebles:hpeebles/ingress_skip_duplicate_message_id

Conversation

@hpeebles

@hpeebles hpeebles commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

induct_messages() computed msg.content().id() for every message, but the result is only ever read by the debug! in the duplicate branch, so in the vast majority of cases the computation was unnecessary

`induct_messages()` computed `msg.content().id()` for every message, but
the result is only ever read by the `debug!` in the duplicate branch;
`is_duplicate()` and `induct_message()` both derive the ID themselves.

`SignedIngressContent::id()` is not memoized, so each call builds a
`BTreeMap`, performs a representation-independent hash (including one
pass over the whole argument blob) and allocates ~20 times. Computing it
only on the duplicate path removes one such derivation per inducted
message, i.e. up to `MAX_INGRESS_MESSAGES_PER_BLOCK` (1000) per round.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the perf label Aug 7, 2026
@hpeebles
hpeebles marked this pull request as ready for review August 7, 2026 14:01
@hpeebles
hpeebles requested a review from a team as a code owner August 7, 2026 14:01
@zeropath-ai

zeropath-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 372ef77.

Security Overview
Detected Code Changes
Change Type Relevant files
Bug Fix ► rs/messaging/src/scheduling/valid_set_rule.rs
    Fix logging of duplicate message ID by deferring to msg.content().id() in debug! statement

@basvandijk basvandijk added the security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR. label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

@core-protocol external-contributor perf security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants