Skip to content

[Java.Interop] use StringComparer.Ordinal for Dictionary #550

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

Merged
merged 1 commit into from
Jan 6, 2020

Conversation

jonathanpeppers
Copy link
Member

Context: https://docs.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings#ordinal-string-operations

When profiling startup for a Xamarin.Forms app, I noticed some heavy
calls to Dictionary:

Total(ms) Self(ms)      Calls Method name
      392       49      14826 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:FindEntry (TKey_REF)
      377       18      12744 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:TryGetValue (TKey_REF,TValue_REF&)
      129       14       3827 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:TryInsert (TKey_REF,TValue_REF,System.Collections.Generic.InsertionBehavior)

Quite a few of these are coming from Java.Interop itself, and I
noticed we were using the default constructor for Dictionary. This
means we are using a culture-aware string comparison for every Java
member we lookup.

I could see a small difference in a Release build of the Xamarin.Forms
integration project in xamarin-android:

Before:
01-03 09:26:24.830  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +763ms
01-03 09:26:28.865  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms
01-03 09:26:32.865  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms
01-03 09:26:36.898  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +759ms
01-03 09:26:40.915  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms
01-03 09:26:44.863  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +752ms
01-03 09:26:48.864  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +759ms
01-03 09:26:52.881  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +753ms
01-03 09:26:56.864  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +746ms
01-03 09:27:00.864  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms
Average(ms): 756
Std Err(ms): 1.60554594383897
Std Dev(ms): 5.07718207057594
After:
01-03 09:48:00.954  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +745ms
01-03 09:48:04.988  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms
01-03 09:48:08.987  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +761ms
01-03 09:48:12.953  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +735ms
01-03 09:48:16.987  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +752ms
01-03 09:48:20.971  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +757ms
01-03 09:48:24.938  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms
01-03 09:48:28.921  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +767ms
01-03 09:48:32.888  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +745ms
01-03 09:48:36.922  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +757ms
Average(ms): 753.3
Std Err(ms): 2.97040962382856
Std Dev(ms): 9.39325999498222

I would guess this saves ~2ms to startup?

I also toyed with changing the capacity value to 0, but that seemed
to make things worse. I don't think there is a way for us to "guess"
the default capacity for these dictionaries, so let's let the BCL do
that.

Context: https://docs.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings#ordinal-string-operations

When profiling startup for a Xamarin.Forms app, I noticed some heavy
calls to `Dictionary`:

    Total(ms) Self(ms)      Calls Method name
          392       49      14826 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:FindEntry (TKey_REF)
          377       18      12744 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:TryGetValue (TKey_REF,TValue_REF&)
          129       14       3827 System.Collections.Generic.Dictionary`2<TKey_REF, TValue_REF>:TryInsert (TKey_REF,TValue_REF,System.Collections.Generic.InsertionBehavior)

Quite a few of these are coming from Java.Interop itself, and I
noticed we were using the default constructor for `Dictionary`. This
means we are using a culture-aware string comparison for every Java
member we lookup.

I could see a small difference in a Release build of the Xamarin.Forms
integration project in xamarin-android:

    Before:
    01-03 09:26:24.830  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +763ms
    01-03 09:26:28.865  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms
    01-03 09:26:32.865  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms
    01-03 09:26:36.898  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +759ms
    01-03 09:26:40.915  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms
    01-03 09:26:44.863  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +752ms
    01-03 09:26:48.864  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +759ms
    01-03 09:26:52.881  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +753ms
    01-03 09:26:56.864  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +746ms
    01-03 09:27:00.864  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms
    Average(ms): 756
    Std Err(ms): 1.60554594383897
    Std Dev(ms): 5.07718207057594
    After:
    01-03 09:48:00.954  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +745ms
    01-03 09:48:04.988  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +754ms
    01-03 09:48:08.987  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +761ms
    01-03 09:48:12.953  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +735ms
    01-03 09:48:16.987  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +752ms
    01-03 09:48:20.971  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +757ms
    01-03 09:48:24.938  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +760ms
    01-03 09:48:28.921  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +767ms
    01-03 09:48:32.888  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +745ms
    01-03 09:48:36.922  1473  1503 I ActivityTaskManager: Displayed Xamarin.Forms_Performance_Integration/xamarin.forms.performance.integration.MainActivity: +757ms
    Average(ms): 753.3
    Std Err(ms): 2.97040962382856
    Std Dev(ms): 9.39325999498222

I would guess this saves ~2ms to startup?

I also toyed with changing the `capacity` value to 0, but that seemed
to make things worse. I don't think there is a way for us to "guess"
the default capacity for these dictionaries, so let's let the BCL do
that.
@jonpryor jonpryor merged commit f26bc27 into dotnet:master Jan 6, 2020
@jonathanpeppers jonathanpeppers deleted the dictionary-ctors branch January 6, 2020 21:13
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants