Skip to content
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

Test allocations of isolated ELF views #3054

Merged

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Jan 13, 2025

Motivation:

We've added isolated views onto EventLoopFutures. These are great, but as initially implemented they add a bunch of overhead to the nonisolated versions. That's not optimal, but before we change the code we should make sure we have a benchmark baseline.

Modifications:

  • Extend the test_future_lots_of_callbacks alloc test to add all the callbacks that the isolated views have.
  • Duplicate that test to one that adds isolated wrappers so we can compare.

Result:

Two regression tests whose allocation counts should be equal, but aren't.

Motivation:

We've added isolated views onto EventLoopFutures. These are great,
but as initially implemented they add a bunch of overhead to the nonisolated
versions. That's not optimal, but before we change the code we should make sure
we have a benchmark baseline.

Modifications:

- Extend the test_future_lots_of_callbacks alloc test to add all the callbacks
    that the isolated views have.
- Duplicate that test to one that adds isolated wrappers so we can compare.

Result:

Two regression tests whose allocation counts should be equal, but aren't.
@Lukasa Lukasa added the semver/none No version bump required. label Jan 13, 2025
@Lukasa Lukasa merged commit 588523e into apple:main Jan 13, 2025
34 of 35 checks passed
@Lukasa Lukasa deleted the cb-regression-test-allocations-of-isolated-views branch January 13, 2025 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants