Trust final fields in records

[Christoph Dreis]

Hi,

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:

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.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   Units  
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.

Cheers,
Christoph

===== 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);