Fixed busy waiting in points 1,2,3,7 #3101
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…busy-looping---Alhusseain
|
iluwatar
requested changes
Jan 11, 2025
| if (buffer.isEmpty()) { | ||
| bufferWait.wait(); | ||
| } | ||
| } | ||
| Thread.sleep(5000); // Flush every 5 seconds. |
There was a problem hiding this comment.
The main problem is this Thread.sleep(5000)
I would fix it with a scheduled executor, e.g.
private void startBufferFlusher() {
ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
scheduler.scheduleAtFixedRate(this::flushBuffer, 5, 5, TimeUnit.SECONDS);
}
| } | ||
|
|
||
| Thread.sleep(1000); |
There was a problem hiding this comment.
This busy-waiting loop can also be fixed by using ScheduledExecutorService in App.java
| @@ -81,9 +85,16 @@ public static void main(String[] args) throws IOException { | |||
| } | |||
|
|
|||
| private static void sessionExpirationTask() { | |||
| new Thread(() -> { | |||
| sessionExpirationThread = new Thread(() -> { | |||
There was a problem hiding this comment.
Use ScheduledExecutorService to run this periodically
| } else { | ||
| twin.draw(); | ||
| twin.move(); | ||
| Thread.sleep(250); |
There was a problem hiding this comment.
This Thread.sleep(250) is the main problem. Instead of using a thread, we could utilize the same aforementioned ScheduledExecutorService
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close issue #2977
Implemented wait and notify functionalities on threads that have busy waiting issues
No. 1 : Server Session / App.java
sessionExpirationTask now waits until at least a single login request is sent, since there would be unnecessary overhead if it runs with no login requests available.
No. 2 : Twin / BallThread.java
run() now waits if suspendMe is called, and resumes if resumeMe is called, since there would be unnecessary overhead if run() is running when ball item is suspended. ( no need to always check if suspended)
No. 3: Log Aggregation / LogAggregator.java
startBufferFlusher runs if there is at least one log is present, similiar to No.1's fix
No. 7: Queue-Based Load Leveling / ServiceExecutor.java
similiar to No.3 and No.1, at least a single msg is added to queue for it to work, until then run() waits.
I did not refactor No. 4,5,6 since I think no unnecessary overhead is present ( as the retry pattern implemented loops until certain number of errors occur, doesn't loop needlessly)
I would much appreciate your feedback, which I would consider and act upon quickly.