Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12490#discussion_r60264341
  
    --- Diff: 
core/src/main/java/org/apache/spark/shuffle/sort/ShuffleInMemorySorter.java ---
    @@ -47,18 +48,25 @@ public int compare(PackedRecordPointer left, 
PackedRecordPointer right) {
       private LongArray array;
     
       /**
    +   * Whether to use radix sort for sorting in-memory partition ids. Radix 
sort is much faster
    +   * but requires additional memory to be reserved memory as pointers are 
added.
    +   */
    +  private final boolean useRadixSort;
    +
    +  /**
        * The position in the pointer array where new records can be inserted.
        */
       private int pos = 0;
     
       private int initialSize;
     
    -  ShuffleInMemorySorter(MemoryConsumer consumer, int initialSize) {
    +  ShuffleInMemorySorter(MemoryConsumer consumer, int initialSize, boolean 
useRadixSort) {
         this.consumer = consumer;
         assert (initialSize > 0);
         this.initialSize = initialSize;
    -    this.array = consumer.allocateArray(initialSize);
    +    this.array = consumer.allocateArray(initialSize * (useRadixSort ? 2 : 
1));
    --- End diff --
    
    Isn't it easier to encode the `2` value (which comes back in a lot of 
forms) as a constant instead of a ternary expression. Something like 
`memoryAllocationFactor`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to