Add baseline performance and allocation tests for scheduleTask and execute #2009
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
11 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
@swift-nio-bot add to allowlist |
Lukasa
requested changes
Dec 13, 2021
IntegrationTests/tests_04_performance/test_01_resources/test_schedule_10000_tasks.swift
Outdated
Show resolved
Hide resolved
IntegrationTests/tests_04_performance/test_01_resources/test_schedule_and_run_10000_tasks.swift
Outdated
Show resolved
Hide resolved
IntegrationTests/tests_04_performance/test_01_resources/test_schedule_and_run_10000_tasks.swift
Outdated
Show resolved
Hide resolved
FranzBusch
force-pushed
the
feature/scheduling-task-tests
branch
from
aa0912e
to
0016916
Compare
|
@swift-nio-bot test perf please |
performance reportbuild id: 86 timestamp: Mon Dec 13 10:18:41 UTC 2021 results
comparison
significant differences found |
FranzBusch
force-pushed
the
feature/scheduling-task-tests
branch
3 times, most recently
from
5ff8490
to
f6f7437
Compare
…d executing ### Motivation: In issue apple#1316, we see a large number of allocations to happen when scheduling tasks. This can definitely be optimized. This PR adds a number of baseline allocation and performance tests for both `scheduleTask` and `execute`. In the next PRs, I am going to try a few optimizations to reduce the number of allocations. ### Modifications: Added baseline performance and allocation tests for `scheduleTask` and `execute`
FranzBusch
force-pushed
the
feature/scheduling-task-tests
branch
from
f6f7437
to
a3880a1
Compare
|
@swift-nio-bot test perf please |
performance reportbuild id: 87 timestamp: Mon Dec 13 15:47:57 UTC 2021 results
comparison
significant differences found |
Lukasa
approved these changes
Dec 13, 2021
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 13, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 13, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
FranzBusch
added a commit
to FranzBusch/swift-nio
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR apple#2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
Lukasa
pushed a commit
that referenced
this pull request
Dec 14, 2021
### Motivation: In my previous PR #2009, I added baseline performance and allocation tests around `scheduleTask` and `execute`. After analysing, the various allocations that happen when scheduling a task there were only a few that could be optimized away potentially. ### Modifications: This PR converts the `ScheduledTask` class to a struct which will reduce the number of allocations for scheduling tasks by 1. The only thing that needs to be worked around when converting to a struct is giving it an identity so that we can implement `Equatable` conformance properly. I explored two options. First, using an `ObjectIdentifier` passed to the init. Second, using an atomic counter per EventLoop. I went with the latter since the former requires an additional allocation in the case of calling `execute` ### Result: `scheduleTask` and `execute` require one less allocation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
In issue #1316, we see a large number of allocations to happen when scheduling tasks. This can definitely be optimized. This PR adds a number of baseline allocation and performance tests for both
scheduleTaskandexecute. In the next PRs, I am going to try a few optimizations to reduce the number of allocations.Modifications:
Added baseline performance and allocation tests for
scheduleTaskandexecute