Skip to content

[hyperactor]: mailbox: CanSend for Mailbox use mailbox port for returns #303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: gh/shayne-fletcher/23/base
Choose a base branch
from

Conversation

shayne-fletcher
Copy link
Contributor

@shayne-fletcher shayne-fletcher commented Jun 18, 2025

Stack from ghstack (oldest at bottom):

Differential Revision: D76926674

NOTE FOR REVIEWERS: This PR has internal Meta-specific changes or comments, please review them on Phabricator!

…urns

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 18, 2025
…urns

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

ghstack-source-id: 291340611
Pull Request resolved: #303
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 18, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 18, 2025
…urns

Pull Request resolved: #303

 the `CanSend` for `Mailbox` produces a self-bound `PortHandle` (allocating and binding only if needed). i don't have any immediate test coverage but removing `monitored_return_handle()` from this code is a definite and substantial improvement.
ghstack-source-id: 291349747

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 18, 2025
…urns

Pull Request resolved: #303

 the `CanSend` for `Mailbox` produces a self-bound `PortHandle` (allocating and binding only if needed). i don't have any immediate test coverage but removing `monitored_return_handle()` from this code is a definite and substantial improvement.
ghstack-source-id: 291396595

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 19, 2025
…urns

Pull Request resolved: #303

 the `CanSend` for `Mailbox` requires an undeliverable message port be self-bound. i don't have any immediate test coverage but removing `monitored_return_handle()` is a clear improvement.

ghstack-source-id: 291398715

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 19, 2025
…urns

Pull Request resolved: #303

 the `CanSend` for `Mailbox` requires an undeliverable message port be self-bound. i don't have any immediate test coverage but removing `monitored_return_handle()` is a clear improvement.

ghstack-source-id: 291407170

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 19, 2025
…urns

Pull Request resolved: #303

 the `CanSend` for `Mailbox` requires an undeliverable message port be self-bound. i don't have any immediate test coverage but removing `monitored_return_handle()` is a clear improvement.

ghstack-source-id: 291418560

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 19, 2025
…urns

Pull Request resolved: #303

 the `CanSend` for `Mailbox` requires an undeliverable message port be self-bound. i don't have any immediate test coverage but removing `monitored_return_handle()` is a clear improvement.

ghstack-source-id: 291425647

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 19, 2025
…urns

Pull Request resolved: #303

the `CanSend` for `Mailbox` requires an undeliverable message port be self-bound.  legacy tests were updated to be compliant with the new requirement.

removing `monitored_return_handle()` from `CanSend` is a clear improvement.

ghstack-source-id: 291502157

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

…ort for returns"

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!

[ghstack-poisoned]
shayne-fletcher pushed a commit that referenced this pull request Jun 19, 2025
…urns

Pull Request resolved: #303

the `CanSend` for `Mailbox` attempts to use a self-bound undeliverable message port and only falls back on `monitored_return_handle()` if one isn't bound.

replacing unconditional use of `monitored_return_handle()` is a significant improvement.

ghstack-source-id: 291503427

Differential Revision: [D76926674](https://our.internmc.facebook.com/intern/diff/D76926674/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76926674/)!
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76926674

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants