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

Allow redefinition of Object in fast HCR #4714

Merged
merged 1 commit into from
Feb 21, 2019

Conversation

gacholio
Copy link
Contributor

Allow java.lang.Object to be redefined (or retransformed) in fast HCR
mode (i.e. runtime HCR, not extended HCR used by the java debugger).

Related: #3410

[ci skip]

Signed-off-by: Graham Chapman graham_chapman@ca.ibm.com

Allow java.lang.Object to be redefined (or retransformed) in fast HCR
mode (i.e. runtime HCR, not extended HCR used by the java debugger).

Related: eclipse-openj9#3410

[ci skip]

Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
@gacholio
Copy link
Contributor Author

Note there is currently no testing for this because:

a) Proper testing requires generating class files (more difficult than just writing .java files)
b) Any test would fail at -Xint today (since that implies extended HCR)

@gacholio
Copy link
Contributor Author

jenkins test all plinux jdk8

@gacholio
Copy link
Contributor Author

I have hand-tested the virtual and non-virtual cases for Object (which also covers the private interface method case). Tests will eventually be added to cover all of these cases.

@fjeremic
Copy link
Contributor

@andrewcraik FYI so we can think about whether JIT optimizer makes certain assumptions about java.lang.Object and it's methods.

@andrewcraik
Copy link
Contributor

There are definitely assumptions in some places - we need to prioritize the dev work to do this against other items since it will take a bit of time to find all the possible dependencies.

@gacholio
Copy link
Contributor Author

@andrewcraik I'd call the JIT work low-priority at this point - we already allow Object to be transformed at load time, so any assumptions you're making can already be broken (and the VM has plenty of its own assumptions around Object).

@andrewcraik
Copy link
Contributor

So just caaturing some thoughts discussion related to this:

  • what are we going to do if Object's finalize method gains a body - is the GC going to start running these? what are we going to do with JITCODE that has been produced assuming that no finalization is necessary?
  • the JIT is going to need to stop special casing Object.hashCode and rely on forcing the inlining to get fastIdentityHashCode exposed to get good performance - I think this may be done, but we need to check
  • we will need to worry about Object.clone changing - if clone gains side-effects they may not be respected by JIT optimizations to object cloning to force it inline - we'll need an appropriate guard
    Given the impacts of the above can there at least be some kind of debug option to disable allowing Object redefinition in case of perf or functional problems?

@DanHeidinga
Copy link
Member

For the case of clone / finalize, can we, as you've elegantly suggested in the past, "throw the car battery in the pool" and dump the entire code cache if those methods are redefined?

@andrewcraik
Copy link
Contributor

certainly possible - depends on the perf behaviours that are expected.

@gacholio
Copy link
Contributor Author

The fix is to not recognize the Object methods at all, and recognize the primitives that they call (and we're not allowing to be modified).

For other things like finalize, we will report the modification to the JIT, so it can break assumptions, etc, as needed.

finalize() in particular is interesting, since we use the tag when we allocate. There are several options:

  1. Disallow Object becoming finalizeable

I like this since there's no way we're going to add finalization to arrays.

  1. Allow it, propagate the finalize bits down the hierarchy

2a) New instances are finalizeable

Maybe.

2b) New and existing instances are finalizeable

Seems like a lot of work for nothing.

@gacholio
Copy link
Contributor Author

Note that the things we're talking about here have nothing to do with HCR - Object can be modified in any of the unpleasant ways we've discussed at load time, so we need to handle it in both cases, which means we should remove as many of our hard-coded assumptions as possible.

I'll open a new issue to deal with our expectations for Object.

@gacholio
Copy link
Contributor Author

Please continue any discussion in #4792

Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants