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

Implement class relationship recording and validating #7089

Merged
merged 5 commits into from Sep 16, 2019

Conversation

@sharon-wang
Copy link
Contributor

commented Sep 16, 2019

Store class relationships to delay verifying class compatibility (child, parent relationships) until triggered by class loading, instead of verifying compatibility upfront. Validate relationships for a class during class initialization, throwing a Verify Error if validation fails.

Enable using -XX:[+/-]ClassRelationshipVerifier. Cannot be used with -Xfuture.
Documentation issue: eclipse/openj9-docs#360

Testing will be added in a future PR.

sharon-wang and others added 4 commits Sep 13, 2019
Store class relationships (child and its superclass/superinterface
relationships) in a hash table (saved in the class loader) instead of
loading the classes and verifying them upfront.

Each child (source class) table entry contains a list of its parents
(target classes), with parent nodes being added as they are
encountered.

Enable feature using `-XX:+ClassRelationshipVerifier`.
Disable feature using `-XX:-ClassRelationshipVerifier`.

Throw error if `-Xfuture` is used with `-XX:+ClassRelationshipVerifier`.
Note that `-Xverify:all` also enables `-Xfuture`.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
Currently, the code always adds the relationship even if it has already
been added.  This can lead to very long lists of parents which delays
processing them.

The insight here is that by keeping the list in the order of the
classname length, we can more quickly weed out duplicates as we only
need to walk the list until there is a longer name, or the same name
already present, before inserting.

Additionally, avoid the parent node allocation until we know we’re
adding something to the list.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
When a class is being initialized, validate that it holds the expected
relationship with each parent in its list.

Free memory for the child hash table entry and parent nodes once
validation is complete.

Throw java/lang/VerifyError if validation of class relationships fails.

Note that this validation check can be moved to any point prior to
a class being made available.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
Instead of allocating a parent node for Throwable, set a bit in the
childEntry flags J9RELATIONSHIP_PARENT_IS_THROWABLE.

During validation, check if the bit is set. If so, retrieve the Throwable
class and check that it is a superclass of the child.

Throwable seems to occur frequently as a parent class, so this change
will avoid the duplicate memory allocations done for Throwable.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
Copy link
Contributor

left a comment

This has previously been reviewed desk-side. Minor formatting changes to the prototype descriptions would be good.

runtime/oti/bcverify_api.h Outdated Show resolved Hide resolved
@DanHeidinga

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

Jenkins test sanity plinux,aix,zlinux jdk8

@DanHeidinga

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

Jenkins test sanity win,osx,xlinux jdk11

@DanHeidinga

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

I see all the tests have passed. @sharon-wang can you push the doc update?

Reduce amount of white space used between parameters and their
descriptions.

Signed-off-by: Sharon Wang <sharon-wang-cpsc@outlook.com>
@sharon-wang sharon-wang force-pushed the sharon-wang:bcvopt branch from 22f4e9c to 96237cb Sep 16, 2019
@sharon-wang

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2019

@DanHeidinga reformatting changes are now pushed

Copy link
Contributor

left a comment

lgtm

@DanHeidinga

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

The travis compile has passed so I won't require another rebuild

@DanHeidinga DanHeidinga merged commit 8d99c64 into eclipse:master Sep 16, 2019
2 checks passed
2 checks passed
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
3 participants
You can’t perform that action at this time.