Github user Ngone51 commented on a diff in the pull request:
https://github.com/apache/spark/pull/19222#discussion_r172018886
--- Diff:
common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java ---
@@ -17,47 +17,168 @@
package org.apache.spark.unsafe.memory;
-import javax.annotation.Nullable;
-
import org.apache.spark.unsafe.Platform;
+import javax.annotation.Nullable;
+
/**
- * A consecutive block of memory, starting at a {@link MemoryLocation}
with a fixed size.
+ * A declaration of interfaces of MemoryBlock classes .
*/
-public class MemoryBlock extends MemoryLocation {
+public abstract class MemoryBlock {
+ @Nullable
+ protected final Object obj;
- private final long length;
+ protected final long offset;
+
+ protected final 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 = -1;
+ private int pageNumber = -1;
public MemoryBlock(@Nullable Object obj, long offset, long length) {
- super(obj, offset);
+ this.obj = obj;
+ this.offset = offset;
this.length = length;
}
+ public MemoryBlock() {
+ this(null, 0, 0);
+ }
+
+ public final Object getBaseObject() {
--- End diff --
>@Ngone51 could you please elaborate on your idea about MemoryLocation
while we do not have MemoryLocation now? I think that it would be good to have
a final method for getBaseObject for performance.
Hi, @kiszk. Actually, this is a comment follow the discussion between
@cloud-fan & @viirya :
> @cloud-fan : we need some document about it. Now we have the general
MemoryBlock abstraction, do we still need to expose the baseObj and baseOffset
concept?
@cloud-fan : BTW if we really need to expose them, shall we expose
MemoryLocation instead?
@viirya Sounds reasonable. Should we still need MemoryLocation to expose a
logical memory location?
ï¼I have no idea why those comments hideï¼
And my suggestion is something like:
```
public abstract class MemoryBlock {
MemoryLocation location;
...
}
```
rather than the original path:
```
public class MemoryBlock extends MemoryLocation {
...
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
