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

Allow spawning of an arbitrary number of tasks. #16

Merged
merged 4 commits into from
Apr 8, 2021

Conversation

jorajeev
Copy link
Member

@jorajeev jorajeev commented Apr 8, 2021

Previously, we required users to specify (via the max_tasks parameter in the Config) the maximum number of tasks that could be spawned during an execution. This change lifts that restriction, and allows spawning of an arbitrary number of tasks.

Note: The PCT scheduler still only supports creation of upto 16 tasks. A forthcoming PR will update PCT to also support spawning of an arbitrary number of tasks.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Previously, we required users to specify (via the `max_tasks` parameter in the `Config`) the maximum number of tasks that could be spawned during an execution.  This change lifts that restriction, and allows spawning of an arbitrary number of tasks.

Note: The PCT scheduler still only supports creation of upto 16 tasks.  A forthcoming PR will update PCT to also support spawning of an arbitrary number of tasks.

let scheduler = RandomScheduler::new(100);
#[test]
fn many_tasks_with_mutex() {
Copy link
Member

Choose a reason for hiding this comment

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

Once we fix PCT, we should duplicate this test and run it under PCT as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Comment on lines 207 to 206
tasks: Vec::with_capacity(max_tasks),
tasks: Vec::with_capacity(DEFAULT_INLINE_TASKS),
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth changing this field into a SmallVec just for consistency.

@jamesbornholt jamesbornholt self-requested a review April 8, 2021 21:33
Comment on lines 199 to 202
if tid.0 >= self.tasks.len() {
self.tasks.resize(2 * self.tasks.len(), false);
}
assert!(self.tasks.len() > tid.0);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if our current size is 32 and then we ask to insert task 70? I think we should just resize to length tid.0 + 1 and let the BitVec decide how it wants to amortize that cost (it just delegates to Vec IIRC).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch; fixing

Rajeev Joshi and others added 3 commits April 8, 2021 16:03
Previously, we required users to specify (via the `max_tasks` parameter in the `Config`) the maximum number of tasks that could be spawned during an execution.  This change lifts that restriction, and allows spawning of an arbitrary number of tasks.

Note: The PCT scheduler still only supports creation of upto 16 tasks.  A forthcoming PR will update PCT to also support spawning of an arbitrary number of tasks.
@jamesbornholt jamesbornholt self-requested a review April 8, 2021 23:15
@jamesbornholt jamesbornholt merged commit 3b89685 into awslabs:main Apr 8, 2021
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.

2 participants