-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Run isolated tasks last #4004
Run isolated tasks last #4004
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Deque<ExclusiveTask> isolatedTasks = new LinkedList<>(); | ||
Deque<ExclusiveTask> sameThreadTasks = new LinkedList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 why are we using LinkedList
instead of ArrayDeque
?
It will probably not really matter performance wise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering that, too, but kept using LinkedList
since it was already in place.
@@ -155,29 +157,34 @@ public void invokeAll(List<? extends TestTask> tasks) { | |||
new ExclusiveTask(tasks.get(0)).execSync(); | |||
return; | |||
} | |||
Deque<ExclusiveTask> nonConcurrentTasks = new LinkedList<>(); | |||
Deque<ExclusiveTask> isolatedTasks = new LinkedList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 The only thing that is a bit weird is that isolated tasks can only be top-level tasks, yet this is invoked on multiple levels. Probably not worth changing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could have a different class for the top-level task and other tasks. 🤔
Resolves #3928.