vinothchandar commented on a change in pull request #2231:
URL: https://github.com/apache/hudi/pull/2231#discussion_r528441170
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/util/ObjectSizeCalculator.java
##########
@@ -384,9 +383,69 @@ public int getArrayHeaderSize() {
@Override
public int getObjectHeaderSize() {
+ return 4;
+ }
+
+ @Override
+ public int getObjectPadding() {
+ return 4;
+ }
+
+ @Override
+ public int getReferenceSize() {
+ return 4;
+ }
+
+ @Override
+ public int getSuperclassFieldPadding() {
+ return 4;
+ }
+ };
+ } else {
+ // it's a 64-bit uncompressed references object model
+ return new MemoryLayoutSpecification() {
+ @Override
+ public int getArrayHeaderSize() {
+ return 16;
+ }
+
+ @Override
+ public int getObjectHeaderSize() {
+ return 16;
+ }
+
+ @Override
+ public int getObjectPadding() {
+ return 8;
+ }
+
+ @Override
+ public int getReferenceSize() {
+ return 8;
+ }
+
+ @Override
+ public int getSuperclassFieldPadding() {
+ return 8;
+ }
+ };
+ }
+ } else {
Review comment:
Guess this was the original code. that only handled hotspot vm
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/util/ObjectSizeCalculator.java
##########
@@ -328,54 +328,53 @@ private static long getPrimitiveFieldSize(Class<?> type) {
static MemoryLayoutSpecification getEffectiveMemoryLayoutSpecification() {
final String vmName = System.getProperty("java.vm.name");
if (vmName == null || !(vmName.startsWith("Java HotSpot(TM) ") ||
vmName.startsWith("OpenJDK")
- || vmName.startsWith("TwitterJDK"))) {
- throw new UnsupportedOperationException("ObjectSizeCalculator only
supported on HotSpot VM");
+ || vmName.startsWith("TwitterJDK") || vmName.startsWith("Eclipse
OpenJ9"))) {
+ throw new UnsupportedOperationException("ObjectSizeCalculator only
supported on HotSpot or Eclipse OpenJ9 VMs");
}
- final String dataModel = System.getProperty("sun.arch.data.model");
- if ("32".equals(dataModel)) {
- // Running with 32-bit data model
- return new MemoryLayoutSpecification() {
- @Override
- public int getArrayHeaderSize() {
- return 12;
- }
+ final String strVmVersion = System.getProperty("java.vm.version");
+ // Support for OpenJ9 JVM
+ if (strVmVersion.startsWith("openj9")) {
+ final String dataModel = System.getProperty("sun.arch.data.model");
+ if ("32".equals(dataModel)) {
+ // Running with 32-bit data model
+ return new MemoryLayoutSpecification() {
Review comment:
Not directly related to this PR. But given we are now actually evolving
this code. Does it make sense to declare all the `MemoryLayoutSpecification`
objects as descriptively named classes? That way this method is very readable.
Food for thought.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
