xiangfu0 commented on a change in pull request #7373:
URL: https://github.com/apache/pinot/pull/7373#discussion_r697824109
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##########
@@ -121,62 +121,40 @@ private OrderByValueExtractor
getOrderByValueExtractor(ExpressionContext express
}
/**
- * Constructs an IntermediateRecord from Record
- * The IntermediateRecord::key is the same Record::key
- * The IntermediateRecord::values contains only the order by columns, in the
query's sort sequence
- * For aggregation values in the order by, the final result is extracted if
the intermediate result is non-comparable
+ * Constructs an IntermediateRecord by extracting the order-by values from
the record.
*/
private IntermediateRecord getIntermediateRecord(Key key, Record record) {
- Comparable[] intermediateRecordValues = new
Comparable[_numOrderByExpressions];
+ Comparable[] orderByValues = new Comparable[_numOrderByExpressions];
for (int i = 0; i < _numOrderByExpressions; i++) {
- intermediateRecordValues[i] = _orderByValueExtractors[i].extract(record);
+ orderByValues[i] = _orderByValueExtractors[i].extract(record);
}
- return new IntermediateRecord(key, intermediateRecordValues, record);
+ return new IntermediateRecord(key, record, orderByValues);
}
/**
- * Trim recordsMap to trimToSize, based on order by information
- * Resize only if number of records is greater than trimToSize
- * The resizer smartly chooses to create PQ of records to evict or records
to retain, based on the number of
- * records and the number of records to evict
+ * Resizes the recordsMap to the given size.
*/
- public Map<Key, Record> resizeRecordsMap(Map<Key, Record> recordsMap, int
trimToSize) {
- int numRecordsToEvict = recordsMap.size() - trimToSize;
- if (numRecordsToEvict > 0) {
- // TODO: compare the performance of converting to IntermediateRecord vs
keeping Record, in cases where we do
- // not need to extract final results
- if (numRecordsToEvict < trimToSize) {
- // num records to evict is smaller than num records to retain
- // make PQ of records to evict
- PriorityQueue<IntermediateRecord> priorityQueue =
- convertToIntermediateRecordsPQ(recordsMap, numRecordsToEvict,
_intermediateRecordComparator);
- for (IntermediateRecord evictRecord : priorityQueue) {
- recordsMap.remove(evictRecord._key);
- }
- return recordsMap;
- } else {
- // num records to retain is smaller than num records to evict
- // make PQ of records to retain
- // TODO - Consider reusing the same map by removing record from the map
- // at the time it is evicted from PQ
- Map<Key, Record> trimmedRecordsMap;
- if (recordsMap instanceof ConcurrentMap) {
- // invoked by ConcurrentIndexedTable
- trimmedRecordsMap = new ConcurrentHashMap<>();
- } else {
- // invoked by SimpleIndexedTable
- trimmedRecordsMap = new HashMap<>();
- }
- Comparator<IntermediateRecord> comparator =
_intermediateRecordComparator.reversed();
- PriorityQueue<IntermediateRecord> priorityQueue =
- convertToIntermediateRecordsPQ(recordsMap, trimToSize, comparator);
- for (IntermediateRecord recordToRetain : priorityQueue) {
- trimmedRecordsMap.put(recordToRetain._key, recordToRetain._record);
- }
- return trimmedRecordsMap;
+ public void resizeRecordsMap(Map<Key, Record> recordsMap, int size) {
+ int numRecordsToEvict = recordsMap.size() - size;
+ if (numRecordsToEvict <= 0) {
+ return;
+ }
+ if (numRecordsToEvict <= size) {
+ // Fewer records to evict than retain, make PQ of records to evict
+ PriorityQueue<IntermediateRecord> priorityQueue =
+ convertToIntermediateRecordsPQ(recordsMap, numRecordsToEvict,
_intermediateRecordComparator);
+ for (IntermediateRecord recordToEvict : priorityQueue) {
+ recordsMap.remove(recordToEvict._key);
+ }
+ } else {
+ // Fewer records to retain than evict, make PQ of records to retain
+ Comparator<IntermediateRecord> comparator =
_intermediateRecordComparator.reversed();
+ PriorityQueue<IntermediateRecord> priorityQueue =
convertToIntermediateRecordsPQ(recordsMap, size, comparator);
Review comment:
```
PriorityQueue<IntermediateRecord> priorityQueue =
convertToIntermediateRecordsPQ(recordsMap, size,
_intermediateRecordComparator.reversed());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
