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 documentation about virtual threads. #8900

Merged
merged 3 commits into from
Nov 21, 2022

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Nov 14, 2022

Added programming guide section about Jetty threading model.

Signed-off-by: Simone Bordet [email protected]

@sbordet sbordet linked an issue Nov 14, 2022 that may be closed by this pull request
Deciding between these two modes depends on whether there is a free thread immediately available to take over production, and this is captured by the `org.eclipse.jetty.util.thread.TryExecutor` interface.

An implementation of `TryExecutor` could be asked whether it can immediately allocate a free thread to run a task, as opposed to a normal `Executor`, that can only queue the task in the hope that there will be a thread in the near future that could run the task.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If a task declares that it can run in either blocking or non blocking mode, then the strategy may choose to use Produce-Consume with the additional invocation flag to force non-blocking mode; or it may choose one of the other strategies used for blocking tasks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this out on purpose to avoid to go too deep into the implementation details.
For example, I do not mention the EITHER invocation type, the PRODUCE_INVOKE_CONSUME mode, etc.
For now I'd leave it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is important to add at least a small mention like this. Else it appears that the documentation does not describe the actual implementation. It doesn't need a deep dive, just a mention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your suggestion is just too obscure for those that don't know the implementation.
99.999% of usages will be either blocking or non-blocking, "either" is really for super-special cases and if a developer is there, reading the code/javadocs is better than reading the documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think that EITHER and PIC needs to be mentioned somewhere in the doco, otherwise it looks like it is for a different version of the code.

Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

Just some questions

@joakime joakime self-requested a review November 16, 2022 20:12
joakime
joakime previously approved these changes Nov 16, 2022
gregw
gregw previously approved these changes Nov 16, 2022
Added programming guide section about Jetty threading model.

Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
@sbordet sbordet dismissed stale reviews from gregw and joakime via 72d5367 November 21, 2022 18:10
@sbordet sbordet force-pushed the fix/jetty-10-document-usevirtualthreads branch from 0980ca2 to 72d5367 Compare November 21, 2022 18:10
@sbordet sbordet merged commit e33c9a1 into jetty-10.0.x Nov 21, 2022
@sbordet sbordet deleted the fix/jetty-10-document-usevirtualthreads branch November 21, 2022 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document useVirtualThreads
4 participants