[ISSUE #3983] Optimize MessageQueue #3984
Conversation
| } | ||
|
|
||
| /** | ||
| * Insert the message at the tail of this queue, waiting for space to become available if the queue is full | ||
| * Inserts the specified MessageEntity object into the queue. | ||
| * |
There was a problem hiding this comment.
I think the original comment seems to be clear for making developer or user understand this method easier and faster. Maybe you can combine original comments and yours. CC @xwm1992
| @@ -78,10 +82,10 @@ public void put(MessageEntity messageEntity) throws InterruptedException { | |||
| } | |||
|
|
|||
| /** | |||
| * Get the first message at this queue, waiting for the message is available if the queue is empty, this method will not remove the message | |||
| * Retrieves and removes the head of the queue. | |||
| * | |||
There was a problem hiding this comment.
I think the original comment seems to be more clear for making developer or user understand this method easier and faster. CC @xwm1992
| @@ -143,24 +147,30 @@ public MessageEntity getTail() { | |||
| } | |||
|
|
|||
| /** | |||
| * Get the message by offset, since the offset is increment, so we can get the first message in this queue and calculate the index of this offset | |||
| * Retrieves the MessageEntity object with the specified offset. | |||
| * | |||
There was a problem hiding this comment.
I think the original comment seems to be more clear for making developer or user understand this class easier and faster. CC @xwm1992
| return null; | ||
| } | ||
| int headIndex = takeIndex; | ||
| int tailIndex = putIndex - 1; | ||
| MessageEntity head = itemAt(headIndex); | ||
| if (head.getOffset() > offset) { |
There was a problem hiding this comment.
int headIndex = takeIndex, this assignment seems to be unnecessary. CC @xwm1992
| tailIndex += items.length; | ||
| } | ||
| MessageEntity tail = itemAt(tailIndex); | ||
| if (tail.getOffset() < offset) { |
There was a problem hiding this comment.
The logic of returning null when tail == null is lost in your modification. CC @xwm1992
| @@ -55,21 +58,22 @@ public MessageQueue(int capacity) { | |||
| this.items = new MessageEntity[capacity]; | |||
| this.lock = new ReentrantLock(); | |||
| this.notEmpty = lock.newCondition(); | |||
| this.notFull = lock.newCondition(); | |||
| this.noteFull = lock.newCondition(); | |||
| /** | ||
| * This is a block queue, can get entity by offset. The queue is a FIFO data structure. | ||
| * This is a block queue, can get entity by offset. The queue is a FIFO data structure |
There was a problem hiding this comment.
It seems meaningless to remove the last period from the original comment and add a space before it. CC @xwm1992
|
@mxsm please take a look for review suggestions, and fix the ci check error, thanks. |
I will |
Codecov Report
@@ Coverage Diff @@
## master #3984 +/- ##
============================================
- Coverage 14.22% 14.21% -0.01%
Complexity 1328 1328
============================================
Files 580 580
Lines 28856 28859 +3
Branches 2792 2793 +1
============================================
- Hits 4105 4103 -2
- Misses 24361 24368 +7
+ Partials 390 388 -2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@mxsm please fix the conflicts. |
* [ISSUE apache#3983] Optimize MessageQueue * modify public of attribute items to private * optimize code readability * optimize code logic
Fixes #3983
Motivation
Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.
Modifications
Optimize MessageQueue and add some comments
Documentation