Skip to content

[ApiXmlAdjuster] Use different data model structures for better performance #756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,12 @@ public JavaStubGrammar ()
identifier.AstConfig.NodeCreator = (ctx, node) => node.AstNode = node.Token.ValueString;
compile_unit.AstConfig.NodeCreator = (ctx, node) => {
ProcessChildren (ctx, node);
node.AstNode = new JavaPackage (null) { Name = (string)node.ChildNodes [0].AstNode, Types = ((IEnumerable<JavaType>)node.ChildNodes [2].AstNode).ToList () };
var pkg = new JavaPackage (null) { Name = (string) node.ChildNodes [0].AstNode };

foreach (var t in (IEnumerable<JavaType>) node.ChildNodes [2].AstNode)
pkg.AddType (t);

node.AstNode = pkg;
};
opt_package_decl.AstConfig.NodeCreator = SelectSingleChild;
package_decl.AstConfig.NodeCreator = SelectChildValueAt (1);
Expand Down Expand Up @@ -637,9 +642,13 @@ public JavaStubParser ()
void FlattenNestedTypes (JavaPackage package)
{
var results = new List<JavaType> ();
foreach (var t in package.Types)
foreach (var t in package.AllTypes)
Flatten (results, t);
package.Types = results.ToList ();

package.ClearTypes ();

foreach (var t in results)
package.AddType (t);

void Flatten (List<JavaType> list, JavaType t)
{
Expand Down
68 changes: 58 additions & 10 deletions src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,77 @@ public partial class JavaApi
{
public JavaApi ()
{
Packages = new List<JavaPackage> ();
Packages = new Dictionary<string, JavaPackage> ();
}

public string? ExtendedApiSource { get; set; }
public string? Platform { get; set; }
public IList<JavaPackage> Packages { get; set; }
public IDictionary<string, JavaPackage> Packages { get; }

public ICollection<JavaPackage> AllPackages => Packages.Values;
}

public partial class JavaPackage
{
private Dictionary<string, List<JavaType>> types = new Dictionary<string, List<JavaType>> ();

public JavaPackage (JavaApi? parent)
{
Parent = parent;

Types = new List<JavaType> ();
}

public JavaApi? Parent { get; private set; }

public string? Name { get; set; }
public string? JniName { get; set; }
public IList<JavaType> Types { get; set; }


// Yes, there can be multiple types with the same *Java* name.
// For example:
// - MyInterface
// - MyInterfaceConsts
// It's debatable whether we handle this "properly", as most callers just
// do `First ()`, but it's been working for years so I'm not changing it.
// Exposes an IReadOnlyDictionary so caller cannot bypass our AddType/RemoveType code.
public IReadOnlyDictionary<string, List<JavaType>> Types => types;

// Use this for a flat list of *all* types
public IEnumerable<JavaType> AllTypes => Types.Values.SelectMany (v => v);

public void AddType (JavaType type)
{
// If this is a duplicate key, add it to existing list
if (Types.TryGetValue (type.Name!, out var list)) {
list.Add (type);
return;
}

// Add to a new list
var new_list = new List<JavaType> ();
new_list.Add (type);

types.Add (type.Name!, new_list);
}

public void RemoveType (JavaType type)
{
if (!Types.TryGetValue (type.Name!, out var list))
return;

// Remove 1 type from list if it contains multiple types
if (list.Count > 1) {
list.Remove (type);
return;
}

// Remove the whole dictionary entry
types.Remove (type.Name!);
}

public void ClearTypes ()
{
types.Clear ();
}

// Content of this value is not stable.
public override string ToString ()
{
Expand Down Expand Up @@ -123,14 +171,14 @@ static ManagedType ()
dummy_system_package = new JavaPackage (null) { Name = "System" };
system_object = new ManagedType (dummy_system_package) { Name = "Object" };
system_exception = new ManagedType (dummy_system_package) { Name = "Exception" };
dummy_system_package.Types.Add (system_object);
dummy_system_package.Types.Add (system_exception);
dummy_system_package.AddType (system_object);
dummy_system_package.AddType (system_exception);
dummy_system_io_package = new JavaPackage (null) { Name = "System.IO" };
system_io_stream = new ManagedType (dummy_system_io_package) { Name = "Stream" };
dummy_system_io_package.Types.Add (system_io_stream);
dummy_system_io_package.AddType (system_io_stream);
dummy_system_xml_package = new JavaPackage (null) { Name = "System.Xml" };
system_xml_xmlreader = new ManagedType (dummy_system_xml_package) { Name = "XmlReader" };
dummy_system_io_package.Types.Add (system_xml_xmlreader);
dummy_system_io_package.AddType (system_xml_xmlreader);
}

public static IEnumerable<JavaPackage> DummyManagedPackages {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public static class JavaApiDefectFinderExtensions
{
public static void FindDefects (this JavaApi api)
{
foreach (var type in api.Packages.SelectMany (p => p.Types).Where (t => !t.IsReferenceOnly))
foreach (var type in api.AllPackages.SelectMany (p => p.AllTypes).Where (t => !t.IsReferenceOnly))
type.FindDefects ();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public static class JavaApiGenericInheritanceMapperExtensions
{
public static void CreateGenericInheritanceMapping (this JavaApi api)
{
foreach (var kls in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ())
foreach (var kls in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ())
kls.PrepareGenericInheritanceMapping ();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public static class JavaApiNonBindableStripper
public static void StripNonBindables (this JavaApi api)
{
var invalids = new List<JavaMember> ();
foreach (var member in api.Packages.SelectMany (p => p.Types)
foreach (var member in api.AllPackages.SelectMany (p => p.AllTypes)
.SelectMany (t => t.Members).Where (m => m.Name != null && m.Name.Contains ('$')))
invalids.Add (member);
foreach (var invalid in invalids)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public static void MarkOverrides (this JavaApi api)

public static void MarkOverrides (this JavaApi api, HashSet<JavaClass> doneList)
{
foreach (var kls in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ())
foreach (var kls in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ())
kls.MarkOverrides (doneList);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,31 @@ static JavaTypeReference JavaTypeNameToReference (this JavaApi api, JavaTypeName

public static JavaType FindNonGenericType (this JavaApi api, string? name)
{
var ret = FindPackages (api, name ?? "")
.SelectMany (p => p.Types)
.FirstOrDefault (t => name == (t.Parent?.Name != null ? t.Parent.Name + "." : "") + t.Name);
if (ret == null)
ret = ManagedType.DummyManagedPackages
.SelectMany (p => p.Types)
.FirstOrDefault (t => t.FullName == name);
// Given a type name like 'android.graphics.BitmapFactory.Options'
// We're going to search for:
// - Pkg: android.graphics.BitmapFactory Type: Options
// - Pkg: android.graphics Type: BitmapFactory.Options
// - Pkg: android Type: graphics.BitmapFactory.Options
// etc. We will short-circuit as soon as we find a match
var index = name?.LastIndexOf ('.') ?? -1;

while (index > 0) {
var ns = name!.Substring (0, index);
var type_name = name.Substring (index + 1);

if (api.Packages.TryGetValue (ns, out var pkg)) {
if (pkg.Types.TryGetValue (type_name, out var type))
return type.First ();
}

index = name.LastIndexOf ('.', index - 1);
}

// See if it's purely a C# type
var ret = ManagedType.DummyManagedPackages
.SelectMany (p => p.AllTypes)
.FirstOrDefault (t => t.FullName == name);

if (ret == null) {
// We moved this type to "mono.android.app.IntentService" which makes this
// type resolution fail if a user tries to reference it in Java.
Expand All @@ -58,43 +76,26 @@ public static JavaType FindNonGenericType (this JavaApi api, string? name)
return ret;
}

static IEnumerable<JavaPackage> FindPackages (JavaApi api, string name)
{
// Given a type name like "java.lang.Object", return packages that could
// possibly contain the type so we don't search all packages, ie:
// - java.lang
// - java
var package_names = new List<string> ();
int index;

while ((index = name.LastIndexOf ('.')) >= 0) {
name = name.Substring (0, index);
package_names.Add (name);
}

return api.Packages.Where (p => package_names.Contains (p.Name, StringComparer.Ordinal)).ToList ();
}

public static void Resolve (this JavaApi api)
{
while (true) {
bool errors = false;
foreach (var t in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ().ToArray ())
foreach (var t in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ().ToArray ())
try {
t.Resolve ();
}
catch (JavaTypeResolutionException ex) {
Log.LogError ("Error while processing type '{0}': {1}", t, ex.Message);
errors = true;
t.Parent?.Types.Remove (t);
t.Parent?.RemoveType (t);
}
foreach (var t in api.Packages.SelectMany (p => p.Types).OfType<JavaInterface> ().ToArray ())
foreach (var t in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaInterface> ().ToArray ())
try {
t.Resolve ();
} catch (JavaTypeResolutionException ex) {
Log.LogError ("Error while processing type '{0}': {1}", t, ex.Message);
errors = true;
t.Parent?.Types.Remove (t);
t.Parent?.RemoveType (t);
}
if (!errors)
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public static void Save (this JavaApi api, XmlWriter writer)
if (api.Platform != null)
writer.WriteAttributeString ("platform", api.Platform);

foreach (var pkg in api.Packages) {
if (!pkg.Types.Any (t => !t.IsReferenceOnly))
foreach (var pkg in api.AllPackages) {
if (!pkg.AllTypes.Any (t => !t.IsReferenceOnly))
continue;
writer.WriteStartElement ("package");
writer.WriteAttributeString ("name", pkg.Name);
if (!string.IsNullOrEmpty (pkg.JniName)) {
writer.WriteAttributeString ("jni-name", pkg.JniName);
}
foreach (var type in pkg.Types) {
foreach (var type in pkg.AllTypes) {
if (type.IsReferenceOnly)
continue; // skip reference only types
if (type is JavaClass)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ public static void Load (this JavaApi api, XmlReader reader, bool isReferenceOnl
break; // </api>
if (reader.NodeType != XmlNodeType.Element || reader.LocalName != "package")
throw XmlUtil.UnexpectedElementOrContent ("api", reader, "package");
var pkg = api.Packages.FirstOrDefault (p => p.Name == reader.GetAttribute ("name"));
if (pkg == null) {

var name = reader.GetAttribute ("name");

if (!api.Packages.TryGetValue (name, out var pkg)) {
pkg = new JavaPackage (api);
api.Packages.Add (pkg);
api.Packages.Add (name, pkg);
}

pkg.Load (reader, isReferenceOnly);
} while (true);

Expand Down Expand Up @@ -67,11 +70,11 @@ public static void Load (this JavaPackage package, XmlReader reader, bool isRefe
if (reader.LocalName == "class") {
var kls = new JavaClass (package) { IsReferenceOnly = isReferenceOnly };
kls.Load (reader);
package.Types.Add (kls);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…back on the "smaller diffs!" front, if we did have a class JavaTypes : KeyedCollection<string, JavaType> class, this statement wouldn't need to change, nor the many other statements just like it…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested out KeyedCollection, but it cannot handle duplicate keys:

System.ArgumentException: 'An item with the same key has already been added. Key: key1'

package.AddType (kls);
} else if (reader.LocalName == "interface") {
var iface = new JavaInterface (package) { IsReferenceOnly = isReferenceOnly };
iface.Load (reader);
package.Types.Add (iface);
package.AddType (iface);
} else
throw XmlUtil.UnexpectedElementOrContent ("package", reader, "class", "interface");
} while (true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public static void m (String text) {
Assert.AreEqual (null, package.Name);
Assert.AreEqual (1, package.Types.Count);

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

Assert.AreEqual (1, Example_Type.Members.Count);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public void SetupFixture ()
[Test]
public void TestToString ()
{
var pkg = api.Packages.First (p => p.Name == "android.database");
var pkg = api.AllPackages.First (p => p.Name == "android.database");
Assert.AreEqual ("[Package] android.database", pkg.ToString ());
var kls = pkg.Types.First (t => t.FullName == "android.database.ContentObservable");
var kls = pkg.AllTypes.First (t => t.FullName == "android.database.ContentObservable");
Assert.AreEqual ("[Class] android.database.ContentObservable", kls.ToString ());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void AncestralOverrides ()
xapi.Resolve ();
xapi.CreateGenericInheritanceMapping ();
xapi.MarkOverrides ();
var t = xapi.Packages.First (_ => _.Name == "XXX").Types.First (_ => _.Name == "SherlockExpandableListActivity");
var t = xapi.AllPackages.First (_ => _.Name == "XXX").AllTypes.First (_ => _.Name == "SherlockExpandableListActivity");
var m = t.Members.OfType<JavaMethod> ().First (_ => _.Name == "addContentView");
Assert.IsNotNull (m.BaseMethod, "base method not found");
}
Expand Down Expand Up @@ -88,7 +88,7 @@ public void GenericConstructors ()
xapi = new JavaApi ();
using (var xr = XmlReader.Create (new StringReader (sw.ToString ())))
xapi.Load (xr, true);
var t = xapi.Packages.First (_ => _.Name == "XXX").Types.First (_ => _.Name == "GenericConstructors");
var t = xapi.AllPackages.First (_ => _.Name == "XXX").AllTypes.First (_ => _.Name == "GenericConstructors");
var m = t.Members.OfType<JavaConstructor> ().FirstOrDefault ();
Assert.IsNotNull (m.TypeParameters, "constructor not found");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,20 @@ public void IntentServiceHack ()
var api = JavaApiTestHelper.GetLoadedApi ();

// Create "mono.android.app" package
var mono_android_app = new JavaPackage (api) { Name = "mono.android.app", JniName = "mono/android/app", Types = new List<JavaType> () };
api.Packages.Add (mono_android_app);
var mono_android_app = new JavaPackage (api) { Name = "mono.android.app", JniName = "mono/android/app" };
api.Packages.Add (mono_android_app.Name, mono_android_app);

// Remove "android.app.IntentService" type
var android_app = api.Packages.Single (p => p.Name == "android.app");
var intent_service = android_app.Types.Single (t => t.Name == "IntentService");
android_app.Types.Remove (intent_service);
var android_app = api.AllPackages.Single (p => p.Name == "android.app");
var intent_service = android_app.AllTypes.Single (t => t.Name == "IntentService");
android_app.RemoveType (intent_service);

// Create new "mono.android.app.IntentService" type
var new_intent_service = new JavaClass (mono_android_app) {
Name = intent_service.Name,
};

mono_android_app.Types.Add (new_intent_service);
mono_android_app.AddType (new_intent_service);

api.Resolve ();

Expand Down
Loading