Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding DDR support for flattened arrays #7614

Merged

Conversation

@XxAdi101xX
Copy link
Contributor

XxAdi101xX commented Oct 28, 2019

Adding DDR support for flattened arrays

This PR provides support for running the !j9object command on flattened arrays and provides support for running !flatobject on arrays in general.

Signed-off-by: Adithya Venkatarao adi_101@live.com

@XxAdi101xX XxAdi101xX closed this Oct 29, 2019
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:ddr-flattened-class-cache-entry-bugfix branch from acb42d4 to 4d54631 Oct 29, 2019
@XxAdi101xX XxAdi101xX reopened this Oct 31, 2019
@XxAdi101xX XxAdi101xX marked this pull request as ready for review Nov 1, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 1, 2019

For the examples, we will use this struct

    struct J9Class* clazz = !j9arrayclass 0x1699500   // [L
    Object flags = 0x00000000;
    U_32 size = 0x00000004;
	[0] = !fj9object 0xfffffffffffdc657 = !j9object 0x7ffee32b8
	[1] = !fj9object 0xfffffffffffdc462 = !j9object 0x7ffee2310
	[2] = !fj9object 0xfffffffffffdbecf = !j9object 0x7ffedf678
	[3] = !fj9object 0xfffffffffffdc66a = !j9object 0x7ffee3350

where the first three objects are value types and the fourth object is a flattened array that contains these the three object fields in that specified order.

Running !j9object 0x7ffee3350 shows

    struct J9Class* clazz = !j9arrayclass 0x19BFB00   // [L
    Object flags = 0x00000000;
    U_32 size = 0x00000003;
	[0] = !fj9object 0xffffffffffeeffee = !j9object 0x7ffee3350 [0]
	[1] = !fj9object 0xffffffffffeeffee = !j9object 0x7ffee3350 [1]
	[2] = !fj9object 0xffffffffffeeffee = !j9object 0x7ffee3350 [2]
}

where running !j9object 0x7ffee3350 [1] for example, gives

!J9Object 0x00000007FFEE3350 [1] {
	struct J9Class* clazz = !j9class 0x19BF600 // AssortedValueWithSingleAlignment
	Object flags = 0x00000000;
	QTriangle2D; tri = !j9object 0x7ffee3350 [1].tri (offset = 0) (Triangle2D)
	QPoint2D; point = !j9object 0x7ffee3350 [1].point (offset = 48) (Point2D)
	QFlattenedLine2D; line = !j9object 0x7ffee3350 [1].line (offset = 56) (FlattenedLine2D)
	QValueInt; i = !j9object 0x7ffee3350 [1].i (offset = 72) (ValueInt)
	QValueFloat; f = !j9object 0x7ffee3350 [1].f (offset = 76) (ValueFloat)
	QTriangle2D; tri2 = !j9object 0x7ffee3350 [1].tri2 (offset = 80) (Triangle2D)
}

and you can further look into each object value. For example, !j9object 0x7ffee3350 [1].i shows

!J9Object 0x00000007FFEE3350 [1].i {
	// EA = 0x00000007FFEE3420, offset in top level container = 80;
	struct J9Class* clazz = !j9class 0x19AD800 // ValueInt
	Object flags = 0x00000000;
	I i = 0x00000008 (offset = 0) (ValueInt)
}

while !j9object 0x7ffee3350 [1].line will display

!J9Object 0x00000007FFEE3350 [1].line {
	// EA = 0x00000007FFEE3410, offset in top level container = 64;
	struct J9Class* clazz = !j9class 0x19B0E00 // FlattenedLine2D
	Object flags = 0x00000000;
	QPoint2D; st = !j9object 0x7ffee3350 [1].line.st (offset = 0) (Point2D)
	QPoint2D; en = !j9object 0x7ffee3350 [1].line.en (offset = 8) (Point2D)
}

which can be further navigated into. I've also set different values for some of the fields in the first and second objects respectively and ensured that the !J9Object 0x00000007FFEE3350 [0].field and !J9Object 0x00000007FFEE3350 [0].field show the proper values

@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 1, 2019

Running !flatobject on an array will have the same behaviour of printing all the fields down to its lowest level. It simply just recursively calls itself until !j9object can be applied on the individual fields.

@XxAdi101xX XxAdi101xX changed the title Adding DDR support for flattened value types in arrays Adding DDR support for flattened arrays Nov 1, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 1, 2019

@tajila I've completed the implementation for !j9object on flattened arrays and !flatobject on arrays.

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:ddr-flattened-class-cache-entry-bugfix branch 2 times, most recently from 0e4e321 to 7caf975 Nov 1, 2019
runtime/oti/j9nonbuilder.h Outdated Show resolved Hide resolved
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:ddr-flattened-class-cache-entry-bugfix branch 2 times, most recently from 6ed5411 to 67f2d05 Nov 4, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 14, 2019

@tajila I've address all the pr comments

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:ddr-flattened-class-cache-entry-bugfix branch from a6a808d to e06570f Nov 19, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 22, 2019

@tajila I've addressed the comments, please take a look

@tajila
tajila approved these changes Nov 22, 2019
@tajila

This comment has been minimized.

Copy link
Contributor

tajila commented Nov 22, 2019

@keithc-ca Please review these changes

@tajila

This comment has been minimized.

Copy link
Contributor

tajila commented Nov 22, 2019

@XxAdi101xX Please add an example of what !flatobject outputs similar to your !jobject examples

@tajila

This comment has been minimized.

Copy link
Contributor

tajila commented Nov 22, 2019

@XxAdi101xX Also, does the command still output [0] = !fj9object 0xfffffffffffdc657 = !j9object 0x7ffee32b8 [0] or just [0] = !j9object 0x7ffee32b8 [0]

@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 22, 2019

@keithc-ca @tajila temporarily hold off on the merge please, it seems like !flatobject has broke after my most recent change, currently looking into it

@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 22, 2019

Running flatobject 0x00000007FFECC870 [0] yields

!flatobject 0x00000007FFECC870 [0] {
	// EA = 0x00000007FFECC878, offset in top level container = 8;
	struct J9Class* clazz = !j9class 0x137FE00 // AssortedValueWithSingleAlignment
	Object flags = 0x00000000;
	Triangle2D tri { // EA = 0x00000007FFECC878 (offset = 0)
		FlattenedLine2D v1 { // EA = 0x00000007FFECC878 (offset = 0)
			Point2D st { // EA = 0x00000007FFECC878 (offset = 0)
				I x = 0xFFEEFFEE
				I y = 0xAABBAABB
			}
			Point2D en { // EA = 0x00000007FFECC880 (offset = 8)
				I x = 0xCCDDCCDD
				I y = 0x33443344
			}
		}
		FlattenedLine2D v2 { // EA = 0x00000007FFECC888 (offset = 16)
			Point2D st { // EA = 0x00000007FFECC888 (offset = 0)
				I x = 0xCCDDCCDD
				I y = 0x33443344
			}
			Point2D en { // EA = 0x00000007FFECC890 (offset = 8)
				I x = 0x43211234
				I y = 0xABCDDCBA
			}
		}
		FlattenedLine2D v3 { // EA = 0x00000007FFECC898 (offset = 32)
			Point2D st { // EA = 0x00000007FFECC898 (offset = 0)
				I x = 0xFFEEFFEE
				I y = 0xAABBAABB
			}
			Point2D en { // EA = 0x00000007FFECC8A0 (offset = 8)
				I x = 0x43211234
				I y = 0xABCDDCBA
			}
		}
	}
	Point2D point { // EA = 0x00000007FFECC8A8 (offset = 48)
		I x = 0xFFEEFFEE
		I y = 0xAABBAABB
	}
	FlattenedLine2D line { // EA = 0x00000007FFECC8B0 (offset = 56)
		Point2D st { // EA = 0x00000007FFECC8B0 (offset = 0)
			I x = 0xFFEEFFEE
			I y = 0xAABBAABB
		}
		Point2D en { // EA = 0x00000007FFECC8B8 (offset = 8)
			I x = 0xCCDDCCDD
			I y = 0x33443344
		}
	}
	ValueInt i { // EA = 0x00000007FFECC8C0 (offset = 72)
		I i = 0x7FFFFFFF
	}
	ValueFloat f { // EA = 0x00000007FFECC8C4 (offset = 76)
		F f = 0x7F7FFFFF
	}
	Triangle2D tri2 { // EA = 0x00000007FFECC8C8 (offset = 80)
		FlattenedLine2D v1 { // EA = 0x00000007FFECC8C8 (offset = 0)
			Point2D st { // EA = 0x00000007FFECC8C8 (offset = 0)
				I x = 0xFFEEFFEE
				I y = 0xAABBAABB
			}
			Point2D en { // EA = 0x00000007FFECC8D0 (offset = 8)
				I x = 0xCCDDCCDD
				I y = 0x33443344
			}
		}
		FlattenedLine2D v2 { // EA = 0x00000007FFECC8D8 (offset = 16)
			Point2D st { // EA = 0x00000007FFECC8D8 (offset = 0)
				I x = 0xCCDDCCDD
				I y = 0x33443344
			}
			Point2D en { // EA = 0x00000007FFECC8E0 (offset = 8)
				I x = 0x43211234
				I y = 0xABCDDCBA
			}
		}
		FlattenedLine2D v3 { // EA = 0x00000007FFECC8E8 (offset = 32)
			Point2D st { // EA = 0x00000007FFECC8E8 (offset = 0)
				I x = 0xFFEEFFEE
				I y = 0xAABBAABB
			}
			Point2D en { // EA = 0x00000007FFECC8F0 (offset = 8)
				I x = 0x43211234
				I y = 0xABCDDCBA
			}
		}
	}
}

Similarily, just typing flatobject 0x00000007FFECC870 will respectively print the contents of flatobject 0x00000007FFECC870 [0], flatobject 0x00000007FFECC870 [1] and flatobject 0x00000007FFECC870 [2]

@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 22, 2019

@tajila @keithc-ca the issue has been resolved, it was a minor two line change change to how the container class was being obtained for the flatobject logic since I had changed the logic when refactoring the code. And @tajila , to answer your question, running !j9object on a flattened array will show [0] = !j9object 0x7ffee32b8 [0] but running the command on a non flattened array will still yield [0] = !fj9object 0xfffffffffffdc657 = !j9object 0x7ffee32b8 [0]

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:ddr-flattened-class-cache-entry-bugfix branch 2 times, most recently from af6505e to 6b0f921 Nov 22, 2019
break;

if (J9ClassHelper.isArrayClass(clazz)) {
int index = Integer.parseInt(nestingHierarchy[0].substring(1, nestingHierarchy[0].length() - 1));

This comment has been minimized.

Copy link
@keithc-ca

keithc-ca Nov 25, 2019

Member

Where is the check that the string nestingHierarchy[0] is of the form [\d+] so we know we're parsing something reasonable?

This comment has been minimized.

Copy link
@XxAdi101xX

XxAdi101xX Nov 25, 2019

Author Contributor

So at this point, would it be better to have a check and then throw an IllegalArgumentException if it's not a in form of an int since it would be due to an error by the DDR user?

This comment has been minimized.

Copy link
@keithc-ca

keithc-ca Nov 25, 2019

Member

This sort of impedance mismatch makes me wonder whether it's time to replace
String[] nestingHierarchy with a better context representation.

This comment has been minimized.

Copy link
@XxAdi101xX

XxAdi101xX Nov 25, 2019

Author Contributor

Rather, what I could do is, instead of calling extraArgs[0].startsWith("[") like you suggested, I could do the Pattern match at that very top level before the PrintObjectFields function is even called and then just display an error message there if the arguments are ill-formatted. At the current point, if the nestingHierarchy[0] isn't in the form [\d], our earlier call on findNestedClassHierarchy would not have executed the appropriate logic since the pattern match done in the ValueTypeHelper.java would have returned false hence we'd already been in an invalid state.

@keithc-ca keithc-ca self-assigned this Nov 25, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 25, 2019

@keithc-ca I've addressed most comments, what is your suggestion for the issue regarding the nestingHierarchy[0] check when we parse for an integer?

@keithc-ca

This comment has been minimized.

Copy link
Member

keithc-ca commented Nov 25, 2019

I think we can deal with the 'impedance mismatch' issue separately, but it ought to be given high priority.

@keithc-ca

This comment has been minimized.

Copy link
Member

keithc-ca commented Nov 25, 2019

Please squash before I launch PR testing. Thanks.

This PR provides support for running the !j9object command on flattened arrays and provides support for running !flatobject on arrays in general.

Signed-off-by: Adithya Venkatarao <adi_101@live.com>
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:ddr-flattened-class-cache-entry-bugfix branch from 567c4fc to 8326765 Nov 25, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Nov 25, 2019

@keithc-ca commits have been squashed

@keithc-ca

This comment has been minimized.

Copy link
Member

keithc-ca commented Nov 26, 2019

Jenkins test sanity,extended xlinux jdk11

1 similar comment
@jdekonin

This comment has been minimized.

Copy link
Contributor

jdekonin commented Nov 26, 2019

Jenkins test sanity,extended xlinux jdk11

@keithc-ca keithc-ca merged commit d648a8d into eclipse:master Nov 27, 2019
8 checks passed
8 checks passed
Build_JDK11_x86-64_linux_Personal Build PASSED
Details
Copyright Check Copyrights appear to be up to date
Details
Line Endings Check Line endings appear to be correct
Details
Pull Request - OpenJ9 All jobs passed
Details
Test_openjdk11_j9_extended.functional_x86-64_linux_Personal Build PASSED
Details
Test_openjdk11_j9_sanity.functional_x86-64_linux_Personal Build PASSED
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
eclipsefdn/eca The author(s) of the pull request is covered by necessary legal agreements in order to proceed!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.