Issue #350 Add more efficient shortest queue selection methods. #557
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
myronkscott
reviewed
May 11, 2017
|
|
||
|
|
||
| private static ShortestFindStrategy chooseStrategy(ShortestFindStrategy defaultVal) { | ||
| String stratName = System.getProperty(FINDSTRATEGY_PROPNAME, defaultVal.name()); |
There was a problem hiding this comment.
formalize with a tc.property? It is easier to enable in tests etc.
| } else if (queueCount > 2) { | ||
| LEAP_SHIFT = 1; | ||
| } else { | ||
| LEAP_SHIFT = 0; |
There was a problem hiding this comment.
The vast majority of StageQueue uses are this. Is it worth fastpathing this in find shortest queue (probably never used) and disabling the added infrastructure?
…methods. For StageQueueImpl, and interface StageQueue was extracted; a factory is now used to create StageQueue's. This allowed me to split StageQueueImpl into 2 different implementations. SingletonStageQueueImpl is used when only 1 queue is desired (allowing for much more efficiency in this case, while MultiStageQueueImpl is used for queue counts >1. In MultiStageQueueImpl, I created an enum for two different shortest path impls. BRUTE is the classic brute force approach, while PARTITION simply partitions the queues into sets of 4 (or 2 if the number of queue is <8, or 1 if <2), and then on each findShortestQueue(), we choose the next partition, and brute force the shortest queue within that partition. Since the partitions are so small, the short brute force search is fast, and the clock-cache like behavior of round robin selection of partition should be pretty good at finding the shortest queue most of the time. Synthetic testing showed this to be extremely efficient. Another minheap approach was tried, but the overhead on put()/poll() was too great to be useful. A system property, "tc.stagequeueimpl.findstrategy", is available which cane be set to one of BRUTE or PARTITION to choose the findstrategy while we test in real world conditions. By default, PARTITION is used.
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.
Updated for rework...
For StageQueueImpl, an interface StageQueue was extracted; a factory
is now used to create StageQueue's. This allowed me to split
StageQueueImpl into 2 different implementations. SingletonStageQueueImpl
is used when only 1 queue is desired (allowing for much more efficiency)
in this case, while MultiStageQueueImpl is used for queue counts >1.
In MultiStageQueueImpl, I created an enum for two different shortest path
impls. BRUTE is the classic brute force approach, while PARTITION
simply partitions the queues into sets of 4 (or 2 if the number of
queue is <8, or 1 if <2), and then on each findShortestQueue(),
we choose the next partition, and brute force the shortest queue
within that partition. Since the partitions are so small, the short
brute force search is fast, and the clock-cache like behavior
of round robin selection of partition should be pretty good at finding
the shortest queue most of the time. Synthetic testing showed this to
be extremely efficient.
Another minheap approach was tried, but the overhead on put()/poll()
was too great to be useful.
A system property, "tc.stagequeueimpl.findstrategy", is available which
cane be set to one of BRUTE or PARTITION to choose the findstrategy
while we test in real world conditions. By default, PARTITION is used.