[Blazor WASM] - Investigate 6% size regression in System.Collections.dll.br #51572

Activity

1. ghost added

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Apr 20, 2021
3. GrabYourPitchforks commented on Apr 21, 2021

   

   GrabYourPitchforks

   on Apr 21, 2021

   Member

   More actions

   Likely related to #51571. Seeing lots of infrastructure to support SortedDictionary and SortedSet coming back into the assembly. Will track down the callers.

   
4. GrabYourPitchforks commented on Apr 21, 2021

   

   GrabYourPitchforks

   on Apr 21, 2021

   Member

   More actions

   I believe the code path below is responsible for this regression.

   runtime/src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonObject.IDictionary.cs

   Lines 108 to 124 in ccc25a9

   	public bool Remove(string propertyName)
   	{
   	if (propertyName == null)
   	{
   	throw new ArgumentNullException(nameof(propertyName));
   	}
   	if (!Dictionary.TryGetValue(propertyName, out JsonNode? item))
   	{
   	return false;
   	}
   	bool success = Dictionary.Remove(propertyName);
   	Debug.Assert(success);
   	DetachParent(item);
   	return true;
   	}

   In particular, on line 120, there's a call to Dictionary.Remove(string). However, the Dictionary property getter is typed as IDictionary<,>, backed by an IDictionary<,> field. This means that the linker needs to keep every possible implementation of IDictionary<,>.Remove around, and in the case of SortedDictionary<,> that means also keeping much of the SortedSet<> infrastructure around as well.

   If the Dictionary property getter is changed to be a strongly-typed Dictionary<,> rather than an interface IDictionary<,> I believe this might resolve the issue, since the target method will now be statically known rather than a virtual dispatch.
5. GrabYourPitchforks

   added

   area-System.Text.Json

   on Apr 21, 2021
7. layomia commented on Apr 21, 2021

   

   layomia

   on Apr 21, 2021

   Contributor

   More actions

   cc @steveharter. This overlaps with the effort to use a strongly-typed sorted dictionary to store JsonObject values (to preserve insertion order when enumerating over the object).

   ---

   That aside, this is related to #51311 and my comment here #51571 (comment).
8. layomia

   removed

   untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

   on Apr 21, 2021
9. layomia

   added this to the 6.0.0 milestone on Apr 21, 2021
10. steveharter

    self-assigned this

    on Apr 21, 2021
11. ghost added

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Apr 22, 2021
12. steveharter

    closed this as completedin #51717on Apr 28, 2021
13. ghost removed

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Apr 28, 2021
14. ghost locked as resolved and limited conversation to collaborators on May 28, 2021