Skip to content

Only store failed SignalSyncs#3128

Merged
chenBright merged 1 commit intoapache:masterfrom
chenBright:opt_inuse_signal_syncs
Oct 27, 2025
Merged

Only store failed SignalSyncs#3128
chenBright merged 1 commit intoapache:masterfrom
chenBright:opt_inuse_signal_syncs

Conversation

@chenBright
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number:

Problem Summary:

_inuse_signal_syncs does not need to save successful SignalSyncs, only save failed SignalSyncs.

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

@chenBright chenBright requested a review from Copilot October 26, 2025 07:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes memory usage in the task tracer by only storing failed SignalSync objects instead of all SignalSync objects. Previously, successful SignalSyncs were temporarily added to _inuse_signal_syncs and then removed, which was unnecessary overhead.

  • Removes the unconditional addition of SignalSync to _inuse_signal_syncs before the polling operation
  • Adds SignalSync to _inuse_signal_syncs only when polling fails (EINTR errno case)
  • Removes the pop operation for successful SignalSyncs since they're no longer added

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/bthread/task_tracer.cpp
@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented Oct 27, 2025

LGTM

@chenBright chenBright merged commit 0141e9e into apache:master Oct 27, 2025
17 checks passed
@chenBright chenBright deleted the opt_inuse_signal_syncs branch October 27, 2025 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants