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

Fixes #6652 - Improve ReservedThreadExecutor dump. #6653

Merged

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Aug 23, 2021

Now adding/removing ReservedThread instances to _threads only in reservedWait().
In this way the window of time in which they can be caught for a dump() is shorter, producing more accurate dumps.

Signed-off-by: Simone Bordet simone.bordet@gmail.com

Now adding/removing ReservedThread instances to _threads only in reservedWait().
In this way the window of time in which they can be caught for a dump() is shorter, producing more accurate dumps.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested review from gregw and lorban August 23, 2021 10:21
@sbordet sbordet linked an issue Aug 23, 2021 that may be closed by this pull request
Fixed test failing due to the changes in toString().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this pull request Aug 23, 2021
Builds on #6647 and #6653.

Fixed occurrences of Callbacks that did not override getInvocationType() to properly declare whether they block or not.

Added test case for blocking writes.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
finally
{
// No longer a reserved thread.
_threads.remove(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this extra work here. We want minimum time and effort between allocating a reserved thread and it acting on the task.
Making it modifyf the _threads collection here just wastes time. See my alternative cleanup of dump in #6658

gregw and others added 2 commits August 24, 2021 10:46
+ Revert RTE to not mutate `_threads` on critical path.
+ filter `_threads` in dump and doStop

(cherry picked from commit 69d7add)
Incorporate suggestions from review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested a review from gregw August 24, 2021 08:57
Updated doStop() to interrupt non-null threads.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet merged commit b2a0236 into jetty-9.4.x Aug 24, 2021
@sbordet sbordet deleted the jetty-9.4.x-6652-improve-reserved-thread-executor-dump branch August 24, 2021 21:12
sbordet added a commit that referenced this pull request Aug 24, 2021
Fixes #6652 - Improve ReservedThreadExecutor dump.

Filtering out non-reserved threads in dump() and doStop().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
(cherry picked from commit b2a0236)
sbordet added a commit that referenced this pull request Aug 25, 2021
Fixes #6652 - Improve ReservedThreadExecutor dump.

Filtering out non-reserved threads in dump() and doStop().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
(cherry picked from commit b2a0236)
@sbordet sbordet added this to In progress in Jetty 9.4.44 FROZEN via automation Aug 25, 2021
@sbordet sbordet moved this from In progress to Done in Jetty 9.4.44 FROZEN Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Improve ReservedThreadExecutor dump
2 participants