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

Improvements for Relocatable Compilations Part 1 #2918

Closed
wants to merge 5 commits into from

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Sep 18, 2018

Depends on eclipse-omr/omr#2977

This commit introduces the Symbol Validation Manager infrastructure which allows the compiler to run previously (implicitly) disabled optimizations under AOT. The fundamental idea for the Symbol Validation Manager was thought up by @jdmpapin ; the current implementation is a collaboration between myself and @jdmpapin .

Issue tracking all the PRs: #2921

Important point worth noting: all of the new AOT validations are NOT enabled by default; they're guarded by an option.

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 18, 2018

Ugh, I need to take a look at the relocations for the abstract method inlining for P and Z. (The reason I missed this is because Part2 and Part3 are currently only supported on x86, whereas Part1 should work on all platforms)

@andrewcraik
Copy link
Contributor

@dsouzai should this be WIP based on your previous comment?

@fjeremic fjeremic added comp:jit depends:omr Pull request is dependent on a corresponding change in OMR labels Sep 18, 2018
@dsouzai dsouzai changed the title Improvements for Relocatable Compilations Part 1 WIP: Improvements for Relocatable Compilations Part 1 Sep 18, 2018
@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch from 7e37a57 to 376e1ac Compare September 18, 2018 21:08
@dsouzai dsouzai changed the title WIP: Improvements for Relocatable Compilations Part 1 Improvements for Relocatable Compilations Part 1 Sep 18, 2018
@dsouzai dsouzai changed the title Improvements for Relocatable Compilations Part 1 WIP: Improvements for Relocatable Compilations Part 1 Sep 18, 2018
@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch 2 times, most recently from c2e033b to 6084fd6 Compare September 18, 2018 21:38
@dsouzai dsouzai changed the title WIP: Improvements for Relocatable Compilations Part 1 Improvements for Relocatable Compilations Part 1 Sep 18, 2018
@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 18, 2018

@mstoodle @jdmpapin could you please review?

Copy link
Contributor

@jdmpapin jdmpapin left a comment

Choose a reason for hiding this comment

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

Should there be a case for TR_InlinedAbstractMethodWithNopGuard in TR_RelocationRecord::create()?

NULL);
}

if (abstractClass)
Copy link
Contributor

Choose a reason for hiding this comment

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

The method resolution might have failed even if the class resolution succeeded. In that case we would pass J9JIT_INTERP_VTABLE_OFFSET to findSingleAbstractImplementer(), at which point I'm not sure what would happen. It might be safest to check explicitly that both succeeded

@@ -500,7 +500,7 @@ TR_PersistentCHTable::findSingleAbstractImplementer(
{
if (comp->getOption(TR_DisableCHOpts))
return 0;
TR_PersistentClassInfo * classInfo = findClassInfoAfterLocking(thisClass, comp);
TR_PersistentClassInfo * classInfo = findClassInfoAfterLocking(thisClass, comp, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be more conventional to introduce a default parameter, and pass it through here. However, it looks to me like you hit all of the callers except for one in invariant argument preexistence, which I believe (currently) doesn't run for AOT anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I sort of followed what happens in findSingleInterfaceImplementer, which does something similar :P

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 19, 2018

Should there be a case for TR_InlinedAbstractMethodWithNopGuard in TR_RelocationRecord::create()?

You are correct; it's there in a later changeset, but when I logically separated them, I guess I missed the creation... sigh; I"ll make the requested changes.

@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch from 6084fd6 to edda601 Compare September 19, 2018 14:54
@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 19, 2018

Soo... turns out Abstract Method Inlining only worked under AOT because it was running with the new AOT Validations that are introduced in Part 2. So unfortunately I'm going to have to punt over the Abstract Method Inlining change over to Part 2.

(The issue I'm running into is a crash during class validation, which is probably because the cpIndex is -1 or something like that; the cpIndex being -1 isn't a problem after Part 2)

EDIT: I just combined the old Part1 and Part2 PRs into this current PR.

@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch 4 times, most recently from dbe42e8 to 35ab5d0 Compare September 19, 2018 19:24
Irwin D'Souza added 2 commits September 19, 2018 23:02
When the JIT heuristically decides to disable the SelectiveNoOptServer,
it only does so on the JIT Options object. This commit also sets the
option on the AOT Options object.

Signed-off-by: Irwin D'Souza <[email protected]>
If the user specifies higher method counts, don't delay relocation and
make the scount the same as the count.

Signed-off-by: Irwin D'Souza <[email protected]>
@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch 2 times, most recently from 4e8380e to a7218f2 Compare September 21, 2018 18:17
Irwin D'Souza added 2 commits September 21, 2018 15:04
In AOT, the compiler can only konw about classes that can be remembered
(ie, stored in the Shared Class Cache (SCC)). However, for functional
correctness, before loading an AOT compiled body, the code needs to be
validated in order to guarantee that the assumptions made about the java
environment during the compile run are still valid in the load run.

Currently, there are two types of validations; Arbitrary Class
validations and Class From Constant Pool (CP) validations. The former
can only apply to classes the compiler gets by name, when the name is a
string literal (eg a class loaded by the Bootstrap Class Loader). The
latter is used for classes the compiler gets from the CP of another
class.

Many queries in the compiler that get Class by Name have a
default parameter that can be set to vett the callsite for AOT. However,
these validations are not sufficient to provide confidence to vett a
site. The reason is because the query that gets the class by name is
used when the class name string is parsed from the signature of method;
in this case, the class that should be returned can be different
depending on who the "beholder" is. Additionally, if the compiler finds
a class that's not by name and not through a CP (eg string peepholes),
then under the current AOT infrastructure, the class can't be used.

This commit adds the infrastructure to enable validations of classes
regardless of how the compiler acquired them. The infrastructure is
general, in that it can be extended to enable validations of any kind of
symbol that the compiler acquires that may be different or non-existent
in the load run.

The fundamental idea is to store validation records every time a front
end query is made. The record holds information about the symbol
acquired, how it was acquired, and what information is required to
acquire it in the load run. Each symbol is given a unique ID. In the
load run, the AOT validation infrastructure goes through the validation
records and asks the same questions the compiler asked during the AOT
compilation, and if the answers vary in any way, the AOT load fails.
Otherwise, we are guaranteed that the code is entirely valid.

For example, if the method being compiled is A.foo(), then before the
compilation begins, the compiler creates:

1. Root Class Record for A
2. Method From Class Record for foo()

This will result in A getting ID 1, and foo() getting ID 2. As the
compilation continues, if the compiler gets class B in two different
ways, namely once by name and once via CP, then two other records get
created:

3. Class by Name for B
4. Class from CP for B

and B gets assigned ID 3.

In the AOT load run, all the AOT infrastructure can see are IDs.
Therefore, it has to reconstruct the symbols from these IDs. First it
goes through the Root Class Record, and determines what Class A is in
the new environment. Then it goes through the Method From Class Record
and searches for method foo(). Then it goes through the Class by Name
record and determines what B is. Then it goes through the Class from CP
record, and validates that the class it gets from the CP is the same as
the class it got by name, since in the compile run, those two queries
yielded the same answer. If after going through all the records
everything stays consistent, then the load passes.

An added subtlety that was not described in the example above is that
whenever a class record is created, a class chain record is also
created. The reason for this is because AOT can only work with classes
that can be remembered. Additionally, the class chains ensure that the
shape of the class is the same from run to run.

Another subtlety comes from array classes. Because array classes don't
have a unique ROMClass, the validation manager remembers the leaf
component class, and stores as many Array Class from Component Class
records as there are dimensions of the array class being validated.

Signed-off-by: Irwin D'Souza
The Symbol Validation Manager is used to remember all the different ways
the compiler acquired a symbol. However, there are regions where this
information isn't necessary for the load run. For example, when asking
for something from the Persistent CH Table, all that matters is the
input to the query and the output; however, the query involves going
through multiple classes before finally returning the answer. If the
compiler were to store valiations for everything that occurs during this
query, it would require consistency between the compile and load run for
more than what is only required for the compilation.

Therefore, this commit adds the means to denote regions that are not
strictly necessary to validate for the load run.

Signed-off-by: Irwin D'Souza <[email protected]>
@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch from a7218f2 to 71dd928 Compare September 21, 2018 19:06
@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 24, 2018

@mstoodle friendly reminder for review :)

@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch 2 times, most recently from 173b6ad to 4aea1c2 Compare September 25, 2018 19:44
This commit adds uses of the new Symbol Validation Manager infrastructure
to do AOT validations (under an option). It also introduces the
following two changes:

1. Use resolved interpreter dispatch for AOT

Previously, because almost everything was "unresolved" under AOT, all
calls would use the unresolved dispatch. However, with the new
validations, very few unresolved pointers are only unresolved because of
AOT. Therefore, generating an unresolved dispatch is incorrect.

The solution to this problem is to assume, during an AOT compile, that
all methods are interpreted. Thus, if a method is resolved, the compiler
will generate a resolved interpreter dispatch snippet, which can patch
the caller if/when the method is compiled.

2. Fix ObjectAllocationInlining under AOT

Previously, Object Allocation Inlining under AOT maintained certain
assumtpions about the trees that was only true if the class was
unresolved. This commit fixes those assumptions.

Signed-off-by: Irwin D'Souza
@dsouzai dsouzai force-pushed the AOTImprovementsPart1 branch from 4aea1c2 to f6b546f Compare September 27, 2018 15:31
@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 28, 2018

In the interest of time, I'm closing this PR since #2919 will contain all the relocatable compilation improvements.

@dsouzai dsouzai closed this Sep 28, 2018
@dsouzai dsouzai deleted the AOTImprovementsPart1 branch October 15, 2019 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jit depends:omr Pull request is dependent on a corresponding change in OMR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants