-
Notifications
You must be signed in to change notification settings - Fork 63
[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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,12 +10,14 @@ public partial class JavaApi | |
{ | ||
public JavaApi () | ||
{ | ||
Packages = new List<JavaPackage> (); | ||
Packages = new Dictionary<string, JavaPackage> (StringComparer.OrdinalIgnoreCase); | ||
} | ||
|
||
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 | ||
|
@@ -24,15 +26,61 @@ public JavaPackage (JavaApi? parent) | |
{ | ||
Parent = parent; | ||
|
||
Types = new List<JavaType> (); | ||
Types = new Dictionary<string, List<JavaType>> (StringComparer.OrdinalIgnoreCase); | ||
} | ||
|
||
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 { get; } | ||
|
||
|
||
// Use this for a flat list of *all* types | ||
public IEnumerable<JavaType> AllTypes => Types.Values.SelectMany (v => v); | ||
|
||
public void AddType (string name, JavaType type) | ||
jpobst marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
{ | ||
// If this is a duplicate key, add it to existing list | ||
if (Types.TryGetValue (name, out var list)) { | ||
list.Add (type); | ||
return; | ||
} | ||
|
||
// Add to a new list | ||
var new_list = new List<JavaType> (); | ||
new_list.Add (type); | ||
jonpryor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
((IDictionary<string, List<JavaType>>)Types).Add (name, new_list); | ||
jpobst marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
} | ||
|
||
public void RemoveType (string name, JavaType type) | ||
jpobst marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
{ | ||
if (!Types.TryGetValue (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 | ||
((IDictionary<string, List<JavaType>>) Types).Remove (name); | ||
} | ||
|
||
public void ClearTypes () | ||
{ | ||
((IDictionary<string, List<JavaType>>) Types).Clear (); | ||
} | ||
|
||
// Content of this value is not stable. | ||
public override string ToString () | ||
{ | ||
|
@@ -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.Name, system_object); | ||
dummy_system_package.AddType (system_exception.Name, 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.Name, 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.Name, system_xml_xmlreader); | ||
} | ||
|
||
public static IEnumerable<JavaPackage> DummyManagedPackages { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
|
||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. …back on the "smaller diffs!" front, if we did have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested out
|
||
package.AddType (kls.Name!, kls); | ||
} else if (reader.LocalName == "interface") { | ||
var iface = new JavaInterface (package) { IsReferenceOnly = isReferenceOnly }; | ||
iface.Load (reader); | ||
package.Types.Add (iface); | ||
package.AddType (iface.Name!, iface); | ||
} else | ||
throw XmlUtil.UnexpectedElementOrContent ("package", reader, "class", "interface"); | ||
} while (true); | ||
|
Uh oh!
There was an error while loading. Please reload this page.