Skip to content

Commit

Permalink
[objcruntime] Do not pre-compute the Selector.Name property (#12518)
Browse files Browse the repository at this point in the history
it's not often required.

Delaying it's retrieval allows the linker to remove additional code (and
lower the memory for each selector) if the app's code never uses the
selector name.

```diff
@@ -2990,8 +2985,6 @@
 	{
 		private IntPtr handle;

-		private string name;
-
 		public IntPtr Handle => handle;

 		public Selector(IntPtr P_0)
@@ -3001,13 +2994,11 @@
 				ThrowHelper.ThrowArgumentException("sel", "Not a selector handle.");
 			}
 			handle = P_0;
-			name = GetName(P_0);
 		}

 		internal Selector(IntPtr P_0, bool P_1)
 		{
 			handle = P_0;
-			name = GetName(P_0);
 		}

 		public sealed override bool Equals(object? P_0)
@@ -3029,14 +3020,6 @@
 			return handle.GetHashCode();
 		}

-		internal static string GetName(IntPtr P_0)
-		{
-			return Marshal.PtrToStringAuto(sel_getName(P_0));
-		}
-
-		[DllImport("/usr/lib/libobjc.dylib")]
-		private static extern IntPtr sel_getName(IntPtr P_0);
-
 		[DllImport("/usr/lib/libobjc.dylib", EntryPoint = "sel_registerName")]
 		public static extern IntPtr GetHandle(string P_0);

```
  • Loading branch information
spouliot authored Aug 24, 2021
1 parent fbbaa7f commit 207285a
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/ObjCRuntime/Selector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,14 @@ public partial class Selector : IEquatable<Selector>, INativeObject {
#endif

IntPtr handle;
string name;
string? name;

public Selector (IntPtr sel)
{
if (!sel_isMapped (sel))
ObjCRuntime.ThrowHelper.ThrowArgumentException (nameof (sel), "Not a selector handle.");

this.handle = sel;
name = GetName (sel);
}

// this .ctor is required, like for any INativeObject implementation
Expand All @@ -59,7 +58,6 @@ public Selector (IntPtr sel)
internal Selector (IntPtr handle, bool /* unused */ owns)
{
this.handle = handle;
name = GetName (handle);
}

public Selector (string name)
Expand All @@ -73,7 +71,11 @@ public IntPtr Handle {
}

public string Name {
get { return name; }
get {
if (name == null)
name = GetName (handle);
return name;
}
}

public static bool operator!= (Selector left, Selector right) {
Expand Down

5 comments on commit 207285a

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

❌ [CI Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

API Diff (from PR only) (no change)
Generator Diff (only version changes)

Packages generated

View packages

Test results

1 tests failed, 248 tests passed.

Failed tests

  • Documentation/All: Failed

Pipeline on Agent XAMBOT-1033.BigSur'
[objcruntime] Do not pre-compute the Selector.Name property (#12518)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Tests were not ran (VSTS: device tests iOS). ⚠️

Results were skipped for this run due to provisioning problems Azure Devops. Please contact the bot administrator.

Pipeline on Agent
[objcruntime] Do not pre-compute the Selector.Name property (#12518)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

❌ Tests failed on macOS M1 - Mac Big Sur (11.5) ❌

Tests failed on M1 - Mac Big Sur (11.5).

Failed tests are:

  • xammac_tests

Pipeline on Agent
[objcruntime] Do not pre-compute the Selector.Name property (#12518)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Tests were not ran (VSTS: device tests tvOS). ⚠️

Results were skipped for this run due to provisioning problems Azure Devops. Please contact the bot administrator.

Pipeline on Agent
[objcruntime] Do not pre-compute the Selector.Name property (#12518)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

✅ Tests passed on macOS Mac Mojave (10.14) ✅

Tests passed

All tests on macOS X Mac Mojave (10.14) passed.

Pipeline on Agent
[objcruntime] Do not pre-compute the Selector.Name property (#12518)

Please sign in to comment.