xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.62k stars 1.87k forks source link

[Spec] Backwards compatible merged dictionary speed boost #12699

Open Tommigun1980 opened 4 years ago

Tommigun1980 commented 4 years ago

Microsoft's documentation says to use styles sparingly as they require multiple lookups when using merged dictionaries.

I think even calling it "merged dictionary" is a misnomer as XF keeps a list of dictionaries, one per "merged" dictionary, so the more dictionaries you merge in the more lookups are required. I don't think we should have to choose between putting all styles in one large file or having performance impacted.

Furthermore - when using the new merged dictionary annotation...

<ResourceDictionary.MergedDictionaries>
    <ResourceDictionary Source="SomeDictionary.xaml" />
</ResourceDictionary.MergedDictionaries>

... Resources["SomeValueInSomeDictionary"] will throw a not found exception, while Resources.TryGetValue("SomeValueInSomeDictionary", out object someValue) does work.

When using the old annotation...

<ResourceDictionary.MergedDictionaries>
    <local:SomeDictionary />
</ResourceDictionary.MergedDictionaries>

... both the indexing operator and TryGetValue work. According to https://github.com/xamarin/Xamarin.Forms/issues/12592 this is a design choice that can't be reverted at this time (presumably because of backward compatibility).

Thus I propose the following spec that is 100% backwards compatible but fixes the performance issues and also makes the indexing operator work as expected:

1) Have only one dictionary internally, a true merged dictionary, instead of a collection of dictionaries. 2) Its values would be something like '{ object value, string sourceDictionary, bool isMerged }'. (Duplicate keys would overwrite existing values, with last-in-wins). 3) The indexing operator and TryGetValue would return the '.value' field, so this would be transparent to the user. Same goes of course also for any XAML side access. 4) When unloading a merged dictionary, it would now have all the required information about what to do.

Tommigun1980 commented 3 years ago

https://github.com/xamarin/Xamarin.Forms/issues/12592#issuecomment-716635731 said this may make it to Forms 6, which is MAUI now. Do you guys still think that would be possible?

The speed boost would be very welcome, and the ability to use dictionaries properly would also be nice.

Thank you. @jsuarezruiz