[ApiXmlAdjuster] Use different data model structures for better performance

[jpobst]

Today, our 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; }
}

Then it is queried using LINQ like this:

// 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 GPS 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.

For good measure I ran a XA build with this to ensure Mono.Android ApiCompat didn't find any issues: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4282925.

[jonpryor]

…alternatively, should this be Set<JavaType> instead of List<JavaType>? Does it make sense to allow for the same instance to be added multiple times?

[jonpryor]

The same "compatibility"/"diff minimization" question/concern applies to Types vs. AllTypes: I suspect that if Types became an IEnumerable<JavaType> property and AllTypes were the "Dictionary-like" property, fewer changes would be required.

[jpobst]

IMO, the changed Packages and Types properties are the canonical API here that every consumer should use, and are the correct types.

I added AllPackages/AllTypes as hacks to minimize the diffs. In an ideal world every consumer would be updated to use the canonical API and ensure they properly handle any duplicate types if needed, but I opted for a smaller diff instead. 😄

[jonpryor]

Java type names are not filesystem-bound constructs; they are case sensitive, and it is entirely valid to have a package example and package Example in the same app. (It'll cause pain & suffering on case-insensitive filesystems! But Java allows it.)

I think this should use StringComparer.Ordinal.

[jpobst]

Good point, fixed.

[jonpryor]

…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…

[jpobst]

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

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