RFR 8222289: Overhaul logic for reading/writing constant pool entries

[Maurizio Cimadamore]

Hi,
this patch significantly rewrites the way in which constant pool entries 
are read/written/handled by javac. A good description of the issues in 
the current architecture is given in the JBS entry [4].

Webrev:

http://cr.openjdk.java.net/~mcimadamore/8222289/

A crucial part of the solution is the new PoolConstant interface. This 
is an interface that all javac entities that want to be written onto the 
constant pool must adhere to. The interface has methods for obtaining a 
pool tag, and, crucially, to obtain a unique key with which the entity 
can be stored in the pool data structure, so as to quickly check for 
duplicates.

I've also defined a sub-interface of the PoolConstant interface called 
LoadableConstant, which represents the pool constants that can be loaded 
with ldc, and stored as static args to bootstrap methods in indy/condy.

DynamicMethodSymbol now has been rewritten to express static argument 
list using LoadableConstant, rather than Object.

This patch takes care of most issues in the existing architecture; most 
notably, thanks to PoolReader, most of the duplication in 
ModuleNameReader can be removed. Also, I've took great care in removing 
all eager conversions to pool indices ahead of instruction writing 
(which then resulted in back-requests to the pool in order to fetch the 
entry at given address from the Code class). To do this, I tweaked the 
Code class a bit, so that all methods for emitting opcodes will now take 
a pool constant object (which can be null if an instruction does not 
refer to the pool).

Some changes were also necessary in Items, more specifically, in order 
to represent the value of an ImmediateItem as a LoadableConstant, rather 
than just an Object.

The PoolWriter/Reader API is mostly straightforward, with a bunch of 
putXYZ/getXYZ methods, where XYZ is some Javac entity that needs to be 
written/read. Among the things to note, PoolWriter needs some special 
logic to handle the case where a pool entry write request is generated 
in the middle of another request (a queue has been used to deal with 
this). The PoolReader uses the same two-phase index + resolution  
approach used currently by ClassReader (resolving all entries eagerly is 
not only a performance nightmare, since not all entries will be used, 
but also a source of bugs, since javac is very sensitive to when entries 
- esp. those for classes - are turned into real symbols).

Extensive testing has been carried out. All relevant tests pass on the 
new code. In addition to that, I also compared the output of javac 
before/after the patch and made sure the results were the same. Since 
the output in general is different (the CP indices before/after are 
different), I have put together a javap patch [5] which stabilizes the 
javap output by normalizing all CP indices. This has proven to be a very 
useful tool, and we might pursue that on a followup change.

Performance testing has also been carried out using our internal tools - 
all results seem to indicate that, at worst, the new code is 1.5% slower 
than the old one. Given that the new code is significantly more 
expressive than the old one, I think the performance difference is 
totally acceptable. Furthermore, when value types will be available I 
expect a lot of the cost associated with boxing constants (e.g. from 
String to LoadableConstant) to fade away.


Alternatives considered:

* Model the constant pool with a proper hierarchy of classes each of 
which models a different CP entry. This approach has been tried in 
Valhalla (see [1], [2] and [3]), but ultimately it was rejected because 
it was not possible to make it perform in a way that was satisfactory. 
Also, when reading the pool, fully materializing the entries (even the 
unused ones) was prohibitive, and was avoided (even in the Valhalla 
branch) in order to have acceptable performances.

* Use the new ConstantDesc; while this approach has the potential to get 
rid of the nasty boxing for primitive constants (thanks to the fact that 
e.g. String already implements ConstantDesc), the ConstantDesc API is 
not sufficiently expressive for modeling an entire constant pool. First, 
not all entries are modeled (e.g. only loadable constant are, by 
design); secondly, there's no way to query e.g. the pool tag given a 
ConstantDesc. We might revise the implementation should some of these 
issues be addressed in future releases.

Cheers
Maurizio


[1] - 
http://hg.openjdk.java.net/valhalla/valhalla9/langtools/file/85cc92a65da8/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/PoolWriter.java
[2] - 
http://hg.openjdk.java.net/valhalla/valhalla9/langtools/file/85cc92a65da8/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/PoolReader.java
[3] - 
http://hg.openjdk.java.net/valhalla/valhalla9/langtools/file/85cc92a65da8/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Pool.java
[4] - https://bugs.openjdk.java.net/browse/JDK-8222289
[5] - http://cr.openjdk.java.net/~mcimadamore/x-javapStableOutput_v3/