Trust final fields in records

Christoph Dreis christoph.dreis at
Fri Jun 12 20:42:51 UTC 2020

Hi Mandy,


I will try to add tests for the missing pieces over the weekend.


About your concern of calling Class::isRecord. Isn’t that already a problem with Class::isHidden?

After all, Class:isHidden is a native call as well, but is annotated with @HotSpotIntrinsicCandidate.

Is that maybe something which should be added to Class::isRecord as well?





Von: Mandy Chung <mandy.chung at>
Datum: Freitag, 12. Juni 2020 um 22:25
An: Remi Forax <forax at>, Christoph Dreis <christoph.dreis at>
Cc: amber-dev <amber-dev at>, hotspot-runtime-dev <hotspot-runtime-dev at>, valhalla-dev <valhalla-dev at>
Betreff: Re: Trust final fields in records


Hi Remi,

VarHandle does not support write access mode on final fields (see the spec of Lookup::findVarHandle[1] and Lookup::unreflectVarHandle[2]).   I agree and we need new tests to verify it.


Can you please add the tests for VarHandle and sun.misc.Unsafe access to records?  You can reference these tests:


Regarding the patch, I'm a little concerned on the performance impact to check if it's a record as Class::isRecord is currently a JVM native call in particular in UnsafeFieldAccessorFactory and sun.misc.Unsafe.   I can add a MemberName::isTrustedFinalField.  I have to look into ways to avoid the JVM call to check if a Field's declaring class is a record.


On 6/12/20 6:01 AM, Remi Forax wrote:
Hi Christoph,
do you have checked if VarHandle access is disable,
because VarHandle are built on to of jdk.internal.misc.Unsafe and not sun.misc.Unafe ?
otherwise it looks good but Unsafe tests are missing.
----- Mail original -----
De: "Christoph Dreis" <christoph.dreis at>
À: "mandy chung" <mandy.chung at>
Cc: "amber-dev" <amber-dev at>, "hotspot-runtime-dev" <hotspot-runtime-dev at>,
"valhalla-dev" <valhalla-dev at>
Envoyé: Vendredi 12 Juin 2020 08:32:39
Objet: Re: Trust final fields in records
Hi Mandy,
Thanks for taking this. Highly appreciated.
I have attached a patch with the needed changes – I think – on the reflection
side of things with an additional test.
But yeah, I would need help completing the picture in case I’m missing anything.
Von: Mandy Chung <mandy.chung at>
Datum: Freitag, 12. Juni 2020 um 03:57
An: Brian Goetz <brian.goetz at>, Christoph Dreis
<christoph.dreis at>
Cc: Amber dev <amber-dev at>, valhalla-dev
<valhalla-dev at>, "hotspot-runtime-dev at"
<hotspot-runtime-dev at>
Betreff: Re: Trust final fields in records
Hi Christoph,
I can sponsor your patch.  I create
Do you want to contribute to the core reflection change?  I can help too.
On 6/11/20 3:23 PM, Brian Goetz wrote:
Yes, please.
On 6/11/2020 5:49 PM, Mandy Chung wrote:
I really like to see "final fields truly final" at least start with the new
features such as inline classes and records.
Final fields of hidden classes have no write access [1] regardless of the
accessible flag.  I'd propose to make final fields of records and inline
classes non-modifiable in a similar fashion as hidden classes.
On 6/11/20 1:38 PM, Christoph Dreis wrote:
I’ve played around with records the other day and wondered if their (final)
fields could be maybe trusted.
This would allow further optimizations to kick in.
E.g. with the following benchmark:
public class MyBenchmark {
   static final Rectangle rectangle;
   static {
       rectangle = new Rectangle(1, 1);
   record Rectangle(int length, int width) {
       public int size() {
           return length * width;
   @Benchmark public int testSize() { return 1000 / rectangle.size(); }
I see the following results when I apply the attached patch:
Benchmark                                        Mode  Cnt   Score    Error
MyBenchmark.testSizeBefore       avgt   10   3,873 ±  0,044   ns/op
MyBenchmark.testSizePatched     avgt   10   1,740 ±  0,058   ns/op
After all, records state that they are "shallowly immutable" - whatever "
shallowly" means here.
The risk that I see here is that people could still use reflection on records to
change fields - for reasons.
Maybe that aspect could be tightened though before records go non-experimental
in favor of the optimization?
I wonder if this could be considered. If so, I would highly appreciate it if
someone can sponsor the patch.
Let me know what you think.
===== PATCH =====
diff -r 984fde9a0b7f src/hotspot/share/ci/ciField.cpp
--- a/src/hotspot/share/ci/ciField.cpp    Tue Jun 09 16:22:54 2020 +0000
+++ b/src/hotspot/share/ci/ciField.cpp    Thu Jun 11 22:25:02 2020 +0200
@@ -231,6 +231,9 @@
   // Trust final fields in all boxed classes
   if (holder->is_box_klass())
     return true;
+  // Trust final fields in records
+  if (holder->is_record())
+    return true;
   // Trust final fields in String
   if (holder->name() == ciSymbol::java_lang_String())
     return true;
diff -r 984fde9a0b7f src/hotspot/share/ci/ciInstanceKlass.cpp
--- a/src/hotspot/share/ci/ciInstanceKlass.cpp    Tue Jun 09 16:22:54 2020 +0000
+++ b/src/hotspot/share/ci/ciInstanceKlass.cpp    Thu Jun 11 22:25:02 2020 +0200
@@ -64,6 +64,7 @@
   _has_nonstatic_concrete_methods = ik->has_nonstatic_concrete_methods();
   _is_unsafe_anonymous = ik->is_unsafe_anonymous();
   _is_hidden = ik->is_hidden();
+  _is_record = ik->is_record();
   _nonstatic_fields = NULL; // initialized lazily by compute_nonstatic_fields:
   _has_injected_fields = -1;
   _implementor = NULL; // we will fill these lazily
@@ -125,6 +126,7 @@
   _has_injected_fields = -1;
   _is_unsafe_anonymous = false;
   _is_hidden = false;
+  _is_record = false;
   _loader = loader;
   _protection_domain = protection_domain;
   _is_shared = false;
diff -r 984fde9a0b7f src/hotspot/share/ci/ciInstanceKlass.hpp
--- a/src/hotspot/share/ci/ciInstanceKlass.hpp    Tue Jun 09 16:22:54 2020 +0000
+++ b/src/hotspot/share/ci/ciInstanceKlass.hpp    Thu Jun 11 22:25:02 2020 +0200
@@ -57,6 +57,7 @@
   bool                   _has_nonstatic_concrete_methods;
   bool                   _is_unsafe_anonymous;
   bool                   _is_hidden;
+  bool                   _is_record;
     ciFlags                _flags;
   jint                   _nonstatic_field_size;
@@ -200,6 +201,10 @@
     return _is_hidden;
 +  bool is_record() const {
+    return _is_record;
+  }
   ciInstanceKlass* get_canonical_holder(int offset);
   ciField* get_field_by_offset(int field_offset, bool is_static);
    ciField* get_field_by_name(ciSymbol* name, ciSymbol* signature, bool is_static);

More information about the valhalla-dev mailing list