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

Improved ReservedThreads idle timeout #1806

Closed
sbordet opened this issue Sep 8, 2017 · 3 comments
Closed

Improved ReservedThreads idle timeout #1806

sbordet opened this issue Sep 8, 2017 · 3 comments
Assignees

Comments

@sbordet
Copy link
Contributor

sbordet commented Sep 8, 2017

After #1805, ReservedThreads are lazily allocated on demand.
If the server goes idle, these threads currently remain allocated.
It would be good if they can sit idle for a while and then be deallocated.

ReservedThreadExecutor currently use a circular buffer to hold ReservedThreads and there is a strong assumption in the code that ReservedThreads are used from a queue.

Implementing idle timeout means that when a ReservedThread may idle timeout it should be removed from an arbitrary index of the circular buffer, which would then have a hole that would then require other elements to be shifted, adjusting head and tail pointers. Perhaps just use a JDK queue ?

@gregw
Copy link
Contributor

gregw commented Sep 13, 2017

I've created a proposed fix in branch https://github.com/eclipse/jetty.project/compare/jetty-9.4.x-1806?expand=1

However, it will only shrink if the whole pool is idle for the period.

It may be better to have a stack of threads rather than a queue, so last in first out will be applied (good for data and instruction caches) and we can idle out unused threads in a moderately active reserved pool.

gregw added a commit that referenced this issue Sep 13, 2017
@gregw
Copy link
Contributor

gregw commented Sep 13, 2017

@sbordet I have updated the branch to use a stack of reserved threads. This thus is we have a low level of activity, a few reserved threads may be used (with hotter caches) and the threads on the bottom of the stack will timeout.

Review is needed!

gregw added a commit that referenced this issue Sep 22, 2017
Added a concurrent stack class
Converted ReservedThreadExcecutor to use the concurrent stack
Added support for idle
@gregw
Copy link
Contributor

gregw commented Sep 22, 2017

@sbordet In branch (jetty-9.4.x-1806-ConcurrentReservedThreadExecutor](https://github.com/eclipse/jetty.project/compare/jetty-9.4.x-1806-ConcurrentReservedThreadExecutor?expand=1) I have reimplemented ReservedThreadExecutor to use a non blocking stack and to handle timeouts.

gregw added a commit that referenced this issue Sep 28, 2017
Squashed commit of the following:

commit ab7f711
Author: Greg Wilkins <gregw@webtide.com>
Date:   Thu Sep 28 10:20:08 2017 +1000

    Fixed format

commit 062b051
Merge: d1e27d4 7d98cbb
Author: Greg Wilkins <gregw@webtide.com>
Date:   Thu Sep 28 09:19:40 2017 +1000

    Merge branch 'jetty-9.4.x' into jetty-9.4.x-1806-ConcurrentReservedThreadExecutor

commit d1e27d4
Author: Greg Wilkins <gregw@webtide.com>
Date:   Tue Sep 26 11:28:21 2017 +1000

    improvements after review

commit 477e3ac
Merge: 51a3ac3 35d0b59
Author: Greg Wilkins <gregw@webtide.com>
Date:   Tue Sep 26 10:10:30 2017 +1000

    Merge branch 'jetty-9.4.x' into jetty-9.4.x-1806-ConcurrentReservedThreadExecutor

commit 51a3ac3
Author: Greg Wilkins <gregw@webtide.com>
Date:   Tue Sep 26 10:08:55 2017 +1000

    improvements after review

commit 23df855
Merge: b52c6de 5764afc
Author: Greg Wilkins <gregw@webtide.com>
Date:   Tue Sep 26 09:30:42 2017 +1000

    Merge branch 'jetty-9.4.x' into jetty-9.4.x-1806-ConcurrentReservedThreadExecutor

commit b52c6de
Author: Greg Wilkins <gregw@webtide.com>
Date:   Fri Sep 22 15:36:53 2017 +1000

    Issue #1806

    Added a concurrent stack class
    Converted ReservedThreadExcecutor to use the concurrent stack
    Added support for idle
@gregw gregw closed this as completed Oct 3, 2017
@joakime joakime changed the title ReservedThreads idle timeout Improved ReservedThreads idle timeout Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants