Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

add some basic parallelism to improve performance #799

Merged
merged 1 commit into from
Dec 1, 2023
Merged

add some basic parallelism to improve performance #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 32 additions & 7 deletions SwiftReflector/ErrorHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace SwiftReflector {
public class ErrorHandling {
object messagesLock = new object ();
Copy link
Member

@mandel-macaque mandel-macaque Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I believe you can make your life easier by using https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentbag-1?view=net-8.0

You will need to make a decision, but looking at your code you can move from a List to a Bag since:

  1. You are not using an indexer.
  2. Looks like you do not care about order.
  3. You access the collections via Linq and the ConcurrentBag does provide the required interfaces to continue doing it.
  4. Provides a nice thread-safe property "IsEmpty" which you can use in some of the properties.

There is a few draw backs, one is that you won't have an AddRange method: dotnet/runtime#61291

The second one I can't think, by definition all LINQ operations are thread-safe because they only read, the diff is that the concurrent bag provides a snapshot of the collection while iterating, that will change the behavior and probably, if that must be kept, is best to use a lock.

You'll have to probably write that the class must be thread-safe for the future team.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opened an issue for it #801

List<ReflectorError> messages;

public ErrorHandling ()
Expand All @@ -37,37 +38,61 @@ public IEnumerable<ReflectorError> Warnings {

public void Add (ErrorHandling eh)
{
messages.AddRange (eh.messages);
lock (messagesLock) {
lock (eh.messagesLock) {
messages.AddRange (eh.messages);
}
}
}

public void Add (params ReflectorError [] errors)
{
messages.AddRange (errors);
lock (messagesLock) {
messages.AddRange (errors);
}
}

public void Add (Exception exception)
{
lock (messagesLock) {
#if CRASH_ON_EXCEPTION
ExceptionDispatchInfo.Capture (exception).Throw ();
#else
messages.Add (new ReflectorError (exception));
messages.Add (new ReflectorError (exception));
#endif
}
}

public bool AnyMessages {
get { return messages.Count > 0; }
get {
lock (messagesLock) {
return messages.Count > 0;
}
}
}

public bool AnyErrors {
get { return messages.Any ((v) => !v.IsWarning); }
get {
lock (messagesLock) {
return messages.Any ((v) => !v.IsWarning);
}
}
}

public int WarningCount {
get { return messages.Count ((v) => v.IsWarning); }
get {
lock (messagesLock) {
return messages.Count ((v) => v.IsWarning);
}
}
}

public int ErrorCount {
get { return messages.Count ((v) => !v.IsWarning); }
get {
lock (messagesLock) {
return messages.Count ((v) => !v.IsWarning);
}
}
}

public int Show (int verbosity)
Expand Down
12 changes: 7 additions & 5 deletions SwiftReflector/Inventory/ClassInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ public override void Add (TLDefinition tld, Stream srcStm)
return;
SwiftName className = ToClassName (tld);
SwiftClassName formalName = ToFormalClassName (tld);
ClassContents contents = null;
if (!values.TryGetValue (className, out contents)) {
contents = new ClassContents (formalName, sizeofMachinePointer);
values.Add (className, contents);
lock (valuesLock) {
Copy link
Member

@mandel-macaque mandel-macaque Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Same as with the ErrorHandler, we could use https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2?view=net-8.0

Looking at the code on which you are locking, you could remove the need to the TryGetValue, use TryAdd which will add only if not present. If you were to update, TryUpdate would be the method you want.

I don't know the code base, but you might be in a situation in which some Invetories are thread-safe and others are not.

In this case, I cannot think of a drawback looking at your code AND will ensure that all subclasses are threadsafe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opened an issue for it. #800

ClassContents contents = null;
if (!values.TryGetValue (className, out contents)) {
contents = new ClassContents (formalName, sizeofMachinePointer);
values.Add (className, contents);
}
contents.Add (tld, srcStm);
}
contents.Add (tld, srcStm);
}

public static SwiftClassName ToFormalClassName (TLDefinition tld)
Expand Down
12 changes: 7 additions & 5 deletions SwiftReflector/Inventory/FunctionInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ public override void Add (TLDefinition tld, Stream srcStm)
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 10, $"expected a top-level function but got a {tld.GetType ().Name}");

OverloadInventory overloads = null;
if (!values.TryGetValue (tlf.Name, out overloads)) {
overloads = new OverloadInventory (tlf.Name, sizeofMachinePointer);
values.Add (tlf.Name, overloads);
lock (valuesLock) {
OverloadInventory overloads = null;
if (!values.TryGetValue (tlf.Name, out overloads)) {
overloads = new OverloadInventory (tlf.Name, sizeofMachinePointer);
values.Add (tlf.Name, overloads);
}
overloads.Add (tlf, srcStm);
}
Comment on lines +25 to 32
Copy link
Member

Choose a reason for hiding this comment

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

Same, which also means that changing the dictionary in the base type will make all of your subclasses thread-safe.

overloads.Add (tlf, srcStm);
}

public TLFunction ContainsEquivalentFunction (TLFunction func)
Expand Down
9 changes: 7 additions & 2 deletions SwiftReflector/Inventory/Inventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace SwiftReflector.Inventory {
public abstract class Inventory<T> {
protected object valuesLock = new object ();
protected Dictionary<SwiftName, T> values = new Dictionary<SwiftName, T> ();
Copy link
Member

Choose a reason for hiding this comment

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

A ConcurrentDict will make your life easier.


public IEnumerable<SwiftName> Names { get { return values.Keys; } }
Expand All @@ -17,7 +18,9 @@ public abstract class Inventory<T> {

public bool ContainsName (SwiftName name)
{
return values.ContainsKey (name);
lock (valuesLock) {
return values.ContainsKey (name);
}
}
public bool ContainsName (string name)
{
Expand All @@ -26,7 +29,9 @@ public bool ContainsName (string name)

public bool TryGetValue (SwiftName name, out T value)
{
return values.TryGetValue (name, out value);
lock (valuesLock) {
return values.TryGetValue (name, out value);
}
}

public bool TryGetValue (string name, out T value)
Expand Down
38 changes: 33 additions & 5 deletions SwiftReflector/Inventory/ModuleInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using SwiftReflector.Demangling;
using ObjCRuntime;
using SwiftRuntimeLibrary;
using System.Threading.Tasks;

namespace SwiftReflector.Inventory {
public class ModuleInventory : Inventory<ModuleContents> {
Expand All @@ -24,12 +25,14 @@ public int SizeofMachinePointer {

public override void Add (TLDefinition tld, Stream srcStm)
{
ModuleContents module = null;
if (!values.TryGetValue (tld.Module, out module)) {
module = new ModuleContents (tld.Module, SizeofMachinePointer);
values.Add (tld.Module, module);
lock (valuesLock) {
ModuleContents module = null;
if (!values.TryGetValue (tld.Module, out module)) {
module = new ModuleContents (tld.Module, SizeofMachinePointer);
values.Add (tld.Module, module);
}
module.Add (tld, srcStm);
Comment on lines +28 to +34
Copy link
Member

Choose a reason for hiding this comment

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

Same, all this methods are yy most of the time.

}
module.Add (tld, srcStm);
}

public IEnumerable<SwiftName> ModuleNames {
Expand Down Expand Up @@ -111,6 +114,23 @@ public static ModuleInventory FromFile (string pathToDynamicLibrary, ErrorHandli
}
}

public static async Task<ModuleInventory> FromFilesAsync (IEnumerable<string> pathsToLibraryFiles, ErrorHandling errors)
{
var streams = pathsToLibraryFiles.Select (path => new FileStream (path, FileMode.Open, FileAccess.Read, FileShare.Read)).ToList ();
if (streams.Count == 0)
return null;
var inventory = new ModuleInventory ();
try {
var tasks = streams.Select (stm => FromStreamIntoAsync (stm, inventory, errors, stm.Name));
await Task.WhenAll (tasks);
} finally {
foreach (var stm in streams) {
stm.Dispose ();
}
Comment on lines +127 to +129
Copy link
Member

Choose a reason for hiding this comment

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

The Reactive Extensions (https://github.com/dotnet/reactive) provides a nice way to deal with a collection of IDisposables: https://learn.microsoft.com/en-us/previous-versions/dotnet/reactive-extensions/hh229918(v=vs.103)

I don't know how you feel about adding it as a dependency "System.Reactive" but will probably help.

}
return inventory;
}

public static ModuleInventory FromFiles (IEnumerable<string> pathsToLibraryFiles, ErrorHandling errors)
{
ModuleInventory inventory = null;
Expand All @@ -124,6 +144,14 @@ public static ModuleInventory FromFiles (IEnumerable<string> pathsToLibraryFiles
return inventory;
}

static async Task<ModuleInventory> FromStreamIntoAsync (Stream stm, ModuleInventory inventory,
ErrorHandling errors, string fileName = null)
{
return await Task.Run (() => {
return FromStreamInto (stm, inventory, errors, fileName);
});
}

static ModuleInventory FromStreamInto (Stream stm, ModuleInventory inventory,
ErrorHandling errors, string fileName = null)
{
Expand Down
16 changes: 9 additions & 7 deletions SwiftReflector/Inventory/OverloadInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ public OverloadInventory (SwiftName name, int sizeofMachinePointer)

public override void Add (TLDefinition tld, Stream srcStm)
{
TLFunction tlf = tld as TLFunction;
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 11, $"expected a top-level function but got a {tld.GetType ().Name}");
if (Functions.Exists (f => tlf.MangledName == f.MangledName)) {
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 12, $"duplicate function found for {tlf.MangledName}");
} else {
Functions.Add (tlf);
lock (valuesLock) {
TLFunction tlf = tld as TLFunction;
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 11, $"expected a top-level function but got a {tld.GetType ().Name}");
if (Functions.Exists (f => tlf.MangledName == f.MangledName)) {
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 12, $"duplicate function found for {tlf.MangledName}");
} else {
Functions.Add (tlf);
}
}
}

Expand Down
30 changes: 16 additions & 14 deletions SwiftReflector/Inventory/PropertyInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,23 @@ public PropertyInventory (int sizeofMachinePointer)

public override void Add (TLDefinition tld, Stream srcStm)
{
TLFunction tlf = tld as TLFunction;
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 7, $"Expected a TLFunction for a property but got a {tld.GetType ().Name}.");

SwiftPropertyType prop = tlf.Signature as SwiftPropertyType;
if (prop == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 8, $"Expected a function of property type but got a {tlf.Signature.GetType ().Name}.");

PropertyContents contents = null;
SwiftName nameToUse = prop.PrivateName ?? prop.Name;
if (!values.TryGetValue (nameToUse, out contents)) {
contents = new PropertyContents (tlf.Class, nameToUse, sizeofMachinePointer);
values.Add (nameToUse, contents);
lock (valuesLock) {
TLFunction tlf = tld as TLFunction;
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 7, $"Expected a TLFunction for a property but got a {tld.GetType ().Name}.");

SwiftPropertyType prop = tlf.Signature as SwiftPropertyType;
if (prop == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 8, $"Expected a function of property type but got a {tlf.Signature.GetType ().Name}.");

PropertyContents contents = null;
SwiftName nameToUse = prop.PrivateName ?? prop.Name;
if (!values.TryGetValue (nameToUse, out contents)) {
contents = new PropertyContents (tlf.Class, nameToUse, sizeofMachinePointer);
values.Add (nameToUse, contents);
}
contents.Add (tlf, prop);
}
contents.Add (tlf, prop);
}

public PropertyContents PropertyWithName (SwiftName name)
Expand Down
16 changes: 9 additions & 7 deletions SwiftReflector/Inventory/ProtocolInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ public ProtocolInventory (int sizeofMachinePointer)

public override void Add (TLDefinition tld, Stream srcStm)
{
SwiftName className = ClassInventory.ToClassName (tld);
SwiftClassName formalName = ClassInventory.ToFormalClassName (tld);
ProtocolContents contents = null;
if (!values.TryGetValue (className, out contents)) {
contents = new ProtocolContents (formalName, sizeofMachinePointer);
values.Add (className, contents);
lock (valuesLock) {
SwiftName className = ClassInventory.ToClassName (tld);
SwiftClassName formalName = ClassInventory.ToFormalClassName (tld);
ProtocolContents contents = null;
if (!values.TryGetValue (className, out contents)) {
contents = new ProtocolContents (formalName, sizeofMachinePointer);
values.Add (className, contents);
}
contents.Add (tld, srcStm);
}
contents.Add (tld, srcStm);
}
}
}
Expand Down
32 changes: 17 additions & 15 deletions SwiftReflector/Inventory/VariableInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,25 @@ public VariableInventory (int sizeofMachinePointer)
}
public override void Add (TLDefinition tld, Stream srcStm)
{
TLVariable vari = tld as TLVariable;
if (vari != null) {
VariableContents contents = GoGetIt (vari.Name);
if (contents.Variable != null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 4, $"duplicate variable {vari.Name.Name}.");
contents.Variable = vari;
return;
}
lock (valuesLock) {
TLVariable vari = tld as TLVariable;
if (vari != null) {
VariableContents contents = GoGetIt (vari.Name);
if (contents.Variable != null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 4, $"duplicate variable {vari.Name.Name}.");
contents.Variable = vari;
return;
}

TLFunction tlf = tld as TLFunction;
if (tlf != null) {
VariableContents contents = GoGetIt (tlf.Name);
contents.Addressors.Add (tlf);
return;
}
TLFunction tlf = tld as TLFunction;
if (tlf != null) {
VariableContents contents = GoGetIt (tlf.Name);
contents.Addressors.Add (tlf);
return;
}

throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 5, $"expected a top-level function or top-level variable but got a {tld.GetType ().Name}");
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 5, $"expected a top-level function or top-level variable but got a {tld.GetType ().Name}");
}

}

Expand Down
17 changes: 10 additions & 7 deletions SwiftReflector/Inventory/WitnessInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace SwiftReflector.Inventory {
public class WitnessInventory {
object valuesLock = new object ();
Dictionary<string, TLFunction> values = new Dictionary<string, TLFunction> ();
int sizeofMachinePointer;

Expand All @@ -21,14 +22,16 @@ public WitnessInventory (int sizeofMachinePointer)

public void Add (TLDefinition tld, Stream srcStm)
{
TLFunction tlf = tld as TLFunction;
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 9, $"Expected a TLFunction for a witness table but got a {tld.GetType ().Name}.");
lock (valuesLock) {
TLFunction tlf = tld as TLFunction;
if (tlf == null)
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 9, $"Expected a TLFunction for a witness table but got a {tld.GetType ().Name}.");

if (values.ContainsKey (tlf.MangledName))
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 10, $"Already received witness table entry for {tlf.MangledName}.");
values.Add (tlf.MangledName, tlf);
LoadWitnessTable (tlf, srcStm);
if (values.ContainsKey (tlf.MangledName))
throw ErrorHelper.CreateError (ReflectorError.kInventoryBase + 10, $"Already received witness table entry for {tlf.MangledName}.");
values.Add (tlf.MangledName, tlf);
LoadWitnessTable (tlf, srcStm);
}
}

public IEnumerable<string> MangledNames { get { return values.Keys; } }
Expand Down
Loading