You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 [^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.
0 commit comments