Avoid memory allocation by reusing TrackerClient in DegraderStrategy #1121
Conversation
|
|
||
| private List<DegraderTrackerClient> castToDegraderTrackerClients(Map<URI, TrackerClient> trackerClients) | ||
| { | ||
| List<DegraderTrackerClient> degraderTrackerClients = new ArrayList<>(trackerClients.size()); |
There was a problem hiding this comment.
Memory allocation followed by O(N) operations.
| } | ||
|
|
||
| List<DegraderTrackerClient> degraderTrackerClients = castToDegraderTrackerClients(trackerClients); | ||
| Collection<TrackerClient> trackerClientList = trackerClients.values(); |
There was a problem hiding this comment.
castToDegraderTrackerClients return an empty list if trackerClients cannot be cast to DegradedTrackerClient. This is to mimic that behavior.
| if (excludedUris == null) | ||
| { | ||
| excludedUris = new HashSet<>(); | ||
| excludedUris = Collections.emptySet(); |
There was a problem hiding this comment.
Avoid another unnecessary memory allocation.
| warn(_log, "Can not find hash ring to use"); | ||
| } | ||
|
|
||
| Map<URI, DegraderTrackerClient> trackerClientMap = new HashMap<>(trackerClients.size()); |
There was a problem hiding this comment.
Another memory allocation followed by O(N) complexity.
| } | ||
|
|
||
| private DegraderTrackerClient searchClientFromUri(URI uri, List<DegraderTrackerClient> trackerClients) | ||
| private DegraderTrackerClient searchClientFromUri(URI uri, Map<URI, TrackerClient> trackerClients) |
There was a problem hiding this comment.
When it is a map, we do the search in O(1) time rather than O(N).
| * Whether to force update | ||
| * @return True if we should update, and false otherwise. | ||
| */ | ||
| protected static boolean shouldUpdatePartition(long clusterGenerationId, PartitionDegraderLoadBalancerState partitionState, |
There was a problem hiding this comment.
Can we create a new method and call the new one from the old? This is a protected method and we'd like backwards compatibility.
There was a problem hiding this comment.
Kept the old one and created the overloaded version of it. The old one does the conversion (inefficient but still accessible).
2 participants
The call from SimpleLoadBalancer to DegraderStrategy passes a
Map<URI, TrackerClient>object for thegetTrackerClient()method to handle load balancing. This object was being converted toList<DegraderTrackerClient>usingcastToDegraderTrackerClients()method. However, during the load balancing operation, this casting operation was not needed. This PR aims to rectify that. The PR also have positive impact ongetRing()method which was suffering from the same process.We have updated the unit tests accordingly.