[GitHub] spark pull request #19222: [SPARK-10399][CORE][SQL] Introduce multiple Memor...

[viirya <gi...@git.apache.org>]

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

    https://github.com/apache/spark/pull/19222#discussion_r179313290
  
    --- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java ---
    @@ -45,38 +45,162 @@
        */
       public static final int FREED_IN_ALLOCATOR_PAGE_NUMBER = -3;
     
    -  private final long length;
    +  @Nullable
    +  protected Object obj;
    +
    +  protected long offset;
    +
    +  protected long length;
     
       /**
        * Optional page number; used when this MemoryBlock represents a page allocated by a
    -   * TaskMemoryManager. This field is public so that it can be modified by the TaskMemoryManager,
    -   * which lives in a different package.
    +   * TaskMemoryManager. This field can be updated using setPageNumber method so that
    +   * this can be modified by the TaskMemoryManager, which lives in a different package.
        */
    -  public int pageNumber = NO_PAGE_NUMBER;
    +  private int pageNumber = NO_PAGE_NUMBER;
     
    -  public MemoryBlock(@Nullable Object obj, long offset, long length) {
    -    super(obj, offset);
    +  protected MemoryBlock(@Nullable Object obj, long offset, long length) {
    +    if (offset < 0 || length < 0) {
    +      throw new ArrayIndexOutOfBoundsException(
    --- End diff --
    
    nit: seems not really array index out of bounds? Maybe an `IllegalArgumentException`?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org