search

dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.92k stars 526 forks source link

[XABT] Replace SortedSet with HashSet. #9280

Closed jpobst closed 2 weeks ago

jpobst commented 2 weeks ago

One question that came up with https://github.com/dotnet/android/pull/9208 was "why SortedSet instead of HashSet"?

Indeed a HashSet seems to be 10ms - 15ms faster to construct than the SortedSet while taking the same ~1ms to perform the SetEquals operation.

grendello commented 2 weeks ago

Using a running hash or checksum might not be accurate enough, as the integer types we'd use for them have limited range. However, what I think might speed things up a little bit more is not using LINQ to populate the HashSet<> instances. LINQ (even in such simple instances like here) hides a lot of overhead, a for loop might be faster.

jpobst commented 2 weeks ago

For the dotnet new android case, which contains ~8500 JLO's due to Mono.Android.dll, the entire operation takes about 5 ms, so there isn't much more performance to squeeze here. 😉

If anything, we could probably omit this step for a Debug build. It is largely just ensuring that something that "can't happen" isn't happening. It's probably safe to skip this for Debug and only do it on Release builds. But that's a bigger change...

grendello commented 2 weeks ago

For the dotnet new android case, which contains ~8500 JLO's due to Mono.Android.dll, the entire operation takes about 5 ms, so there isn't much more performance to squeeze here. 😉

If anything, we could probably omit this step for a Debug build. It is largely just ensuring that something that "can't happen" isn't happening. It's probably safe to skip this for Debug and only do it on Release builds. But that's a bigger change...

Yeah, it's a good idea to omit it for Debug, the assemblies there should be identical.