Replaced string concatenation by StringBuilder usage

[YohDeadfall]

Description

The proposed change is just a simple optimization of mail address collection encoding and touches only the collection type itself, not MailAddress which uses string concatenation internally in many places.

Customer Impact

A bit smaller pressure on the GC.

Regression

Nope.

Testing

Existing tests are enough to cover the change.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

The proposed change is just a simple optimization of mail address collection encoding and touches only the collection type itself, not MailAddress which uses string concatenation internally in many places.

Customer Impact

A bit smaller pressure on the GC.

Regression

Nope.

Testing

Existing tests are enough to cover the change.

Author:	YohDeadfall
Assignees:	-
Labels:	area-System.Net, community-contribution
Milestone:	-

[rzikm]

looks good so far, @YohDeadfall can you look into using ValueStringBuilder as huoyaoyuan suggested?

[YohDeadfall]

Yup, will use it instead of the StringBuilder.

[YohDeadfall]

@rzikm, could you help me with that weird error happening only for some builds?

src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddressCollection.cs(54,40): error CS0122: (NETCORE_ENGINEERING_TELEMETRY=Build) 'ValueStringBuilder' is inaccessible due to its protection level

[ManickaP]

ValueStringBuilder is an internal type shared by linking the source file. You need to add it to the csproj like for example here:

runtime/src/libraries/System.Net.Http/src/System.Net.Http.csproj

Lines 161 to 162 in 9e57b6a

	<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
	Link="Common\System\Text\ValueStringBuilder.cs" />

[YohDeadfall]

That's exactly what I did:

runtime/src/libraries/System.Net.Mail/src/System.Net.Mail.csproj

Lines 71 to 72 in fc389e6

	<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
	Link="Common\System\Text\ValueStringBuilder.cs" />

And it's in the same block where MailAddressCollection.cs is, no more groups where the collection is mentioned. For whatever reason it's not included in some cases while I see no directives which might control that.

[rzikm]

@YohDeadfall The build failure is in unit test project, you need to include the ValueStringBuilder.cs in src\libraries\System.Net.Mail\tests\Unit\System.Net.Mail .Unit.Tests.csproj as well

This is a gimmick of some unit test projects in this repo, we compile the same source codes again as part of unit test projects in order to be able to use internal APIs for testing.

[YohDeadfall]

@rzikm, now it looks fine, thank you!

[rzikm]

CI is passing, the one failure is unrelated. looks good now.

[rzikm]

Thanks a lot for the patience with the contribution!