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

Add an allocation test for execute which needs to hop threads. #2047

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

fabianfett
Copy link
Member

We have a bunch of allocation tests around scheduling tasks. However we have calls execute on an EventLoop that we are not executing on.

Motivation:

  • Know how expensive a call to execute is, if we are of the EventLoop.

Modifications:

  • Add an allocation test for execute which needs to hop threads.

Result:

The more allocation knowledge, the better to optimize.

Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

LGTM but we need to add the allocation limits for this test

@glbrntt glbrntt added the needs-no-version-bump For PRs that when merged do not need a bump in version number. label Feb 17, 2022
@fabianfett
Copy link
Member Author

fabianfett commented Feb 17, 2022

@glbrntt we are also missing MAX_ALLOCS_ALLOWED_1000_copying_bytebufferview_to_array=1000 do you mind if I add those as well? Looks like they were forgotten here: https://github.com/apple/swift-nio/pull/2037/files

@Lukasa Lukasa merged commit c562f96 into apple:main Feb 17, 2022
@fabianfett fabianfett deleted the ff-add-allocation-test-execute-hop branch February 17, 2022 09:29
Lukasa added a commit to Lukasa/swift-nio that referenced this pull request Apr 26, 2023
Motivation

This patch extends the NIO integration test harness to track
file descriptors, in particular to search for leaks. This
change has been validated on Linux and Darwin, and in both cases
correctly diagnoses FD leaks.

The goal is to enable us to regression test for things like

Modifications

- Add support for hooking socket and close calls.
- Wire up this support into the test harness.
- Extend the test harness to handle the logging.
- Add new regression test for apple#2047.

Results

We can write regression tests for FD leaks.
Lukasa added a commit that referenced this pull request Apr 26, 2023
* Extend the integration test harness to track FDs

Motivation

This patch extends the NIO integration test harness to track
file descriptors, in particular to search for leaks. This
change has been validated on Linux and Darwin, and in both cases
correctly diagnoses FD leaks.

The goal is to enable us to regression test for things like

Modifications

- Add support for hooking socket and close calls.
- Wire up this support into the test harness.
- Extend the test harness to handle the logging.
- Add new regression test for #2047.

Results

We can write regression tests for FD leaks.

* Disable FD checking in most builds.

I'm doing this for speed reasons

* Always print the leaked fds number
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-no-version-bump For PRs that when merged do not need a bump in version number.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants