I think the point Chris is raising is a bit more subtle.
The VM has a notion of classes in which final fields are trusted - when
that happens, the value of final fields are constant folded by the JIT
(yayy!).
There is a problem though: in practice, final fields can be mutated e.g.
by reflection, or via 292 (var handles) or via Unsafe.
For this reason, when we trust final fields in class C, we typically do
other changes to prevent fields in class C from being exposed (e.g. via
reflection), so as to decrease the potential that JIT optimization would
be invalidated, either accidentally or maliciously.
I think the current state of affair for records has at least 3 issues:
* The paths which controls which fields are trusted by VM and which
fields are exposed via reflection/292/unsafe do not align 100%. For
instance, Unsafe refuses to give up field offset based on the stricter
Class::isRecord definition, thus creating a safety gap.
* In an attempt to prevent changes to records fields via reflection, the
specification of Field::set was updated to say that if the field holder
is a record, the field update will fail. This is good, if it weren't
that the javadoc has to define what it means for a field holder to be a
record; and the way that's done is by referring to Class::isRecord,
which is not what the implementation does (the implementation uses the
broader "isRecord" definition based on the presence/absence of the
record attribute - the same used by the VM).
* All clients have now a way to enable the (fragile) final field
optimization: generate a class, then attach a record attribute to it
(e.g. with ASM). The VM will trust all fields in that class no matter
what. I think this is probably not what was originally intended by the
changes in JDK-8247444. In other words, we now have a mechanism which is
kind of like the internal @ForceInline annotation - not as handy as an
annotation perhaps, but much more public (because, any classfile can be
augmented to contain an extra, maybe empty, record attribute).
So, the way I see it, is that there are few ways out of this conundrum:
1) Back away from final field optimization for records; we could in fact
get rid of all the asymmetries described above by simply _not_ trusting
record fields - at which point VM support would become again very minimal.
2) Enforce a definition of "is a record" which makes sense - and then
enable extra optimizations on top. Since we need to be able to specific
reflection restriction (Field::set) I think the only defensible
definition for what constitutes a record is to delegate to
Class::isRecord. This doesn't mean that the JVM will have to start
taking into account the JLS definition of a record at verification time
- it simply means that the final field optimization, instead of being
gated on the mere _presence_ of the record attribute, it would be
governed by a more complex definition which matches the behavior of
Class::isRecord. Note that this is all below the JVMS surface.
What Chris is saying is (I think) that the current implementation is
caught in some inconsistent place between (1) and (2) - a place where,
on one hand, we claim VM neutrality w.r.t. the record definition
contained in the JLS but where, at the same time, we apply extra
optimizations to speed up access to record fields.
Maurizio
On 11/11/2020 02:39, Dan Smith wrote:
On Nov 10, 2020, at 1:51 PM, Chris Hegarty <chris.hega...@oracle.com> wrote:
My issue is with how the VM determines whether a field is trusted or not. The
VM trusts fields in (among other types) “record” classes. So what is a record
class to the VM? (that is the question that I am trying to resolve) - the
answer is not in the JVMS ( which is fine ).
Suggestion: the VM shouldn't bother to provide any definition for "record
class". That's up to Java language compilers and reflection (which should be in
agreement).
Instead, can't we say the VM trusts fields in classes that have a Record attribute? Who cares
whether those classes are "real" records or not? (I may be missing something because I
don't fully understand the concept of "trusted final field", but it seems to me like
HotSpot has a lot of freedom to decide to trust or not trust whatever fields it wants.)
