issue-link js-issue-link" data-error-text="Failed to load title" data-id="2466716751" data-permission-text="Title is private" data-url="https://github.com/dotnet/java-interop/issues/1243" data-hovercard-type="issue" data-hovercard-url="/dotnet/java-interop/issues/1243/hovercard" href="https://github.com/dotnet/java-interop/issues/1243">#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 [^1]. 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][0], 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.0029098 | 2909 | | "nullable" caching (not thread-safe; #1248 approach) | 65 | 0.0000827 | 82 | | Inlined thread-safe caching | 48 | 0.0000664 | 66 | | `ReadOnlyProperty<T>` caching | 19+21 = 40 | 0.0001548 | 154 | 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? [0]: #1248 (comment) [^1]: Not strictly true; *instance* fields can change within the object constructor, and *static* fields change change within the static constructor. As #1248 is about static fields of *bound* types, there should be no way for us to observe this. Things become trickier with instance fields.

[Java.Interop] ReadOnlyProperty<T>?#1249jonpryor wants to merge 1 commit intomaindotnet/java-interop:mainfrom dev/jonp/jonp-ReadOnlyPropertydotnet/java-interop:dev/jonp/jonp-ReadOnlyPropertyCopy head branch name to clipboard

Commits

Commits on Sep 4, 2024

[Java.Interop] ReadOnlyProperty<T>?

[Java.Interop] ReadOnlyProperty<T>?#1249
jonpryor wants to merge 1 commit intomainfrom
dev/jonp/jonp-ReadOnlyProperty