[Java.Interop] ReadOnlyProperty<T>?

[jonpryor]

Context: #1243
Context: #1248

Java's final keyword is contextual, and maps to (at least?) three separate keywords in C#:

• const on fields
• readonly on fields
• sealed on types and methods

When binding fields, we only support "const" final fields: fields for which the value is known at compile-time.

Non-const fields are bound as properties, requiring a lookup for every property access.

This can be problematic, performance-wise, as final fields without a compile-time value only need to be looked up once; afterward, their value cannot change ¹. As such, we should consider altering our binding of "readonly" static properties to cache the value.

PR #1248 implemented a "nullable"-based approach to caching the field value. While this approach works for reference types, it is likely not thread safe for int? and other value types.

There is a comment on #1248 to make the approach thread-safe, but @jonpryor isn't entirely sure if it's correct. The "straightfoward" approach would be to use a C# lock statement, but that requires a GC-allocated lock object, which would increase memory use. Furthermore, if this code is wrong, the only way to fix it is by regenerating the bindings.

@jonpryor considered moving the thread-safety logic into a separate type, moving it outside of the generated code. This is implemented as ReadOnlyProperty<T>, in this commit.

To help figure this out, along with the performance implications, add a ReadOnlyPropertyTiming test fixture to
Java.Interop-PerformanceTests.dll to measure performance, and update JavaTiming to have the various proposed binding ideas so that we can determine the resulting code size.

Results are as follows:

Approach	Code Size (bytes)	Total (s)	Amortized (ticks)
No caching (current)	21	0.0029275	2927
"nullable" caching (not thread-safe; #1248 approach)	65	0.0000823	82
Inlined thread-safe caching	48	0.0000656	65
ReadOnlyProperty<T> caching	24+17 = 41	0.0001644	164

Worst performance is to not cache anything. At least the expected behavior is verified.

"Nullable" caching is quite performant. Pity it isn't thread-safe.

"Inlined thread-safe caching" is ~20% faster than "nullable" caching.

ReadOnlyProperty<T> caching is nearly 2x slower than "nullable".

Can ReadOnlyProperty<T> be made faster?

Footnotes

1. Not strictly true; instance fields can change within the
   object constructor, and static fields change change within
   the static constructor. As [generator] Cache static final field values. #1248 is about static fields of
   bound types, there should be no way for us to observe this.
   Things become trickier with instance fields. ↩

[Copilot]

Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (2)

tests/Java.Interop-PerformanceTests/Java.Interop/JavaTiming.cs:48

• [nitpick] The variable '_StaticReadonlyField_Cache' should be renamed to '_staticReadonlyFieldCache' for better readability.

static int? _StaticReadonlyField_Cache;

tests/Java.Interop-PerformanceTests/Java.Interop/JavaTiming.cs:74

• [nitpick] The method '_GetInt32Value' should be renamed to 'GetStaticReadonlyFieldValue' for better readability.

static int _GetInt32Value ()