Skip to content

Commit 088fb93

Browse files
committed
[Java.Interop] ReadOnlyProperty<T>?
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.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? [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.
1 parent d30d554 commit 088fb93

File tree

3 files changed

+122
-0
lines changed

3 files changed

+122
-0
lines changed

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

+44
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Collections.Concurrent;
4+
using System.Threading;
45

56
using Java.Interop;
67
using Java.Interop.GenericMarshaler;
@@ -36,6 +37,49 @@ public unsafe JavaTiming ()
3637
Construct (ref peer, JniObjectReferenceOptions.CopyAndDispose);
3738
}
3839

40+
public static int StaticReadonlyField_NoCache {
41+
get {
42+
const string __id = "STATIC_READONLY_FIELD.I";
43+
var __v = _members.StaticFields.GetInt32Value (__id);
44+
return __v;
45+
}
46+
}
47+
48+
static int? _StaticReadonlyField_Cache;
49+
public static int StaticReadonlyField_ThreadUnsafeCache {
50+
get {
51+
if (_StaticReadonlyField_Cache.HasValue)
52+
return _StaticReadonlyField_Cache.Value;
53+
const string __id = "STATIC_READONLY_FIELD.I";
54+
var __v = _members.StaticFields.GetInt32Value (__id);
55+
return (int) (_StaticReadonlyField_Cache = __v);
56+
}
57+
}
58+
static int _StaticReadonlyField_haveValue;
59+
static int _StaticReadonlyField_value;
60+
61+
public static int StaticReadonlyField_ThreadSafeCache {
62+
get {
63+
if (1 == Interlocked.CompareExchange (ref _StaticReadonlyField_haveValue, 1, 0))
64+
return _StaticReadonlyField_value;
65+
const string __id = "STATIC_READONLY_FIELD.I";
66+
var __v = _members.StaticFields.GetInt32Value (__id);
67+
return _StaticReadonlyField_value = __v;
68+
}
69+
}
70+
71+
static ReadOnlyProperty<int> _rop_StaticReadonlyField = new ReadOnlyProperty<int> ();
72+
public static unsafe int StaticReadonlyField_Rop {
73+
get {
74+
static int _GetInt32Value (string encodedMember)
75+
{
76+
return _members.StaticFields.GetInt32Value (encodedMember);
77+
}
78+
delegate *managed <string, int> c = &_GetInt32Value;
79+
return _rop_StaticReadonlyField.GetValue (c, "STATIC_READONLY_FIELD.I");
80+
}
81+
}
82+
3983
static JniMethodInfo svm;
4084
public static void StaticVoidMethod ()
4185
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Diagnostics;
4+
using System.Globalization;
5+
using System.Linq;
6+
using System.Reflection;
7+
using System.Runtime.CompilerServices;
8+
using System.Runtime.InteropServices;
9+
using System.Threading;
10+
11+
using Java.Interop;
12+
13+
using NUnit.Framework;
14+
15+
namespace Java.Interop.PerformanceTests {
16+
17+
18+
[TestFixture]
19+
class ReadOnlyPropertyTiming : Java.InteropTests.JavaVMFixture {
20+
[Test]
21+
public void StaticReadOnlyPropertyTiming ()
22+
{
23+
const int count = 1000;
24+
25+
var noCache = Stopwatch.StartNew ();
26+
for (int i = 0; i < count; ++i) {
27+
_ = JavaTiming.StaticReadonlyField_NoCache;
28+
}
29+
noCache.Stop ();
30+
31+
var badCache = Stopwatch.StartNew ();
32+
for (int i = 0; i < count; ++i) {
33+
_ = JavaTiming.StaticReadonlyField_ThreadUnsafeCache;
34+
}
35+
badCache.Stop ();
36+
37+
var goodCache = Stopwatch.StartNew ();
38+
for (int i = 0; i < count; ++i) {
39+
_ = JavaTiming.StaticReadonlyField_ThreadSafeCache;
40+
}
41+
goodCache.Stop ();
42+
43+
var ropCache = Stopwatch.StartNew ();
44+
for (int i = 0; i < count; ++i) {
45+
_ = JavaTiming.StaticReadonlyField_Rop;
46+
}
47+
ropCache.Stop ();
48+
49+
Console.WriteLine ("Static ReadOnly Property Lookup + Invoke Timing:");
50+
Console.WriteLine ("\t No caching: {0}, {1} ticks", noCache.Elapsed, noCache.ElapsedTicks / count);
51+
Console.WriteLine ("\t Thread Unsafe Cache: {0}, {1} ticks", badCache.Elapsed, badCache.ElapsedTicks / count);
52+
Console.WriteLine ("\t Thread-Safe Cache: {0}, {1} ticks", goodCache.Elapsed, goodCache.ElapsedTicks / count);
53+
Console.WriteLine ("\tReadOnlyProperty<int> Cache: {0}, {1} ticks", ropCache.Elapsed, ropCache.ElapsedTicks / count);
54+
}
55+
}
56+
57+
struct ReadOnlyProperty<T> {
58+
int have_value;
59+
T value;
60+
61+
[MethodImpl (MethodImplOptions.AggressiveInlining)]
62+
public unsafe T GetValue (delegate *managed <string, T> c, string encodedMember)
63+
{
64+
if (1 == Interlocked.CompareExchange (ref have_value, 1, 0))
65+
return value;
66+
var __v = c (encodedMember);
67+
value = __v;
68+
return value;
69+
}
70+
}
71+
}

tests/Java.Interop-PerformanceTests/java/com/xamarin/interop/performance/JavaTiming.java

+7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22

33
public class JavaTiming {
44

5+
public static final int STATIC_READONLY_FIELD = getStaticReadonlyFieldValue ();
6+
7+
static int getStaticReadonlyFieldValue ()
8+
{
9+
return 42;
10+
}
11+
512
public static void StaticVoidMethod ()
613
{
714
}

0 commit comments

Comments
 (0)