Skip to content

Commit 0cb8e2d

Browse files
jpobstjpobst
authored
[Xamarin.Android.Tools.ApiXmlAdjuster] Use Dictionaries for perf (#756)
The `Xamarin.Android.Tools.ApiXmlAdjuster` process builds an in-memory model of every Java type we know about, and then queries this model many times in order to ensure we can resolve every needed Java type to build a binding. The data structure of this model is: public class JavaApi { public List<JavaPackage> Packages { get; } } public class JavaPackage { public List<JavaType> Types { get; } } The model is then queried using LINQ:: // Bad var type = api.Packages.SelectMany (p => p.Types) .FirstOrDefault (t => t.Name == type_name); // Less bad var type = api.Packages.FirstOrDefault (p => p.Name == pkg_name) ?.Types.FirstOrDefault (t => t.Name == type_name); In the random GooglePlayServices package I used for testing, `JavaApi` contained 310 packages and a total of 5941 types. Repeatedly looping through them looking for the correct type takes a considerable amount of time. Instead, we can use a `Dictionary` to store packages and types keyed by name to significantly speed up type resolution: public class JavaApi { public Dictionary<string, JavaPackage> Packages { get; } } public class JavaPackage { public Dictionary<string, List<JavaType>> Types { get; } } For the GPS project, this reduced time taken considerably: | Version | Time | | ------- | ---- | | master | 9.3s | | This PR | 1.4s | The only "interesting" detail is that we can have multiple types with the same *Java* name, such as `MyInterface` and `MyInterfaceConsts`. Thus we need to use `Dictionary<string, List<JavaType>>` to ensure we collect them all.
1 parent 99f8990 commit 0cb8e2d

File tree

17 files changed

+150
-92
lines changed

17 files changed

+150
-92
lines changed

‎src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/JavaStubParser.cs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,12 @@ public JavaStubGrammar ()
315315
identifier.AstConfig.NodeCreator = (ctx, node) => node.AstNode = node.Token.ValueString;
316316
compile_unit.AstConfig.NodeCreator = (ctx, node) => {
317317
ProcessChildren (ctx, node);
318-
node.AstNode = new JavaPackage (null) { Name = (string)node.ChildNodes [0].AstNode, Types = ((IEnumerable<JavaType>)node.ChildNodes [2].AstNode).ToList () };
318+
var pkg = new JavaPackage (null) { Name = (string) node.ChildNodes [0].AstNode };
319+
320+
foreach (var t in (IEnumerable<JavaType>) node.ChildNodes [2].AstNode)
321+
pkg.AddType (t);
322+
323+
node.AstNode = pkg;
319324
};
320325
opt_package_decl.AstConfig.NodeCreator = SelectSingleChild;
321326
package_decl.AstConfig.NodeCreator = SelectChildValueAt (1);
@@ -637,9 +642,13 @@ public JavaStubParser ()
637642
void FlattenNestedTypes (JavaPackage package)
638643
{
639644
var results = new List<JavaType> ();
640-
foreach (var t in package.Types)
645+
foreach (var t in package.AllTypes)
641646
Flatten (results, t);
642-
package.Types = results.ToList ();
647+
648+
package.ClearTypes ();
649+
650+
foreach (var t in results)
651+
package.AddType (t);
643652

644653
void Flatten (List<JavaType> list, JavaType t)
645654
{

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs‎

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,77 @@ public partial class JavaApi
1010
{
1111
public JavaApi ()
1212
{
13-
Packages = new List<JavaPackage> ();
13+
Packages = new Dictionary<string, JavaPackage> ();
1414
}
1515

1616
public string? ExtendedApiSource { get; set; }
1717
public string? Platform { get; set; }
18-
public IList<JavaPackage> Packages { get; set; }
18+
public IDictionary<string, JavaPackage> Packages { get; }
19+
20+
public ICollection<JavaPackage> AllPackages => Packages.Values;
1921
}
2022

2123
public partial class JavaPackage
2224
{
25+
private Dictionary<string, List<JavaType>> types = new Dictionary<string, List<JavaType>> ();
26+
2327
public JavaPackage (JavaApi? parent)
2428
{
2529
Parent = parent;
26-
27-
Types = new List<JavaType> ();
2830
}
2931

3032
public JavaApi? Parent { get; private set; }
3133

3234
public string? Name { get; set; }
3335
public string? JniName { get; set; }
34-
public IList<JavaType> Types { get; set; }
35-
36+
37+
// Yes, there can be multiple types with the same *Java* name.
38+
// For example:
39+
// - MyInterface
40+
// - MyInterfaceConsts
41+
// It's debatable whether we handle this "properly", as most callers just
42+
// do `First ()`, but it's been working for years so I'm not changing it.
43+
// Exposes an IReadOnlyDictionary so caller cannot bypass our AddType/RemoveType code.
44+
public IReadOnlyDictionary<string, List<JavaType>> Types => types;
45+
46+
// Use this for a flat list of *all* types
47+
public IEnumerable<JavaType> AllTypes => Types.Values.SelectMany (v => v);
48+
49+
public void AddType (JavaType type)
50+
{
51+
// If this is a duplicate key, add it to existing list
52+
if (Types.TryGetValue (type.Name!, out var list)) {
53+
list.Add (type);
54+
return;
55+
}
56+
57+
// Add to a new list
58+
var new_list = new List<JavaType> ();
59+
new_list.Add (type);
60+
61+
types.Add (type.Name!, new_list);
62+
}
63+
64+
public void RemoveType (JavaType type)
65+
{
66+
if (!Types.TryGetValue (type.Name!, out var list))
67+
return;
68+
69+
// Remove 1 type from list if it contains multiple types
70+
if (list.Count > 1) {
71+
list.Remove (type);
72+
return;
73+
}
74+
75+
// Remove the whole dictionary entry
76+
types.Remove (type.Name!);
77+
}
78+
79+
public void ClearTypes ()
80+
{
81+
types.Clear ();
82+
}
83+
3684
// Content of this value is not stable.
3785
public override string ToString ()
3886
{
@@ -123,14 +171,14 @@ static ManagedType ()
123171
dummy_system_package = new JavaPackage (null) { Name = "System" };
124172
system_object = new ManagedType (dummy_system_package) { Name = "Object" };
125173
system_exception = new ManagedType (dummy_system_package) { Name = "Exception" };
126-
dummy_system_package.Types.Add (system_object);
127-
dummy_system_package.Types.Add (system_exception);
174+
dummy_system_package.AddType (system_object);
175+
dummy_system_package.AddType (system_exception);
128176
dummy_system_io_package = new JavaPackage (null) { Name = "System.IO" };
129177
system_io_stream = new ManagedType (dummy_system_io_package) { Name = "Stream" };
130-
dummy_system_io_package.Types.Add (system_io_stream);
178+
dummy_system_io_package.AddType (system_io_stream);
131179
dummy_system_xml_package = new JavaPackage (null) { Name = "System.Xml" };
132180
system_xml_xmlreader = new ManagedType (dummy_system_xml_package) { Name = "XmlReader" };
133-
dummy_system_io_package.Types.Add (system_xml_xmlreader);
181+
dummy_system_io_package.AddType (system_xml_xmlreader);
134182
}
135183

136184
public static IEnumerable<JavaPackage> DummyManagedPackages {

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiDefectFinderExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ public static class JavaApiDefectFinderExtensions
77
{
88
public static void FindDefects (this JavaApi api)
99
{
10-
foreach (var type in api.Packages.SelectMany (p => p.Types).Where (t => !t.IsReferenceOnly))
10+
foreach (var type in api.AllPackages.SelectMany (p => p.AllTypes).Where (t => !t.IsReferenceOnly))
1111
type.FindDefects ();
1212
}
1313

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiGenericInheritanceMapperExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ public static class JavaApiGenericInheritanceMapperExtensions
88
{
99
public static void CreateGenericInheritanceMapping (this JavaApi api)
1010
{
11-
foreach (var kls in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ())
11+
foreach (var kls in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ())
1212
kls.PrepareGenericInheritanceMapping ();
1313
}
1414

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiNonBindableStripper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ public static class JavaApiNonBindableStripper
99
public static void StripNonBindables (this JavaApi api)
1010
{
1111
var invalids = new List<JavaMember> ();
12-
foreach (var member in api.Packages.SelectMany (p => p.Types)
12+
foreach (var member in api.AllPackages.SelectMany (p => p.AllTypes)
1313
.SelectMany (t => t.Members).Where (m => m.Name != null && m.Name.Contains ('$')))
1414
invalids.Add (member);
1515
foreach (var invalid in invalids)

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiOverrideMarkerExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public static void MarkOverrides (this JavaApi api)
1414

1515
public static void MarkOverrides (this JavaApi api, HashSet<JavaClass> doneList)
1616
{
17-
foreach (var kls in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ())
17+
foreach (var kls in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ())
1818
kls.MarkOverrides (doneList);
1919
}
2020

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiTypeResolverExtensions.cs‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,31 @@ static JavaTypeReference JavaTypeNameToReference (this JavaApi api, JavaTypeName
3939

4040
public static JavaType FindNonGenericType (this JavaApi api, string? name)
4141
{
42-
var ret = FindPackages (api, name ?? "")
43-
.SelectMany (p => p.Types)
44-
.FirstOrDefault (t => name == (t.Parent?.Name != null ? t.Parent.Name + "." : "") + t.Name);
45-
if (ret == null)
46-
ret = ManagedType.DummyManagedPackages
47-
.SelectMany (p => p.Types)
48-
.FirstOrDefault (t => t.FullName == name);
42+
// Given a type name like 'android.graphics.BitmapFactory.Options'
43+
// We're going to search for:
44+
// - Pkg: android.graphics.BitmapFactory Type: Options
45+
// - Pkg: android.graphics Type: BitmapFactory.Options
46+
// - Pkg: android Type: graphics.BitmapFactory.Options
47+
// etc. We will short-circuit as soon as we find a match
48+
var index = name?.LastIndexOf ('.') ?? -1;
49+
50+
while (index > 0) {
51+
var ns = name!.Substring (0, index);
52+
var type_name = name.Substring (index + 1);
53+
54+
if (api.Packages.TryGetValue (ns, out var pkg)) {
55+
if (pkg.Types.TryGetValue (type_name, out var type))
56+
return type.First ();
57+
}
58+
59+
index = name.LastIndexOf ('.', index - 1);
60+
}
61+
62+
// See if it's purely a C# type
63+
var ret = ManagedType.DummyManagedPackages
64+
.SelectMany (p => p.AllTypes)
65+
.FirstOrDefault (t => t.FullName == name);
66+
4967
if (ret == null) {
5068
// We moved this type to "mono.android.app.IntentService" which makes this
5169
// type resolution fail if a user tries to reference it in Java.
@@ -58,43 +76,26 @@ public static JavaType FindNonGenericType (this JavaApi api, string? name)
5876
return ret;
5977
}
6078

61-
static IEnumerable<JavaPackage> FindPackages (JavaApi api, string name)
62-
{
63-
// Given a type name like "java.lang.Object", return packages that could
64-
// possibly contain the type so we don't search all packages, ie:
65-
// - java.lang
66-
// - java
67-
var package_names = new List<string> ();
68-
int index;
69-
70-
while ((index = name.LastIndexOf ('.')) >= 0) {
71-
name = name.Substring (0, index);
72-
package_names.Add (name);
73-
}
74-
75-
return api.Packages.Where (p => package_names.Contains (p.Name, StringComparer.Ordinal)).ToList ();
76-
}
77-
7879
public static void Resolve (this JavaApi api)
7980
{
8081
while (true) {
8182
bool errors = false;
82-
foreach (var t in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ().ToArray ())
83+
foreach (var t in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ().ToArray ())
8384
try {
8485
t.Resolve ();
8586
}
8687
catch (JavaTypeResolutionException ex) {
8788
Log.LogError ("Error while processing type '{0}': {1}", t, ex.Message);
8889
errors = true;
89-
t.Parent?.Types.Remove (t);
90+
t.Parent?.RemoveType (t);
9091
}
91-
foreach (var t in api.Packages.SelectMany (p => p.Types).OfType<JavaInterface> ().ToArray ())
92+
foreach (var t in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaInterface> ().ToArray ())
9293
try {
9394
t.Resolve ();
9495
} catch (JavaTypeResolutionException ex) {
9596
Log.LogError ("Error while processing type '{0}': {1}", t, ex.Message);
9697
errors = true;
97-
t.Parent?.Types.Remove (t);
98+
t.Parent?.RemoveType (t);
9899
}
99100
if (!errors)
100101
break;

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@ public static void Save (this JavaApi api, XmlWriter writer)
2424
if (api.Platform != null)
2525
writer.WriteAttributeString ("platform", api.Platform);
2626

27-
foreach (var pkg in api.Packages) {
28-
if (!pkg.Types.Any (t => !t.IsReferenceOnly))
27+
foreach (var pkg in api.AllPackages) {
28+
if (!pkg.AllTypes.Any (t => !t.IsReferenceOnly))
2929
continue;
3030
writer.WriteStartElement ("package");
3131
writer.WriteAttributeString ("name", pkg.Name);
3232
if (!string.IsNullOrEmpty (pkg.JniName)) {
3333
writer.WriteAttributeString ("jni-name", pkg.JniName);
3434
}
35-
foreach (var type in pkg.Types) {
35+
foreach (var type in pkg.AllTypes) {
3636
if (type.IsReferenceOnly)
3737
continue; // skip reference only types
3838
if (type is JavaClass)

‎src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,14 @@ public static void Load (this JavaApi api, XmlReader reader, bool isReferenceOnl
3232
break; // </api>
3333
if (reader.NodeType != XmlNodeType.Element || reader.LocalName != "package")
3434
throw XmlUtil.UnexpectedElementOrContent ("api", reader, "package");
35-
var pkg = api.Packages.FirstOrDefault (p => p.Name == reader.GetAttribute ("name"));
36-
if (pkg == null) {
35+
36+
var name = reader.GetAttribute ("name");
37+
38+
if (!api.Packages.TryGetValue (name, out var pkg)) {
3739
pkg = new JavaPackage (api);
38-
api.Packages.Add (pkg);
40+
api.Packages.Add (name, pkg);
3941
}
42+
4043
pkg.Load (reader, isReferenceOnly);
4144
} while (true);
4245

@@ -67,11 +70,11 @@ public static void Load (this JavaPackage package, XmlReader reader, bool isRefe
6770
if (reader.LocalName == "class") {
6871
var kls = new JavaClass (package) { IsReferenceOnly = isReferenceOnly };
6972
kls.Load (reader);
70-
package.Types.Add (kls);
73+
package.AddType (kls);
7174
} else if (reader.LocalName == "interface") {
7275
var iface = new JavaInterface (package) { IsReferenceOnly = isReferenceOnly };
7376
iface.Load (reader);
74-
package.Types.Add (iface);
77+
package.AddType (iface);
7578
} else
7679
throw XmlUtil.UnexpectedElementOrContent ("package", reader, "class", "interface");
7780
} while (true);

‎tests/Java.Interop.Tools.JavaSource-Tests/JavaStubParserTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public static void m (String text) {
3535
Assert.AreEqual (null, package.Name);
3636
Assert.AreEqual (1, package.Types.Count);
3737

38-
var Example_Type = package.Types [0];
38+
var Example_Type = package.AllTypes.First ();
3939
Assert.AreEqual ("Example", Example_Type.FullName);
4040

4141
Assert.AreEqual (1, Example_Type.Members.Count);

0 commit comments

Comments
 (0)