Skip to content

clean: Thread.sleep() util; demo simpleLoop() w.o. output#17

Merged
vorburger merged 3 commits into
enola-dev:mainfrom
vorburger:LongIncrementingTask3
Oct 4, 2025
Merged

clean: Thread.sleep() util; demo simpleLoop() w.o. output#17
vorburger merged 3 commits into
enola-dev:mainfrom
vorburger:LongIncrementingTask3

Conversation

@vorburger
Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings October 4, 2025 20:52
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @vorburger, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances thread management by introducing a new Threads utility class that provides a more robust and safer sleep method. This utility is then integrated into the LongIncrementingTask to standardize sleep operations. Additionally, a new demonstration is included to illustrate the utility's usage in a basic looping scenario.

Highlights

  • New Threads.sleep() Utility: Introduced a Threads utility class with a sleep method that correctly handles InterruptedException by re-interrupting the current thread, validates against negative durations, and optimizes for zero duration.
  • LongIncrementingTask Refactoring: Updated LongIncrementingTask to utilize the newly introduced Threads.sleep() utility, replacing direct Thread.sleep() calls for improved robustness.
  • simpleLoop() Demonstration: Added a simpleLoop() method to LongIncrementingTask's main method to demonstrate looping with the new Threads.sleep() utility, specifically showcasing a scenario without output.
  • Minor TaskCallable Message Update: Modified the NullPointerException message in TaskCallable for better clarity.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a Thread.sleep() utility wrapper and demonstrates a simple loop without output functionality. The changes clean up thread sleep handling by consolidating the interruption logic into a reusable utility.

  • Adds a new Threads utility class with a safer sleep() method that properly handles InterruptedException
  • Refactors LongIncrementingTask to use the new sleep utility and adds a demonstration method
  • Makes minor formatting and error message improvements in TaskCallable

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/dev/enola/be/task/demo/Threads.java New utility class providing a safer Thread.sleep() wrapper with proper interrupt handling
src/dev/enola/be/task/demo/LongIncrementingTask.java Refactored to use new sleep utility and added simpleLoop() demo method
src/dev/enola/be/task/TaskCallable.java Minor formatting improvements and simplified error message

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/dev/enola/be/task/demo/Threads.java Outdated
public static void main(String[] args) throws InterruptedException {
// Count to max, with 1ms pause between each increment
var max = 10000;
var sleep = Duration.ofMillis(0);
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Using a sleep duration of 0 milliseconds may not provide meaningful demonstration of the sleep functionality. Consider using a small positive duration like Duration.ofMillis(1) to better demonstrate the sleep behavior.

Suggested change
var sleep = Duration.ofMillis(0);
var sleep = Duration.ofMillis(1);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a Threads.sleep() utility method to centralize sleep logic with proper interrupt handling. It also updates LongIncrementingTask to use this utility and adds a simpleLoop method for demonstration/comparison purposes. The changes are a good step towards cleaner concurrency code. My review includes suggestions to improve the readability of the new demo code and to correct and clarify the Javadoc for the new Threads utility class.

Comment on lines +44 to +50
System.out.println(
"Looped to "
+ max
+ " with "
+ sleep
+ " sleep, but without output, in "
+ duration);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The multi-line string concatenation for logging can be made more readable and efficient. Consider using String.format() or System.out.printf() for a cleaner implementation.

        System.out.printf("Looped to %d with %s sleep, but without output, in %s%n", max, sleep, duration);

Comment on lines +15 to +16
* as a checked exception. It also checks for negative duration (which the original method just
* ignores, which could hide bugs), and has an optimizing shortcut for duration 0.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The Javadoc states that this wrapper checks for negative duration which the "original method" ignores. This is true for Thread.sleep(long), but Thread.sleep(Duration), which is used here, already throws an IllegalArgumentException for negative durations. The comment could be clarified to avoid confusion, for example by specifying which Thread.sleep overload it refers to.

Comment thread src/dev/enola/be/task/demo/Threads.java Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings October 4, 2025 20:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/dev/enola/be/task/demo/Threads.java Outdated
* something different from this).
*
* @param duration Duration to sleep
* @throws UncheckedInterruptedException if interrupted
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

The javadoc mentions throwing UncheckedInterruptedException but the actual implementation throws InterruptedException (line 40). The documentation should be updated to reflect the actual exception type thrown.

Suggested change
* @throws UncheckedInterruptedException if interrupted
* @throws InterruptedException if interrupted

Copilot uses AI. Check for mistakes.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings October 4, 2025 20:57
@vorburger vorburger added this pull request to the merge queue Oct 4, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

var input = new Input(10000, Duration.ofMillis(1));
private static void simpleLoop(long max, Duration sleep) throws InterruptedException {
var start = Instant.now();
for (long i = 0; i < max; i++) Threads.sleep(sleep);
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

The loop calls Threads.sleep() for each iteration without any yield or interrupt check. For large values of max, this could create performance issues and make the method unresponsive to interruption. Consider adding periodic interrupt checks similar to the main execute() method.

Suggested change
for (long i = 0; i < max; i++) Threads.sleep(sleep);
for (long i = 0; i < max; i++) {
Thread.yield();
if (Thread.currentThread().isInterrupted())
throw new InterruptedException("Loop was interrupted");
Threads.sleep(sleep);
}

Copilot uses AI. Check for mistakes.
Merged via the queue into enola-dev:main with commit 3d003c4 Oct 4, 2025
1 check passed
@vorburger vorburger deleted the LongIncrementingTask3 branch October 4, 2025 20:57
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